propose: CALLS noise + resolution — resolved-only edges + EdgeFilter#178
Conversation
Draft propose addressing #177. Two locked moves, no edge-type explosion: 1. CALLS carries only resolved invocations. Phantom + chained-receiver sites move to caller-side UnresolvedCallSite node + UNRESOLVED_AT edge. Phantom Symbol rows deleted from the graph. 2. callee_declaring_role becomes a CALLS edge attribute. neighbors_v2 gains a typed EdgeFilter surface (min_confidence, exclude_strategies, callee_declaring_role, ...) that projects the ordered stream by attribute without breaking (call_site_line, call_site_byte) order. 3 sequential code PRs (PR-1 resolved-only CALLS + ontology v15; PR-2 EdgeFilter + CLI; PR-3 dedup + HINTS-V4 high-fanout template). 19 locked decisions. 20-row use-case re-walk. Explicitly supersedes the MCP-V2 'no per-edge filter on neighbors' design rule.
…, Decisions 20-33 Round-1 author grill amendments: - Add §4.5 'Pre-#177 use cases (regression-style)' with HV21-HV37 - Add Surface B: neighbors_v2(include_unresolved=True) interleaves UnresolvedCallSite projections with CALLS rows in (line,byte) order - Add row_kind discriminator ('call_edge' | 'unresolved_call_site') - Add second HINTS-V4 template: TPL_NEIGHBORS_CALLS_HAS_UNRESOLVED - Lock Decisions 20-33: 20 callee_declaring_role source; 21 depth>1 filter at every hop; 22 NodeFilter+EdgeFilter AND-compose; 23 unresolved telemetry counters; 24 cursor-pr-review consumer; 25 cross-MS skips not in UnresolvedCallSite; 26 include_unresolved mutually exclusive with edge_filter; 27 no MCP find-unresolved-callers; 28 find_callers keeps min_confidence; 29 package-relativity OOS; 30 brownfield role transparent; 31 NodeFilter.role vs EdgeFilter.callee_declaring_role NOT renamed; 32 Kuzu predicate-pushdown perf named-scenario; 33 only role exposed - Expand out-of-scope table with 6 new rows - Update Appendix B traceability
Six fixes from author-grill round 2: G1+G2: Drop Decision 26 mutual-exclusivity (now Decision 25, reversed). include_unresolved composes with edge_filter — filter applies to resolved rows; unresolved rows pass through unfiltered. Add HV38 (delegation skeleton + unresolved sites looking like delegations). G3: row_kind is global on EdgeRowBase (default 'resolved'), not CALLS-only. Future-proofs the discriminator for similar splits. G6: New Decision 33 — pass3_calls dedups multi-candidate resolution by (src_id, call_site_line, call_site_byte) BEFORE emit. Preference: concrete > interface, same-MS > cross-MS, first candidate as tiebreak. One source-line, one CALLS edge, one callee_declaring_role. Add HV37 covering the fixture test. G8: Delete HV28 (cursor-pr-review use case) and Decision 24 (cursor-pr-review as downstream consumer). .cursor skills are dev-time tooling, not end-agent workflows on the indexed codebase. Renumber HV29-HV37 → HV28-HV36, Decisions 25-33 → 24-32. G10: Decision 23 replaces JSON-map column with three discrete INT64 counters (pass3_unresolved_phantom_receiver / _chained / _name_only). Kuzu can WHERE/SUM these directly. Plus: §5 out-of-scope rows for callee_microservice and unresolved_filter axis to pre-empt obvious bolt-ons. HV35 (perf) names the OrderService.process scenario and pre-PR-2 baseline. HV34/Decision 30 collision-hint trigger refined to 'dominantly OTHER rows' (zero-rows under-fires). Decisions: 33 (1-33 contiguous, no gaps). Use cases: HV1-HV20 + HV21-HV38 = 38 rows. Principles: 8 (unchanged).
HumanBean17
left a comment
There was a problem hiding this comment.
Critical review pass on the propose. I think the issue is real and the core direction is good (keep CALLS ordered; add role-aware projection instead of splitting semantic edge types), but I would not merge this propose as-is.
Main blockers:
-
The plan drops useful known-receiver external-call data. Current
CALLSdistinguishes true zero-confidence receiver failures from the case where the receiver is resolved but the callee method is not indexed (JDK/Spring/external/library calls). That path preservesconfidence,strategy,arg_count, and a deterministic phantom FQN; README also documents JDK/Spring/Lombok callees as phantom method symbols. The proposedUnresolvedCallSite(reason='name_only_zero_candidates')shape only carriescallee_simple,receiver_expr, andreason, so it loses graph-quality and transcript data. Either preserve those fields inUnresolvedCallSite, or keep known-receiver external calls as non-traversableCALLS/separate external-call rows. -
Build-time dedup by
(src_id, call_site_line, call_site_byte)erases resolver truth. Current multi-candidate emission is howoverload_ambiguousrepresents ambiguity. If the concrete bug is interface+concrete duplication for one source call, solve that specific duplicate class. A blanket pre-emit winner selection makesfind_callers/ explain flows see one arbitrary callee and hides ambiguity that agents may need to know about. -
Silent no-op
edge_filtersemantics conflict with the existing strict filter contract.mcp_v2.pycurrently frames filters as typed predicates where inapplicable fields fail loud with a teaching message. Returning filteredCALLSplus unfilteredOVERRIDESforedge_types=['CALLS','OVERRIDES'], edge_filter={callee_declaring_role:'SERVICE'}is surprising and likely to reintroduce noise. I would prefer fail-loud or an explicit per-edge-type filter shape. -
include_unresolved=True+edge_filterreintroduces unfiltered noise. Withedge_filter={callee_declaring_role:'SERVICE'}, the proposed behavior returns SERVICE resolved calls plus all unresolved sites, not “service-like unresolved sites.” That undermines the filter. Make these mutually exclusive again, or add a real unresolved filter axis once a concrete workflow justifies it. -
The propose invents
neighbors_v2(depth=2)semantics. Currentneighbors_v2is one-hop only and the README frames MCP walking as one hop at a time. If depth is intended, it needs to be a first-class MCP surface change with docs/tests/tool-description updates; otherwise remove HV28 / Decision 21. -
CLI naming is inconsistent. The proposal says
pc unresolved-calls, but the operator CLI in this repo isjava-codebase-ragwith top-level subcommands. Unlesspcis a planned alias, the propose should usejava-codebase-rag unresolved-calls ....
Suggested safer sequence: PR-1 add callee_declaring_role and edge_filter without deleting rows; PR-2 move only true zero-confidence receiver/chained failures out of CALLS, preserving known-receiver external-call metadata; PR-3 revisit dedup as response-shape only, not build-time candidate loss.
|
Thanks — this is the right grilling. All six blockers stand; I agree with the substance of every one. Itemized: #1 — Known-receiver external calls data loss → AGREE. I missed that The fix I'll lock: only true receiver-failure sites move out of CALLS. Concretely:
That means PR-1's #2 — Build-time dedup erases Fix: rewrite Decision 33 as supertype-walk dedup only. When #3 — Silent no-op Fix: lock that This kills Principle 7 in its current form. Rewriting it as "edge_filter is single-edge-type-scoped and fail-loud on inapplicable attributes." #4 — Fix: re-lock the original mutual exclusivity ( #5 — Fix: delete HV28 + Decision 21. Add row to §5 out-of-scope: "Multi-hop #6 — CLI naming → AGREE. Mechanical fix. On the suggested safer sequence: I think it's strictly better than my 3-PR ordering and I'll adopt it:
This makes PR-1 and PR-2 strictly additive (zero behavior change for existing readers), and PR-3 the only breaking change — with a much narrower blast radius than before. Amendment plan (single force-push to
Will land as a single revision-3 force-push. Final counts will likely be ~30 decisions and ~36 use cases. Going to apply now. |
Applies the 12-item amendment plan posted at #178 issuecomment-4487386547. Reverses three round-1/round-2 decisions that the external review correctly flagged as data-loss or contract violations. Six blockers addressed: 1. Known-external CALLS preservation (build_ast_graph.py:1257-1271). - Decision 1 / Principle 5 / §3.1 / §3.3 / §3.10 / HV37 / Decision 34: CALLS sheds *only* strategy='phantom' (unresolved receiver) and strategy='chained_receiver'. Receiver-resolved-but-callee-not-indexed rows stay in CALLS with resolved=False and preserved receiver-tier strategy/confidence/arg_count metadata. - Decision 2 reversed: 'resolved BOOLEAN' STAYS in CALLS DDL. 2. Multi-candidate dedup scope-narrowed (Decision 33). - Supertype-walk dedup only (one concrete + N inherited supertype declarations -> collapse to concrete). - overload_ambiguous rows preserved as N rows. The 'one row per ambiguous site' phrasing from round-2 deleted; resolver ambiguity stays visible to find_callers/explain. 3. edge_filter fail-loud (Principle 7 + Decision 10 reversed; HV13). - Mixed edge_types + edge_filter raises ValueError with a teaching message matching the existing _nodefilter_inapplicable_fields pattern at mcp_v2.py:191-206. Silent no-op was the wrong call. 4. include_unresolved + edge_filter mutex restored (Decision 25 + HV23). - The round-2 'compose them' reversal would have reintroduced unfiltered phantom/chained noise into the interleaved transcript view. Mutually exclusive (fail-loud), as in revision 1. 5. neighbors_v2 stays one-hop. - HV28 (depth=2 walk) deleted entirely. Decision 21 deleted. - README:12 + mcp_v2.py:39 ('Stored graph edge labels for one-hop neighbors.') are the locked contract; multi-hop needs its own propose (visited-set, cycles, fanout cap, hint behavior). 6. CLI binary rename: 'pc' -> 'java-codebase-rag'. - docs/JAVA-CODEBASE-RAG-CLI.md:1-17 is the source of truth. - All §3.6 / HV8 / HV9 / HV26 CLI references updated. Counters reduced 3 -> 2: - pass3_unresolved_phantom_receiver, pass3_unresolved_chained. - No more pass3_unresolved_name_only (those rows stay in CALLS). - UnresolvedCallSite.reason enum reduced to two values; name_only branch removed. UnresolvedCallSite gains arg_count INT64. New decisions: - Decision 34: known-receiver-external rows preserved in CALLS. - Decision 35: fail-loud validator on _nodefilter_inapplicable_fields. Sub-PR sequence reordered for safer adoption (per reviewer's suggestion): - PR-1: callee_declaring_role + supertype-walk dedup + GraphMeta counters (strictly additive). - PR-2: EdgeFilter + CLI stub (strictly additive). - PR-3: UnresolvedCallSite + UNRESOLVED_AT + phantom/chained branch removal + include_unresolved + dedup_calls (only breaking PR). Counts: 35 decisions (was 33), 37 use cases (HV1-HV37; deleted HV28 and the old HV38, added HV37), 8 principles (Principle 7 reversed), 3 PRs. Tracks: #177 PR: #178
Status: draft
Tracks: #177
Adds
propose/CALLS-NOISE-AND-RESOLUTION-PROPOSE.md.Frame
CALLSis a sequence (ordered bycall_site_line, call_site_byte), not a set. Source-order traversal is the dominant agent use case and must be preserved. Today's noise (~80% in the #177 sample) comes from two unrelated burdens being carried on one edge:Two locked moves
CALLScarries only resolved invocations. Phantom + chained + name-only-zero-candidates sites move to a sibling node tableUnresolvedCallSite+UNRESOLVED_ATedge from the caller Symbol. Phantom Symbol rows +_phantom_method_id+tables.phantomsdeleted. Re-index required; ontology v14 → v15.callee_declaring_rolebecomes aCALLSedge attribute, populated atpass3_callsemission time from the callee parent'srole.neighbors_v2gains a typedEdgeFilterPydantic model (min_confidence,exclude_strategies,include_strategies,callee_declaring_role,callee_declaring_roles,exclude_callee_declaring_roles). Default ordering by(call_site_line, call_site_byte)is unchanged when filter is omitted.Why not a semantic split
A
CALLS→DELEGATES_TO/PERSISTS_VIA/ACCESSES_STATE/UNRESOLVED_CALLsplit was the first reframe I tried — it fails the test "edge type should encode semantic intent only when the agent never needs to interleave edges of different types in a single traversal." Reading a method body is exactly the interleaved case; a split would force 4-way fan-merge on(line, byte). Recorded as locked Decision 6.Three sub-PRs (sequential)
UnresolvedCallSite+UNRESOLVED_AT+describe.unresolved_call_sitesrollup. Ontology bump. Re-index.EdgeFilteronneighbors_v2+pc unresolved-callsCLI subcommand + explicit supersession of the MCP-V2 "no per-edge filter" design rule.dedup_callsonneighbors_v2(default off) +TPL_NEIGHBORS_CALLS_HIGH_FANOUTHINTS-V4 success-path template (threshold = 10).19 locked decisions
Including: no semantic split;
resolved BOOLEANremoved from CALLS DDL;UnresolvedCallSiteis a sibling node table (not a JSON column);callee_declaring_roleis CALLS-specific (no symmetry on HTTP_CALLS/ASYNC_CALLS);find_callersdoes not gaincallee_declaring_rolefiltering;dedup_calls=Falseby default; high-fanout template threshold = 10; no back-compat alias for the removedresolved=Falserows.20-row use-case re-walk
Covers ordered transcript (HV1, HV2), filter projections (HV3, HV4, HV5),
find_callerssemantics (HV6, HV17),trace_request_flowstep (HV7), graph-quality CLI / describe rollup (HV8, HV9), HINTS-V3 interaction (HV10), edge-attribute filter semantics (HV11, HV12, HV13, HV14), dedup (HV15), success-path hint (HV16), 100%-unresolved edge case (HV18), CI invariants (HV19, HV20).Files
propose/CALLS-NOISE-AND-RESOLUTION-PROPOSE.mdDrafted per the propose-doc-author skill structure. Will move
Statustounder reviewafter first review pass.