Skip to content

fix(serve): include the learning sidecar in the graph-context cache key - #3044

Open
Azeem1985 wants to merge 1 commit into
Graphify-Labs:v8from
TelB-io:upstream/mcp-sidecar-cache-key
Open

fix(serve): include the learning sidecar in the graph-context cache key#3044
Azeem1985 wants to merge 1 commit into
Graphify-Labs:v8from
TelB-io:upstream/mcp-sidecar-cache-key

Conversation

@Azeem1985

Copy link
Copy Markdown

Fixes #3041.

_GraphContextCache.load keyed each context on graph.json's (st_mtime_ns, st_size) alone, but the .graphify_learning.json sidecar is baked into the cached graph object at load time (_load_graph attaches it as the learning= annotation overlay). graphify reflect rewrites only the sidecar, never graph.json — by design (0.9.3: "structural truth stays separate") — so a long-running server kept serving pre-reflect lesson annotations until graph.json itself happened to change (measured on a live deployment; details in the issue).

Fix: the key now appends the sidecar's (st_mtime_ns, st_size), with (0, 0) standing in when no sidecar exists:

  • sidecar appears → key changes → reload, annotations appear
  • rewritten by a fresh reflect → reload, new verdicts
  • vanishes → key back to (0,0) → reload, annotations gone
  • nothing changed → same 4-tuple → cache hit, no reload cost

Both the pinned default graph and project_path LRU contexts get this (they share load(), which stays the sole owner of file statting and key construction per its docstring).

Tests: end-to-end on a running HTTP server with a counting _load_graph — the full appear/hit/rewrite/vanish cycle with exact load counts — plus the same contract through a project_path context. serve/serve_http/reflect families green on top of v8 @ 0.9.49; ruff clean.

🤖 Generated with Claude Code

The context cache keyed on graph.json's (mtime_ns, size) alone, but the
.graphify_learning.json sidecar is baked into the cached graph object at
load time (_load_graph attaches it as the learning= annotation overlay).
graphify reflect rewrites only the sidecar, never graph.json — so a
long-running MCP server kept serving stale lesson annotations (or none)
until the graph itself happened to change.

The key now appends the sidecar's (mtime_ns, size), with (0, 0) standing
in when no sidecar exists: appearing, changing, and vanishing each
invalidate exactly like a graph change, on both the pinned default graph
and project_path LRU contexts; an unchanged pair still hits the cache.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

Fixes the MCP server's graph-context cache serving stale learning annotations: _GraphContextCache now folds the .graphify_learning.json sidecar's (mtime_ns, size)(0, 0) when absent — into the cache key for both the pinned default graph and project_path contexts. A graphify reflect run, which rewrites only the sidecar and never graph.json, now invalidates the cached context so a running server picks up the new lessons; a sidecar appearing or vanishing invalidates the same way, and an unchanged pair still hits the cache.

No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 678 functions depend on the 353 functions this change touches.

Health — this change adds coupling hotspots:

  • new: dispatch_command() — 2 callers, 122 callees
  • new: _query_graph_text() — 20 callers, 9 callees
  • new: _score_query() — 15 callers, 5 callees
  • new: _query_terms() — 20 callers, 3 callees
  • new: run_benchmark() — 16 callers, 3 callees
  • new: _build_server() — 2 callers, 16 callees
  • new: _load_graph() — 9 callers, 3 callees
  • new: _query_subgraph_tokens() — 7 callers, 3 callees
  • …and 10 more — each is listed as a finding

Verification — 678 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 517 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify load.

The verifier did not have enough to check load, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 69 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly __init__ raised on the synthesized receiver args — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_load\_entry.

The verifier did not have enough to check \_load\_entry, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly __init__ raised on the synthesized receiver args — names the real obstacle, not a sampling gap)

· 1 grounded finding(s) anchored inline below; 17 more finding(s) on lines outside this diff (see the check run).

Comment thread tests/test_serve_http.py
assert loads["n"] == 4


def test_sidecar_change_invalidates_project_context_too(tmp_path, monkeypatch):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regressiontest_sidecar_change_invalidates_project_context_too()

fans out to 7 callees (efferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

@Azeem1985

Copy link
Copy Markdown
Author

Thanks — noting the clean verdict. The fork runs this in production (verified live: a sidecar-only reflect flipped a node tentative→preferred on a running server with no restart). — Azeem1985/TelB-io fork

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant