Skip to content

add Client graph node and DECLARES_CLIENT edge (PR-LC1)#40

Merged
HumanBean17 merged 2 commits into
masterfrom
feat/list-clients-mcp-tool-PR-LC1
May 6, 2026
Merged

add Client graph node and DECLARES_CLIENT edge (PR-LC1)#40
HumanBean17 merged 2 commits into
masterfrom
feat/list-clients-mcp-tool-PR-LC1

Conversation

@HumanBean17
Copy link
Copy Markdown
Owner

Scope

Implements PR-LC1 from plans/PLAN-LIST-CLIENTS-MCP-TOOL.md.

This PR introduces first-class outbound client declarations in the graph layer by adding:

  • Client node table
  • DECLARES_CLIENT (Symbol -> Client) relation
  • deterministic Client.id generation
  • graph-meta counters for client extraction
  • ontology bump from 9 to 10
  • LC1 extraction/schema tests

Why

After the brownfield annotations v2 split, outbound declarations must be represented independently from inbound Route rows. This PR establishes the persistence/model foundation needed for follow-up work (pass6 hint migration in LC2 and MCP list_clients surface in LC3).

Changes

  • Graph schema and persistence (build_ast_graph.py)

    • Added Client node DDL and DECLARES_CLIENT relation DDL.
    • Wired create/drop lifecycle for new tables.
    • Added row models and GraphTables collections for client rows and declaration edges.
    • Emitted client rows from pass5 using composed resolve_http_client_for_method(...) output.
    • Stamped source_layer based on winning strategy (layer_a_meta, layer_b_ann, layer_b_fqn, layer_c_source, otherwise builtin).
    • Added deterministic client ids (c:<sha1[:16]>) from (microservice, member_fqn, client_kind, path, method).
    • Persisted GraphMeta client counters and by-kind JSON map.
  • Ontology/versioning (ast_java.py)

    • Bumped ONTOLOGY_VERSION to 10.
  • Schema assertions (tests/test_ast_graph_build.py)

    • Updated expected tables to include Client and DECLARES_CLIENT.
  • LC1 tests (tests/test_client_node_extraction.py)

    • Added six tests for:
      • annotation-driven client rows
      • synthesized Feign method rows
      • DECLARES_CLIENT target linkage
      • deterministic id stability across rebuilds
      • source-layer winner behavior
      • schema/meta persistence and queryability
  • Docs (README.md)

    • Added ontology v10 reindex callout for Client/DECLARES_CLIENT and graph meta counters.
    • Updated ontology glossary mention to v10.

Reindex / Compatibility

  • Re-index required: yes (schema + ontology change).
  • Ontology version changed: 9 -> 10.

Validation

Lint

  • ruff check .
    • All checks passed!

Tests

  • pytest tests/test_client_node_extraction.py -q
    • 6 passed
  • pytest tests/test_ast_graph_build.py::test_schema_has_all_expected_tables -q
    • 1 passed
  • pytest tests -v
    • 298 passed, 4 skipped in 249.66s (0:04:09)

Follow-ups

  • LC2: retarget pass6 hint recovery from transient route hints to persisted Client rows.
  • LC3: add list_clients MCP tool and query helper/docs.

Made with Cursor

Introduce first-class outbound Client nodes with DECLARES_CLIENT edges, persist client counters in GraphMeta, and bump ontology_version to 10 so pass5 outputs and metadata stay deterministic for list_clients follow-up work.

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

Review: PR-LC1 — Client node + DECLARES_CLIENT edge

Verdict: Approved ✅

Faithful, narrowly-scoped first slice of the list_clients plan. Schema, deterministic id, source-layer stamping, ontology bump, README callout, and all six plan-mandated tests are present and shaped exactly as plans/PLAN-LIST-CLIENTS-MCP-TOOL.md specifies. Zero LC2/LC3 leakage. Clean approve.

Scope discipline (out-of-scope checks)

Sentinel Status
list_clients (LC3 tool name) ✅ 0 hits
@mcp.tool registration ✅ 0 hits
ClientRowDto / ClientsListOutput ✅ 0 hits
find_route_callers regression coverage (LC2 territory) ✅ 0 hits
test_pass6_* (LC2 test bucket) ✅ 0 hits
pass6 references ✅ 2 hits — both are fixture-builder import + invocation, no pass6 logic changes
test_list_clients ✅ 0 hits

LC2 (pass6 hint retarget) and LC3 (MCP surface + DTOs + query helper) territory is untouched. This is exactly the "schema + extraction + persistence" scope the plan calls out for LC1.

Plan compliance

