Skip to content

feat: retarget pass6 hint recovery to Client declarations (PR-LC2)#42

Merged
HumanBean17 merged 3 commits into
masterfrom
feat/list-clients-lc2-pass6-client-hints
May 6, 2026
Merged

feat: retarget pass6 hint recovery to Client declarations (PR-LC2)#42
HumanBean17 merged 3 commits into
masterfrom
feat/list-clients-lc2-pass6-client-hints

Conversation

@HumanBean17
Copy link
Copy Markdown
Owner

Scope

Implements PR-LC2 from plans/PLAN-LIST-CLIENTS-MCP-TOOL.md by retargeting pass6 hint recovery from caller http_consumer route fallback to caller-side Client declarations (Symbol -> DECLARES_CLIENT -> Client).

This PR intentionally preserves the existing matching semantics and HTTP_CALLS(Symbol -> Route) meaning while adding focused regression coverage for parity and continuity.

What Changed

  • Updated build_ast_graph.py pass6 fallback hint lookup:
    • Builds member-to-client hint index from declares_client_rows and client_rows.
    • When HTTP_CALLS row points to a missing route id during pass6 rematch, recovers Feign hints from persisted Client rows (not from member.decl.outgoing_calls).
    • Reads path, target_service, and method from Client and synthesizes the same transient Feign-style hint route used by existing matcher logic.
  • Added tests/test_client_hint_recovery.py with 4 focused tests:
    1. test_pass6_uses_client_hints_for_feign_resolution
    2. test_cross_service_match_outcome_unchanged_after_client_migration
    3. test_find_route_callers_still_returns_expected_feign_caller
    4. test_missing_client_hint_falls_back_to_existing_unresolved_or_phantom_flow

Semantics / Non-Goals

  • Match outcomes remain unchanged: cross_service, intra_service, ambiguous, phantom, unresolved.
  • HTTP_CALLS edges still resolve caller Symbol to callee Route.
  • No LC3 surface changes (list_clients tool/DTO/docs).
  • No LC1 redesign beyond consuming existing Client / DECLARES_CLIENT rows.

Validation

Lint

  • ruff check .

Targeted tests

  • pytest tests/test_client_hint_recovery.py -v ✅ (4 passed)

Full suite

  • pytest tests -v ✅ (302 passed, 4 skipped)

Additional continuity checks

  • pytest tests/test_feign_not_exposer.py::test_feign_caller_resolves_to_target_endpoint tests/test_client_hint_recovery.py::test_find_route_callers_still_returns_expected_feign_caller -v

Sentinel checks

  • rg "@mcp.tool\(name=\"list_clients\"|ClientRowDto|ClientsListOutput" server.py kuzu_queries.py -> no matches ✅
  • rg "CREATE NODE TABLE Client|CREATE REL TABLE DECLARES_CLIENT|ontology_version\s*=\s*10" build_ast_graph.py java_ontology.py -> only existing LC1 schema lines in build_ast_graph.py, no broader LC1 reshape ✅

Manual evidence

Rebuilt graph per plan instruction:

  • python build_ast_graph.py --source-root tests/bank-chat-system --kuzu-path /tmp/check_lc2 --verbose

Captured behavior:

  • pass6 execution remains stable with expected match breakdowns.
  • Feign cross-service resolution continuity verified by regression tests.
  • find_route_callers continuity verified by regression tests.
  • Missing client-hint scenario falls back to unresolved/phantom flow as before.

Out of Scope Confirmed

Did not implement:

  • LC3 MCP tool/DTO/docs (list_clients).
  • Route-tool redesign or companion tools.
  • Broad LC1 schema extraction/persistence redesign.

Made with Cursor

HumanBean17 and others added 2 commits May 6, 2026 17:44
Switch pass6 feign hint fallback to read caller-side Client rows through DECLARES_CLIENT while preserving HTTP_CALLS semantics and match outcomes. Add focused regression coverage for feign resolution parity, find_route_callers continuity, and missing-hint fallback behavior.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add per-PR Cursor task prompts for the list_clients plan sequence, including scope guardrails, sentinel checks, test commands, and done criteria for LC1, LC2, and LC3 handoffs.

Co-authored-by: Cursor <cursoragent@cursor.com>
@HumanBean17
Copy link
Copy Markdown
Owner Author

Review: PR-LC2 — pass6 hint recovery retargets to Client declarations

Verdict: Approved ✅

Surgical, mechanical retarget: 33-line edit to one function in build_ast_graph.py, plus 4 focused regression tests and a forward-looking Cursor task-prompt doc for LC3. No semantics drift, no schema touch, no MCP surface. Match outcomes (cross_service / intra_service / ambiguous / phantom / unresolved) are preserved by design, and the source_layer propagation is now actually correct (was hardcoded layer_c_source pre-LC2; now flows from the persisted Client.source_layer).

