fix: render rationale in graph read surfaces - #2830
Conversation
7559d61 to
60b9b67
Compare
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. 2 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds an optional --rationale/include_rationale flag across the query and explain CLI commands, _query_graph_text/_subgraph_to_text, and the MCP query_graph/get_node tools to surface node rationale (as a compact WHY suffix or a full detail block). Introduces sanitize_rationale in security.py with MAX_QUERY_RATIONALE_CHARS/MAX_DETAIL_RATIONALE_CHARS caps for single-line and multiline modes.
Worth a look
- String "false" enables rationale disclosure —
graphify/serve.py:1738· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
- String "false" enables node rationale disclosure —
graphify/serve.py:1766· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1571 functions depend on the 534 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 98 callers, 51 callees - new:
build_merge()— 46 callers, 14 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
extract_bash()— 40 callers, 10 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
ingest_scip_json()— 76 callers, 4 callees - new:
to_json()— 49 callers, 6 callees - new:
dispatch_command()— 2 callers, 118 callees - …and 45 more — each is listed as a finding
Verification — 1571 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: 1054 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, 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 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_build\_server.
The verifier did not have enough to check \_build\_server, 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 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ImportError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in \_query\_graph\_text (not a proof).
The verifier ran both versions of \_query\_graph\_text on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in \_subgraph\_to\_text (not a proof).
The verifier ran both versions of \_subgraph\_to\_text on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 2 grounded finding(s) anchored inline below; 51 more finding(s) on lines outside this diff (see the check run).
|
|
||
|
|
||
| def _subgraph_to_text(G: nx.Graph, nodes: set[str], edges: list[tuple], token_budget: int = 2000, *, seeds: list[str] | None = None) -> str: | ||
| def _subgraph_to_text(G: nx.Graph, nodes: set[str], edges: list[tuple], token_budget: int = 2000, *, seeds: list[str] | None = None, include_rationale: bool = False) -> str: |
There was a problem hiding this comment.
_subgraph_to_text()
26 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| lines.append(f" Degree: {G.degree(nid)}") | ||
| return "\n".join(lines) | ||
|
|
||
| def _tool_get_neighbors(arguments: dict) -> str: |
There was a problem hiding this comment.
_tool_get_neighbors()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
60b9b67 to
1e5414b
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. 2 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds an opt-in --rationale/include_rationale flag across the query and explain CLI commands and the MCP query_graph/get_node tools, threading it through _query_graph_text, _subgraph_to_text, and _tool_get_node to emit node rationale (as a compact WHY suffix in query output, full text in detail views). Introduces sanitize_rationale in security.py with MAX_QUERY_RATIONALE_CHARS/MAX_DETAIL_RATIONALE_CHARS caps and single-line vs. paragraph-preserving modes. Updates usage strings and tool input schemas accordingly.
Worth a look
- Rationale sanitizer leaves C1 terminal control characters intact —
graphify/security.py:411· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1573 functions depend on the 536 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 98 callers, 51 callees - new:
build_merge()— 46 callers, 14 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
extract_bash()— 40 callers, 10 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
ingest_scip_json()— 76 callers, 4 callees - new:
to_json()— 50 callers, 6 callees - new:
dispatch_command()— 2 callers, 118 callees - …and 45 more — each is listed as a finding
Verification — 1573 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: 1056 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, 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 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_build\_server.
The verifier did not have enough to check \_build\_server, 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 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ImportError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in \_query\_graph\_text (not a proof).
The verifier ran both versions of \_query\_graph\_text on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in \_subgraph\_to\_text (not a proof).
The verifier ran both versions of \_subgraph\_to\_text on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 2 grounded finding(s) anchored inline below; 51 more finding(s) on lines outside this diff (see the check run).
|
|
||
|
|
||
| def _subgraph_to_text(G: nx.Graph, nodes: set[str], edges: list[tuple], token_budget: int = 2000, *, seeds: list[str] | None = None) -> str: | ||
| def _subgraph_to_text(G: nx.Graph, nodes: set[str], edges: list[tuple], token_budget: int = 2000, *, seeds: list[str] | None = None, include_rationale: bool = False) -> str: |
There was a problem hiding this comment.
_subgraph_to_text()
26 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| lines.append(f" Degree: {G.degree(nid)}") | ||
| return "\n".join(lines) | ||
|
|
||
| def _tool_get_neighbors(arguments: dict) -> str: |
There was a problem hiding this comment.
_tool_get_neighbors()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1e5414b to
cd9fb86
Compare
There was a problem hiding this comment.
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. 2 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds an opt-in --rationale flag to the query and explain CLI commands and a matching include_rationale input to the MCP query/node tools, threading it through _query_graph_text and _subgraph_to_text to render a WHY suffix on nodes. Introduces sanitize_rationale in security.py with compact single-line and detail multi-line modes plus char caps (MAX_QUERY_RATIONALE_CHARS/MAX_DETAIL_RATIONALE_CHARS), and extends _CONTROL_CHAR_RE to strip C1 control characters (\x7f-\x9f).
No blocking issues surfaced. 10 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1576 functions depend on the 539 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 98 callers, 51 callees - new:
build_merge()— 46 callers, 14 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
extract_bash()— 40 callers, 10 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
ingest_scip_json()— 76 callers, 4 callees - new:
to_json()— 50 callers, 6 callees - new:
dispatch_command()— 2 callers, 118 callees - …and 45 more — each is listed as a finding
Verification — 1576 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: 1059 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, 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 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_build\_server.
The verifier did not have enough to check \_build\_server, 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 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ImportError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in \_query\_graph\_text (not a proof).
The verifier ran both versions of \_query\_graph\_text on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in \_subgraph\_to\_text (not a proof).
The verifier ran both versions of \_subgraph\_to\_text on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 2 grounded finding(s) anchored inline below; 51 more finding(s) on lines outside this diff (see the check run).
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
Adds an opt-in --rationale/include_rationale path across the query and explain CLI commands, _query_graph_text/_subgraph_to_text, and the MCP query/get_node tools, rendering node rationales as a WHY suffix (compact) or detail line. Introduces sanitize_rationale in security.py with MAX_QUERY_RATIONALE_CHARS/MAX_DETAIL_RATIONALE_CHARS caps, control-char stripping, and single-line vs newline-preserving modes.
Worth a look
- Rationale sanitizer leaves C1 terminal control characters intact —
graphify/security.py:411· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1527 functions depend on the 524 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 98 callers, 51 callees - new:
build_merge()— 43 callers, 14 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
extract_bash()— 40 callers, 10 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
ingest_scip_json()— 76 callers, 4 callees - new:
to_json()— 49 callers, 6 callees - new:
dispatch_command()— 2 callers, 118 callees - …and 43 more — each is listed as a finding
Verification — 1527 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: 1026 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below; 50 more finding(s) on lines outside this diff (see the check run).
| @@ -1725,20 +1763,30 @@ def _tool_query_graph(arguments: dict) -> str: | |||
|
|
|||
There was a problem hiding this comment.
_tool_get_neighbors()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1 participant
Summary
Fixes #2821.
Rationale is already persisted as a node attribute during semantic cleanup, but the existing read surfaces never rendered it. This made the rationale effectively write-only: query could identify the correct node, but the explanation for why that node was relevant was unavailable to the caller.
This PR adds opt-in rationale rendering to the CLI and MCP read surfaces.
Changes
--rationaleto:graphify querygraphify explaininclude_rationaleto:query_graphget_nodeWHY ...for query/query_graph.Rationale:field for get_node/explain.Backward compatibility
Rationale is opt-in and defaults to disabled.
Existing CLI invocations and MCP callers that do not request rationale retain the existing output format.
Tests
Added coverage for:
get_nodeandexplainrendering--rationaleinclude_rationaleinclude_rationaleis omittedFocused test suites:
The full suite also passes all tests relevant to this change; the remaining failures are pre-existing Windows/POSIX-specific tests unrelated to this PR.