analysis: restore duplicate-FnKey tripwire tolerant of parse-recovered keys by 0xGeorgii · Pull Request #263 · Inferara/inference

0xGeorgii · 2026-07-16T13:28:36Z

Fixes #255.

Restores the duplicate-FnKey tripwire in call_graph.rs::resolve_adjacency that PR #239 removed — in a form that tolerates parse-recovered keys, so the resilient IDE path stays quiet while genuine identity bugs (the #63 class: missed self-edges in A035/A036) become loud again in debug builds.

Changes

On a duplicate FnKey the map still keeps-first (release behavior unchanged), but a debug_assert now trips unless the offending key is parse-recovered.
New PARSE_RECOVERY_MARKER ("<error>") + is_parse_recovered(&FnKey) checking the name/struct_name/spec segments (never module_path, which is filesystem-derived) for the parser's error sentinel from create_error_definition/lower_name_or_error.
Tolerance-predicate approach chosen over compiler-vs-IDE gating because resolve_adjacency is reached only via A035/A036 through a plain TypedContext with no path signal at the call site. Kept local to call_graph.rs rather than on FnKey itself to avoid spreading the parser sentinel into the shared identity crate.
Function comment rewritten to explain the tripwire and the Implement mod hierarchy #63 rationale; CHANGELOG documents both the PR LSP server #239 behavior change and this restoration.

Testing

