Repository navigation
fix(build): keep a collapsed edge's weaker evidence in also_seen (#4881) - #4891
Conversation
…phify-Labs#4881) A same-relation collision keeps the higher-confidence edge (Graphify-Labs#3711), and the other copy was discarded, so an AMBIGUOUS or INFERRED duplicate (typically a semantic-pass edge over an AST edge) left no trace in graph.json. The winner now lists each other copy's confidence, confidence_score, source_file and source_location in `also_seen`, weakest first, deduplicated against the winner and each other so re-feeding graph.json through an update is idempotent. A malformed `also_seen` read back from disk is sanitized. When the incoming edge wins, fields only the loser carried are removed from the stored edge: add_edge merges attribute dicts, so the displaced edge's confidence_score used to stay on an EXTRACTED winner that had none. `graphify path` (CLI and MCP shortest_path) unions the also_seen tags into the hop label, as it already does for parallel edges: [AMBIGUOUS/INFERRED]. Fixes Graphify-Labs#4881 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Syed Fahad <sfahad2904@gmail.com>
|
Thanks for the pull request, @SyedFahad7. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
There was a problem hiding this comment.
Graphify reviewed this change.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; formal verification proved 0 of 3 changed function(s) (3 not verified).
Formal verification. PR-changed functions: 0/3 verified (0 proven, 0 may-equivalent, 0 distinguished) · 3 not verified (3 vacuous).
Not verified on this run: build\_from\_json (vacuous: never exercised), dispatch\_command (vacuous: never exercised), \_shortest\_path\_text (vacuous: never exercised).
Graphify review — findings
Adds an optional also_seen list to edges, so when two edges with the same relation between the same nodes collapse in build_from_json, the winner keeps the loser's confidence, score, file and line instead of discarding them. _fold_also_seen dedupes against the winner's own provenance and clears fields the loser alone carried, which keeps re-feeding a written graph.json idempotent. graphify path in the CLI and _shortest_path_text in the server now union the tags from every also_seen copy into the hop label (e.g. [AMBIGUOUS/INFERRED]), and those code paths read also_seen values straight from graph.json, so what they accept as a confidence string matters.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2299 functions depend on the 384 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
build_from_json()— 281 callers, 21 callees - worse:
dispatch_command()— 5 callers, 136 callees - worse:
main()— 116 callers, 3 callees - worse:
_shortest_path_text()— 15 callers, 4 callees
Verification — 2299 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: 2085 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
470 of 470 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/pi.test.ts— full-run-safetytests/test_affected_cli.py— impact, full-run-safetytests/test_affected_defines.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_affected_overrides.py— full-run-safetytests/test_affected_path_file_node.py— impact, full-run-safetytests/test_agents_platform.py— impact, full-run-safetytests/test_alias_folding.py— impact, full-run-safetytests/test_analyze.py— impact, full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— impact, full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— impact, full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_backend_opt_in.py— impact, full-run-safetytests/test_benchmark.py— impact, full-run-safetytests/test_benchmark_raw_graph.py— impact, full-run-safetytests/test_benchmark_real_query.py— impact, full-run-safetytests/test_binary_media_words.py— full-run-safetytests/test_blade_extractor.py— full-run-safetytests/test_build.py— impact, full-run-safetytests/test_build_located_semantic_identity.py— impact, full-run-safetytests/test_build_merge_dedup_scope.py— impact, full-run-safetytests/test_build_merge_hyperedges_and_prune.py— impact, full-run-safetytests/test_build_merge_shrink_guard.py— impact, full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_c_include_resolution.py— impact, full-run-safetytests/test_cache.py— full-run-safetytests/test_cache_long_paths.py— full-run-safetytests/test_cache_stale_import_target.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— impact, full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— impact, full-run-safetytests/test_chunking.py— impact, full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— impact, full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— impact, full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cloud_cta.py— impact, full-run-safety- … and 420 more
non-code file(s) changed (
docs/how-it-works.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
docs/how-it-works.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify build\_from\_json.
The verifier did not have enough to check build\_from\_json, 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: no capturable inputs from the test suite; property tier: not verifiable: all 11 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous
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: no capturable inputs from the test suite; property tier: not verifiable: all 40 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 \_shortest\_path\_text.
The verifier did not have enough to check \_shortest\_path\_text, 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: no capturable inputs from the test suite; property tier: not verifiable: all 35 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly KeyError — names the real obstacle, not a sampling gap)
· 4 more finding(s) on lines outside this diff (see the check run).
…els, path weakest-hop confidence, Type.Member path endpoints Covers the SyedFahad7 batch: #4891 and #4889 merged on GitHub; #4892 and #4893 landed via authorship-preserving cherry-pick (their CI matrix was gated pending first-contributor approval, so brought in through the local gate instead). Credit by GitHub PR-author login @SyedFahad7. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…4888) #4888 was built on 0.9.97, before #4881 (path weakest-hop refactor), #4889 (qualified C# method labels) and #4891 (also_seen) landed. Its cherry-pick dropped the merge commit that reconciled expectations. Update the per-hop confidence assertions to carry scores ([INFERRED 0.85]) and the C# path/explain/neighbor labels to their owner-qualified form (Exporter.Run, IReport.Build) - the correct composed output of all four features. Resolved the serve.py conflict by keeping #4881's _path_weakest/ _path_confidence_line and adding only #4888's _confidence_tag, applied over edge_evidence so parallel/also_seen evidence keeps its scores. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What does this PR do?
Fixes #4881.
When two edges with the same relation land on the same node pair,
build_from_jsonkeeps the higher-confidence one (#3711,graphify/build.pyat the_CONFIDENCE_RANKcomparison) and throws the other away. The usual case is a semantic-pass edge duplicating an AST edge. An AMBIGUOUS or INFERRED duplicate therefore left no trace ingraph.json, even thoughpathalready shows tag unions such as[EXTRACTED/INFERRED]for parallel edges.This change keeps the winner exactly as before and adds the loser's provenance to it:
also_seen: the surviving edge lists every other copy'sconfidence,confidence_score,source_fileandsource_location, weakest first. An entry that only repeats the winner's own provenance, or another entry, is dropped. That keeps a rebuild that re-feedsgraph.json(theupdatepath) idempotent. A malformedalso_seenread back from disk is reduced to its valid scalar fields.G.add_edge(**attrs)merges attribute dicts. When a stronger edge without a score replaced a scored INFERRED one, the loser'sconfidence_scorestayed on the EXTRACTED winner. Provenance fields that only the displaced edge carried are now removed from the stored edge, and that edge's own values go intoalso_seeninstead.graphify pathand the MCPshortest_pathtool includealso_seentags in the hop label, as they already do for parallel edges. For example:--dispatches_to [AMBIGUOUS/INFERRED]-->. This is a one-line change at each of the two hop-formatting sites.docs/how-it-works.mddocuments the new optional edge field.Repro from the issue: build
Report.cs(IReport.Build(),PdfReport/CsvReportimplementing it,Exporter.Run()calling_report.Build()) together with a weaker same-relation duplicate of thereport_ireport_build -> report_pdfreport_builddispatches_toedge, for example AMBIGUOUS 0.4 from a notes file. Before: the stored edge isINFERRED 0.75 L2and the AMBIGUOUS copy is gone. After: the same winner, plusalso_seen: [{confidence: AMBIGUOUS, confidence_score: 0.4, source_file: notes.md, source_location: L12}], in either arrival order.#4885's path-confidence summary is stacked on this branch as PR B (branch
cursor/4885-path-confidence-b0a3).Overlap with #4882 / PR #4888: that PR rewrites the same
confs = sorted(...)hop-tag line inserve.pyandcli.py. Whichever lands second needs a one-line merge per file:sorted({_confidence_tag(e) for d in datas for e in edge_evidence(d) if e.get("confidence")}).Type of change
Verification & Invariants
Invariant: collapsing same-relation duplicates must not erase provenance (CONTRIBUTING, "Preserve provenance" and "Keep inference visibly separate from extracted fact"). The winner's
relation,confidence,confidence_score,source_fileandsource_locationare decided exactly as before. Only the loser's record is added. A field the winner does not carry no longer survives from the loser.Persisted state:
graph.jsonedges may gain an optionalalso_seenlist. Old graphs load unchanged. A rebuild over a graph that already hasalso_seendoes not duplicate entries (covered by a test). Neo4j/FalkorDB pushes already skip non-scalar properties, GraphML JSON-encodes them, and Cypher export only readsconfidence. Extraction output is unchanged, so no AST cache bump is needed.Limitations:
also_seen. That covers a genericreferencesdropped under a specificcalls, the v0.8.22 regression of #760: fresh /graphify build round-trips correct AST output into reversed calls edges (build_from_json -> to_json undirected round-trip) #1061 reverse-direction duplicate, and a membership edge beating a reversecalls. Those are different facts, not weaker copies of the same one.--no-clusterwrite path'sdedupe_edgesstill keeps the first occurrence and does not recordalso_seen.explain/get_nodeneighbour lines still print only the edge's own tag. [Feature]: print confidence_score inpathandexplain#4882 is changing those lines in parallel, so this PR leaves them alone.How was this tested?
tests/test_edge_also_seen.py(10 tests) covers:graph.json;also_seenbeing sanitized;pathshowing[AMBIGUOUS/INFERRED];Report.cs.With the source change stashed, 9 of the 10 fail; the identical-duplicate case is a boundary test.
Graphify-specific checklist
uv run python -m tools.skillgen --bless) when changing their source fragments. (No fragments touched.)