plan: CALLS noise + resolution (propose rev5 + PLAN + prompts)#179
Conversation
Locks ORDER BY and edge_filter pushdown, exclude_external stance, HINTS PR-3 checklist, supertype dedup pseudocode, and mini-map cross-link. Status under review; per-PR sentinels in plans/PLAN-CALLS-NOISE.md. Co-authored-by: Cursor <cursoragent@cursor.com>
98327fa to
4ff9fc4
Compare
HumanBean17
left a comment
There was a problem hiding this comment.
Review (revision 4 + plan)
Verdict: approve with doc fixes before merge. This is a solid implementation contract on top of merged #178. I would not start PR-1 until fixture anchors and CURSOR-PROMPTS-CALLS-NOISE.md are settled.
What looks good
- ORDER BY + pushdown (Decisions 36–37) match real code:
neighbors_v2flat CALLS path has noORDER BYand slices in Python (mcp_v2.py~1391–1459). LockingORDER BY e.call_site_line, e.call_site_bytewith predicates beforeoffset/limitis the right fix for #177-style truncation. - Phantom split line refs are accurate: remove
chained_receiver(1190–1200) and unresolved-receiverphantom(1202–1212); keep known-external (1257–1271) andoverload_ambiguous(1284–1289). - §3.9.1 HINTS checklist is necessary —
FUZZY_STRATEGY_SETstill includesphantom/chained_receiver; PR-3 will break hints without H1–H8. - 3-PR landing order (additive schema → MCP filter → breaking graph) is sound.
Blockers (please fix in this PR)
-
Fixture / test anchors are fictional or absent on
bank-chat-system:OrderService.processdoes not exist in the fixture (HV1, HV21, HV34, perf test name). Pin a real method — e.g.ClientMessageProcessor#process(~57 outbound CALLS on a fresh bank index).test_pass3_supertype_dedup_jpa_repository_save_one_row(MyRepository extends JpaRepository) — no such type in bank; also zero same-site duplicatesaveCALLS today. Usetests/fixtures/call_graph_smoke/or a small dedicated fixture pertests/README.md.test_pass3_overload_ambiguous_still_n_rowson bank —overload_ambiguouscount is 0 in bank graph; needs a dedicated fixture, not bank alone.
-
Internal contradiction on
exclude_external: Decision 38 / §3.4.2 correctly say it is not onneighbors, but §4.5 HV37 note still says agents can useexclude_external=Truefor library noise without scoping tofind_callers/find_callees. Please fix that sentence (and grep for similar slips). -
Test name drift: propose §3.4.1 has
test_neighbors_calls_edge_filter_pushdown_role; plan PR-2 hastest_neighbors_calls_edge_filter_pushdown_in_cypher— pick one verbatim name in both files. -
Plan header: still points at #178 as the active propose PR; #178 is already merged. Update to #179 / “rev4 in this PR”.
-
Missing
CURSOR-PROMPTS-CALLS-NOISE.md: plan defers until PR-1. Repo norm is plan + prompts together (plans/completed/CURSOR-PROMPTS-*.md). Please add a skeleton prompt file in this PR or open a follow-up before any code lands.
Non-blocking (implementation / prompts)
- Post–PR-3 JDK noise via
exclude_callee_declaring_roles: ['OTHER']is blunt (known-external rows are alsoOTHER) — ensure AGENT-GUIDE states this clearly. - Perf test
1.5× medianwill flake in CI unless gated (JAVA_CODEBASE_RAG_RUN_HEAVYor skip) — plan says “optional heavy” but also lists under deliverables; resolve in prompts. - Prefer deferring
unresolved-callsCLI stub to PR-3 (empty output before UCS tables is misleading). - PR-3: specify pagination / tie-break for
include_unresolvedinterleave and canonical line whendedup_calls=True. - PR-2 sentinels: treat
rg "LIMIT.*call_site"as advisory; rely on named tests for pushdown-before-limit. - PR-1 still changes row count via supertype dedup — call that out in README re-index notes (not only “additive column”).
Merge checklist
- Replace
OrderService.processwith real bank method IDs (or label as illustrative + pin test IDs). - Fixture paths for supertype dedup +
overload_ambiguoustests. - Fix §4.5 HV37
exclude_externalwording. - Unify pushdown test name; update plan header (#178 → #179).
- Add
CURSOR-PROMPTS-CALLS-NOISE.mdor explicit follow-up before code PRs.
Happy to re-review after a small doc pass.
Pin ClientMessageProcessor#process fixture anchors, fix HV37 exclude_external wording, unify test names, defer CLI to PR-3, and add per-PR Cursor handoffs. Co-authored-by: Cursor <cursoragent@cursor.com>
Correct appendix B (prompts landed in #179); add prompts link in header. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
propose/CALLS-NOISE-AND-RESOLUTION-PROPOSE.mdto revision 5 (status: under review) — fixture anchors, HV37 wording, and plan alignment after #179 review.plans/PLAN-CALLS-NOISE.md— 3-PR landing order, pinned fixture anchors, named tests, per-PR sentinel greps.plans/CURSOR-PROMPTS-CALLS-NOISE.md— per-PR Cursor handoffs (PR-1 → PR-3). Do not open PR-1 code until this PR merges.Builds on merged propose #178 (revision 3 on
master). This branch adds revision 4 implementation-contract patches plus revision 5 anchor/prompt fixes.Tracks #177.
What changed in revision 4 (propose)
neighbors(out, ['CALLS'])— source-order transcript at MCP, not just in the graph.edge_filterpushdown — filter in Cypher beforelimit, not post-fetch truncation.exclude_externalstays onfind_callersonly; JDK noise onneighborsusesedge_filter./mini-mapcross-link for accessor noise.What changed in revision 5 (this branch)
ClientMessageProcessor#process(bank);call_graph_smokefor supertype dedup +overload_ambiguous(not fictionalOrderService/MyRepository).exclude_externalonly onfind_callers/find_callees;OTHERrole filter bluntness documented.pushdown_in_cypher, perf test..._client_message_processor, heavy-gated).unresolved-callsCLI deferred to PR-3 (no empty stub).dedup_callscanonical line in plan.CURSOR-PROMPTS-CALLS-NOISE.md— iteration-loop tests, sentinels, scope per PR.Plan (no code in this PR)
callee_declaring_roleon CALLS + supertype dedup + countersedge_filteronneighbors_v2+ ORDER BY + pushdownUnresolvedCallSiteTest plan
plans/CURSOR-PROMPTS-CALLS-NOISE.md