adjacency_tolerates_duplicate_recovered_key (all builds), a predicate-coverage test across every FnKey variant segment, a debug-only tripwire test on a genuine duplicate (cfg(debug_assertions) + catch_unwind with muted panic hook — CONTRIBUTING bans #[should_panic]), and a release-only keep-first determinism test.
cargo test -p inference-analysis debug (77) and --release call_graph suites green; cargo test -p inference-tests analysis 470 green (A035/A036, cross-file recursion); clippy clean debug+release; full default-workspace cargo test green.

Confidence Score: 4/5

Production logic is correct and the release/IDE paths are unaffected; the only gaps are in test coverage and a process-global side effect in one debug-only test.

The is_parse_recovered predicate and debug_assert are implemented correctly across all four FnKey variants. Both findings are confined to the test suite: the predicate coverage test asserts only five of the eight recoverable segments, and the tripwire test replaces the global panic hook without synchronisation.

core/analysis/src/call_graph.rs — specifically the parse_recovered_predicate_matches_every_synthesized_segment and adjacency_tripwire_fires_on_genuine_duplicate_in_debug test functions

Important Files Changed

Filename	Overview
core/analysis/src/call_graph.rs	Restores the duplicate-FnKey debug_assert in resolve_adjacency, adds the is_parse_recovered predicate, and ships four new tests; production logic is correct but the predicate coverage test is missing three of eight positive segment cases and the debug-build tripwire test manipulates a process-global panic hook unsafely under parallel test execution.
CHANGELOG.md	Adds entries documenting the PR #239 regression and its restoration, plus two new link anchors; straightforward documentation change with no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["resolve_adjacency(nodes)"] --> B["iterate nodes"]
    B --> C{"key already in index?"}
    C -- No --> D["index.insert(key, i)"]
    C -- Yes --> E["debug_assert: is_parse_recovered(key)?"]
    E -- "true (recovered key)" --> F["continue - keep first, no assertion fires"]
    E -- "false (genuine duplicate)" --> G["debug_assert fires - panic in debug / no-op in release"]
    G --> H["continue - keep first"]
    D --> B
    F --> B
    H --> B
    B -- done --> I["build adjacency rows for all nodes"]

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["resolve_adjacency(nodes)"] --> B["iterate nodes"]
    B --> C{"key already in index?"}
    C -- No --> D["index.insert(key, i)"]
    C -- Yes --> E["debug_assert: is_parse_recovered(key)?"]
    E -- "true (recovered key)" --> F["continue - keep first, no assertion fires"]
    E -- "false (genuine duplicate)" --> G["debug_assert fires - panic in debug / no-op in release"]
    G --> H["continue - keep first"]
    D --> B
    F --> B
    H --> B
    B -- done --> I["build adjacency rows for all nodes"]

_{Reviews (1): Last reviewed commit: "Restore duplicate-FnKey tripwire in call..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

PR #239 (LSP server) rewrote resolve_adjacency to keep-first on any duplicate FnKey in every build, dropping the previous debug_assert!(false) that guarded FnKey injectivity. That guard was a tripwire for the same-named-type / canonical-key identity bug class this repo has shipped (#63): when two real functions fold to one key, a recursive self-edge can resolve to the wrong same-keyed node and mask a cycle from A035/A036. The removal was necessary, not gratuitous: the resilient IDE path lowers every unparseable construct to an `<error>` placeholder function, so a broken parse legitimately yields two nodes under one key, and the old assert aborted debug builds (and the LSP process) on it. This reinstates the tripwire in a form that tolerates parse recovery. A duplicate key now trips the debug_assert only when the key is not parse-recovered: is_parse_recovered inspects the name/struct/spec identity segments of the FnKey for the parser's `<error>` sentinel (the module_path qualifier is filesystem-derived and never synthesized by recovery, so it is not consulted). Recovered duplicates are exempt in every build, and keep-first behavior is unchanged, so release builds and resilient parses degrade deterministically. resolve_adjacency is reached only through A035/A036, both of which take a plain TypedContext with no compiler-vs-resilient signal, so gating on the entry point is not available here; the tolerance predicate is the robust choice. Tests: a recovered `<error>` duplicate does not trip the guard in any build; a genuine duplicate trips the debug_assert in debug builds (gated with cfg(debug_assertions) and catch_unwind rather than the banned #[should_panic], with the panic hook muted); the release keep-first path is pinned under cfg(not(debug_assertions)); and the predicate is exercised across every FnKey variant segment. CHANGELOG records both the PR #239 behavior change and this restoration. Fixes #255

greptile-apps · 2026-07-16T13:31:48Z

+    fn parse_recovered_predicate_matches_every_synthesized_segment() {
+        assert!(is_parse_recovered(&FnKey::free_in(Vec::new(), "<error>")));
+        assert!(is_parse_recovered(&FnKey::method_in(
+            path(&["a"]),
+            "<error>",
+            "m"
+        )));
+        assert!(is_parse_recovered(&FnKey::method_in(
+            path(&["a"]),
+            "T",
+            "<error>"
+        )));
+        assert!(is_parse_recovered(&FnKey::spec_free_folded(
+            &[],
+            "<error>",
+            "f"
+        )));
+        assert!(is_parse_recovered(&FnKey::spec_method_folded(
+            &[],
+            "S",
+            "<error>",
+            "m"
+        )));
+
+        assert!(!is_parse_recovered(&FnKey::free_in(path(&["a"]), "real")));
+        assert!(!is_parse_recovered(&FnKey::method_in(
+            path(&["a"]),
+            "T",
+            "m"
+        )));
+        assert!(
+            !is_parse_recovered(&FnKey::free_in(path(&["<error>"]), "real")),
+            "a filesystem-derived module segment is not a recovery marker"
+        );
+    }


Missing positive assertions for SpecFree.name, SpecMethod.spec, and SpecMethod.name

The PR description says the predicate test covers "every FnKey variant segment," but three out of eight recoverable segments are untested:

SpecFree { name: "<error>", .. } — only spec is exercised, not name

SpecMethod { spec: "<error>", .. } — only struct_name is exercised

SpecMethod { name: "<error>", .. } — neither spec nor name is tested

If the parser ever synthesises a SpecFree key with a valid spec but an error-recovered function name (or similarly for SpecMethod), the predicate would still return true (the implementation is correct), but the test would not catch a regression that silently zeroed one of those branches.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

greptile-apps · 2026-07-16T13:31:49Z

+    #[cfg(debug_assertions)]
+    #[test]
+    fn adjacency_tripwire_fires_on_genuine_duplicate_in_debug() {
+        let dup_key = || FnKey::free_in(path(&["lib"]), "collide");
+        let prev_hook = std::panic::take_hook();
+        std::panic::set_hook(Box::new(|_| {}));
+        let result = std::panic::catch_unwind(|| {
+            let nodes = vec![node(dup_key(), vec![]), node(dup_key(), vec![])];
+            resolve_adjacency(&nodes)
+        });
+        std::panic::set_hook(prev_hook);
+        assert!(
+            result.is_err(),
+            "a genuine (non-recovered) duplicate FnKey must trip the debug tripwire"
+        );
+    }


Global panic hook mutation races with concurrent tests

std::panic::take_hook / std::panic::set_hook write process-global state. When the test suite runs in parallel (the default), any other thread that panics between the two set_hook calls will have its panic output silently swallowed by the no-op hook. A genuine test failure in a concurrently executing test could be hidden, making CI failures hard to diagnose. The comment acknowledges #[should_panic] is banned per CONTRIBUTING, but the tradeoff is that any unexpected panic from an unrelated test running at the same time becomes invisible for the duration of this window.

codecov · 2026-07-16T13:33:46Z

Codecov Report

❌ Patch coverage is 98.46154% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
core/analysis/src/call_graph.rs	98.46%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

greptile-apps Bot reviewed Jul 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

analysis: restore duplicate-FnKey tripwire tolerant of parse-recovered keys#263

analysis: restore duplicate-FnKey tripwire tolerant of parse-recovered keys#263
0xGeorgii wants to merge 1 commit into
mainfrom
255-refactoring-call-graph-fnkey-tripwire

0xGeorgii commented Jul 16, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

greptile-apps Bot Jul 16, 2026

Uh oh!

greptile-apps Bot Jul 16, 2026

Uh oh!

codecov Bot commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

0xGeorgii commented Jul 16, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Testing

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 16, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

0xGeorgii commented Jul 16, 2026 •

edited by greptile-apps Bot

Loading