# Plan item Verified
1 Client node DDL with all 16 fields build_ast_graph.py:218-225 — every field in plan §1 present, PRIMARY KEY(id)
2 DECLARES_CLIENT(FROM Symbol TO Client, confidence, strategy) build_ast_graph.py:234-237
3 Create/drop lifecycle wired DROP TABLE IF EXISTS Client and DECLARES_CLIENT present in _drop_all
4 Row dataclasses + GraphTables collections ClientRow, DeclaresClientRow dataclasses + client_rows, declares_client_rows on tables
5 Graph-meta counters using STRING-JSON pattern clients_total INT64, declares_client_total INT64, clients_by_kind STRING (json.dumps(clients_by_kind)) — matches Tier1 MAP-as-STRING convention
6 ast_java.py extraction returns outbound payloads call.client_kind, call.method_fqn, call.feign_target_name, etc. consumed in pass5 emitter
7 Feign method synthesis without explicit @CodebaseClient ✅ pass5 branch keys off call.client_kind == "feign_method" (line 185), uses call.feign_target_name for target_service
8 resolve_http_client_for_method composition feeds Client rows ✅ via call.resolution_strategy_client_source_layer
9 source_layer stamped from winning strategy _client_source_layer whitelist {layer_a_meta, layer_b_ann, layer_b_fqn, layer_c_source}, fallback builtin
10 Ontology bump 9 → 10 ast_java.py:ONTOLOGY_VERSION = 10 + Phase 8 comment line
11 All six plan-mandated tests ✅ all present with exact names: test_client_rows_emitted_for_codebase_client_annotations, test_client_rows_synthesized_for_feign_methods, test_declares_client_edge_targets_client_id, test_client_id_is_deterministic_across_rebuilds, test_client_source_layer_reflects_winning_override_layer, test_client_schema_persisted_and_queryable
12 README ontology v10 reindex callout ✅ new bullet at README.md:120-122; glossary line v9 → v10 at README.md:19

Deterministic id contract

def _client_id(*, microservice, member_fqn, client_kind, path, method) -> str:
    key = f"{microservice}|{member_fqn}|{client_kind}|{path}|{method}"
    return f"c:{hashlib.sha1(key.encode()).hexdigest()[:16]}"

Five-tuple matches the propose verbatim. c: prefix is symmetric to the existing r: Route id convention. test_client_id_is_deterministic_across_rebuilds test-locks this. ✅

Source-layer stamping correctness

The _client_source_layer test (test_client_source_layer_reflects_winning_override_layer) is the highest-signal test in the suite — it sets up a YAML http_client_overrides block and a conflicting @CodebaseClient annotation in source, then asserts the YAML wins (layer_b_fqn) and overrides the source path/target. This is the right direction to test (YAML over source); it confirms the resolver-composition output is what's persisted, not just whichever row was emitted first.

Notes that earned my trust

  • Dedup keys for DECLARES_CLIENT: dkey = (member.node_id, cid) with declares_client_seen set — prevents a single member declaring the same Client row twice (two annotations producing identical _client_id). Defensive and cheap.
  • source_layer whitelist with explicit builtin fallback rather than passing through call.resolution_strategy raw — guards against future strategy strings leaking into the persisted enum.
  • clients_by_kind sorted before json.dumps — preserves determinism in the meta blob across rebuilds.
  • Feign synthesis reuses call.feign_target_name instead of re-deriving from interface annotation — single source of truth, no parallel resolver.
  • member_id linkage stored on Client row directly (not just member_fqn) — gives LC2's pass6 query a fast indexable path without needing a Symbol-side join.

Tests

Skipping local test verification per your instruction. PR description claims 298 passed, 4 skipped — that's +8 over the post-PR-#38 baseline of 290/4, which exactly matches the six new test_client_node_extraction.py tests plus the two new schema-table assertions (Client, DECLARES_CLIENT) added to test_ast_graph_build.py::test_schema_has_all_expected_tables. Math checks out.

Observations (non-blocking)

  1. _CREATE_CLIENT Cypher uses positional-style $param template strings, but the param map in _write_kuzu is built dynamically with all 16 keys hand-listed. Consistent with how _CREATE_ROUTE is handled today, but a future helper that derives the param dict from the dataclass via asdict() would prevent drift if the schema grows. Not a blocker — same pattern as Route.
  2. path_template and path_regex are populated for Client rows but not yet consumed by anything in this PR — they're forward-compatibility for path-prefix filtering in LC3. Worth a one-line code comment so a future reader doesn't assume they're dead fields. Defer to LC3 if it adds the filter.
  3. microservice STRING for Client.target_service — plan resolved this as "keep STRING for v1" (line 50), and the implementation matches. No action; calling it out so the decision stays visible.
  4. builtin source_layer label — when none of the four tracked layers wins, the row is stamped builtin. There's a small risk this masks a 5th unforeseen strategy in the future. A # pragma: no cover or explicit assertion path could help; today the test suite covers all four expected branches but not the fallthrough.
  5. Phase 8 comment in ast_java.py says "first-class Client node + DECLARES_CLIENT relation" — accurate, but doesn't mention "ontology v10 also reserves outbound declarations independently from Route". Minor wording polish, not blocking.

Plan deltas needed

None. LC1 done-criteria are all met:

  • Client and DECLARES_CLIENT tables exist and are populated
  • ✅ Ontology version bumped 9 → 10 and reflected in metadata
  • ✅ Deterministic id contract implemented and test-locked
  • ✅ Full tests green (per PR description)

Ready to merge. Next: PR-LC2 — pass6 hint recovery retarget from transient route hints to persisted Client rows. The member_id field stamped on Client here will give LC2 the indexable join key it needs.

Use dataclass serialization for Client inserts, add defensive unknown strategy logging, and clarify outbound Client path/source-layer intent in comments to keep LC1 persistence easier to maintain.

Co-authored-by: Cursor <cursoragent@cursor.com>
@HumanBean17 HumanBean17 merged commit eb34b05 into master May 6, 2026
@HumanBean17 HumanBean17 deleted the feat/list-clients-mcp-tool-PR-LC1 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