Scope discipline (out-of-scope checks)

Sentinel Status
list_clients references ⚠️ 14 hits — all inside the new plans/CURSOR-PROMPTS-LIST-CLIENTS-MCP-TOOL.md task-prompt doc; zero in server.py / kuzu_queries.py
@mcp.tool registrations ⚠️ 3 hits — all in the same prompt doc, none in server.py
ClientRowDto / ClientsListOutput ⚠️ 3 hits each — same: prompt doc only, no DTO additions
CREATE NODE TABLE Client / CREATE REL TABLE DECLARES_CLIENT ⚠️ 1 hit each — sentinel-check examples inside the prompt doc, not real schema edits
ONTOLOGY_VERSION bump ✅ 0 hits — correctly untouched (LC2 has no schema delta)
test_list_clients ⚠️ 2 hits — both in prompt-doc test list, not actual test files

All "leakage" sentinels resolve to the new task-prompt doc — i.e. the plan for LC3, not its implementation. Real code edits are confined to:

  • build_ast_graph.py pass6 fallback block (lines 1835–1893)
  • tests/test_client_hint_recovery.py (new file, 112 lines)

Verified by inspecting each grep hit in context — every match is inside fenced text or sentinel-grep instructions for the next PR. Clean.

Plan compliance

# Plan item (PR-LC2 §) Verified
1 pass6 hint recovery resolves caller member → DECLARES_CLIENTClient build_ast_graph.py:1838-1842 builds client_hints_by_member: dict[str, list[ClientRow]] from declares_client_rows joined to clients_by_id
2 Reads path / target_service / method from Client ✅ all three fields read at :1869-1882 (client.path, client.target_service, client.method)
3 Matcher outcome semantics unchanged ✅ same RouteRow(id="", kind="http_consumer", framework="feign", ...) transient construction as v2; only the source of the data moved, not the matcher contract
4 HTTP_CALLS(Symbol → Route) edge generation/meaning unchanged ✅ no edits to the post-recovery _match_route codepath; transient src_route still feeds the existing matcher loop
5 New file tests/test_client_hint_recovery.py with 4 tests ✅ all 4 plan-mandated test names present and exact: test_pass6_uses_client_hints_for_feign_resolution, test_cross_service_match_outcome_unchanged_after_client_migration, test_find_route_callers_still_returns_expected_feign_caller, test_missing_client_hint_falls_back_to_existing_unresolved_or_phantom_flow
6 bank-chat-system (or equivalent fixture) cross-service Feign call resolves correctly ✅ test #1 uses tests/fixtures/cross_service_smoke with smoke.a.BFeignClient.joinOperator → asserts resolved row's microservice is svc-b
7 find_route_callers continuity preserved ✅ test #3 writes to a real Kuzu DB and runs g.find_route_callers(microservice="svc-b", path_template="/chat/joinOperator", method="POST"), asserts the Feign caller is in the result
8 Definition of done — pass6 no longer depends on caller http_consumer route hints ✅ the previous fallback iterated member.decl.outgoing_calls (in-memory v2 transient state); new path iterates the persisted client_rows set, completely orthogonal to whether caller-side routes ever existed

Diff anatomy (the only real change)

Old (v2 in PR-LC1's wake):

for decl in member.decl.outgoing_calls:
    if decl.client_kind != "feign_method":
        continue
    path_template, path_regex = _normalize_path(decl.path_template_call)
    src_route = RouteRow(...
        feign_name=decl.feign_target_name,
        feign_url=decl.feign_target_url,
        ...
        source_layer="layer_c_source",  # always hardcoded
    )

New (LC2):

for client in client_hints_by_member.get(member.node_id, ()):
    if client.client_kind != "feign_method":
        continue
    path_template, path_regex = _normalize_path(client.path)
    src_route = RouteRow(...
        feign_name=client.target_service,
        feign_url="",  # no URL stored on Client (kept None-equivalent)
        ...
        source_layer=client.source_layer,  # now propagates from LC1's stamping
    )

Mapping is mechanical and 1:1 except for two intentional asymmetries:

  • feign_url empty-string instead of decl.feign_target_urlClient schema doesn't carry feign URL. This is correct because the matcher uses feign_name for service resolution, not URL. Verified by reading the matcher block right after — no consumer of src_route.feign_url exists in pass6.
  • source_layer now data-driven — the previous hardcoded layer_c_source was a known correctness gap (e.g. a Feign hint from a YAML override would be misreported as source-layer). Now reads client.source_layer, which LC1's _client_source_layer whitelist guarantees is one of {layer_a_meta, layer_b_ann, layer_b_fqn, layer_c_source, builtin}. This is a quiet bug fix, not just a refactor.

Notes that earned my trust

  • Hint index built once as client_hints_by_member: dict[str, list[ClientRow]] = defaultdict(list) outside the loop. O(declares + clients) preprocessing instead of O(callers × declares × clients) lookup. Same shape as the existing route_by_id and member_by_id indexes built at the top of pass6.
  • Defensive clients_by_id.get(edge.client_id) with None skipDECLARES_CLIENT referencing a non-existent Client (shouldn't happen, but bad fixtures or partial rebuilds could trigger it) silently no-ops instead of crashing.
  • tables.declares_client_rows and tables.client_rows are read-only here — pass6 doesn't mutate the LC1 collections, only reads. Idempotency contract from the existing pass6 docstring is preserved.
  • Test Add per-PR Cursor task prompts for Tier 1 completion #4 test_missing_client_hint_falls_back_to_existing_unresolved_or_phantom_flow strips both declares_client_rows and client_rows for the target caller, then asserts match in {"unresolved", "phantom"} AND explicitly != "cross_service". The negative-case lock is the strongest signal — it'd catch a future regression where we accidentally re-introduce a caller-route fallback.
  • Test AST by Opus #1 corrupts row.route_id = "missing:route:id" AND row.match = "unresolved" before invoking pass6_match_edges. This forces the recovery path rather than incidentally exercising it; locks down that the new client_hints_by_member lookup is what produced the resolution.
  • Test Add Cursor rules and agent settings for CLI agents #3 actually writes to Kuzu and queries via KuzuGraph.find_route_callers end-to-end. The assertion c.caller_symbol_id == caller_id for microservice="svc-b", path_template="/chat/joinOperator", method="POST" proves the persisted graph round-trips correctly — not just in-memory tables.
  • Reset of KuzuGraph._instance and _instance_path in test Add Cursor rules and agent settings for CLI agents #3 prevents singleton bleed between tests. Subtle, easy to miss, correct.

Tests

Skipping local verification per your instruction. PR description claims 302 passed, 4 skipped against the post-PR-LC1 baseline of 298/4 — exactly +4 for the four new test_client_hint_recovery.py tests. Math checks out.

Observations (non-blocking)

  1. client_hints_by_member keyed by edge.symbol_id but the loop later reads client_hints_by_member.get(member.node_id, ()). This works iff Symbol.id == Member.node_id — which is true today (the Symbol table is populated from members rows in pass2 with id = member.node_id). One-line comment near the hint-index construction noting this invariant would help a future reader; otherwise it looks like a happy coincidence. Not a blocker — documented in plan §132 already.
  2. Multiple Client declarations per member: client_hints_by_member[m] is a list[ClientRow], and the loop uses break after the first client_kind == "feign_method" match. Today, an interface method has at most one Feign declaration so this is fine. If a future PR allows multiple @CodebaseClient annotations on a single method (e.g. one per HTTP verb), the break would silently pick the first by insertion order. A # TODO: revisit when multi-declaration support lands would help.
  3. feign_url="" (empty string) instead of feign_url=None or removing the field from the synthesized hint — minor; matches existing RouteRow field defaults. No consumer cares today, but if a future matcher branch starts treating empty string differently from None this could surprise. Defensible because it mirrors the v2 behaviour for non-Feign hints.
  4. Same fixture tests/fixtures/cross_service_smoke for all 4 tests; great for fast feedback but means one fixture-level breakage (e.g. the BFeignClient interface getting renamed) takes the whole suite down. Trade-off, not a defect.
  5. Cursor task-prompt doc plans/CURSOR-PROMPTS-LIST-CLIENTS-MCP-TOOL.md is shipped alongside the implementation — useful, but this is technically out-of-scope for an "LC2 pass6 retarget" PR. No harm done; just flagging that future PR-X bodies can stay leaner if the task-prompt doc lands separately. (No need to revert here.)

Plan deltas needed

None. PR-LC2 done-criteria are all met:

  • ✅ pass6 no longer depends on caller http_consumer route hints
  • ✅ Regression behavior stable for HTTP_CALLS and find_route_callers
  • ✅ Targeted pass6 regression tests pass (per PR description)
  • ✅ Full suite green (302/4)

Ready to merge. Next: PR-LC3list_clients MCP tool, query helper, DTOs, and README docs. The Cursor task-prompt for it is already drafted in plans/CURSOR-PROMPTS-LIST-CLIENTS-MCP-TOOL.md (this PR), so the next step is straightforward delegation.

Clarify the Symbol/member id invariant used by DECLARES_CLIENT lookups, make multi-client feign fallback deterministic by sorting per-member hints, and document why feign_url remains empty in the synthesized pass6 hint route.

Co-authored-by: Cursor <cursoragent@cursor.com>
@HumanBean17 HumanBean17 merged commit 90f84cf into master May 6, 2026
@HumanBean17 HumanBean17 deleted the feat/list-clients-lc2-pass6-client-hints branch May 10, 2026 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant