Skip to content

migrate brownfield annotations to v2 shape#38

Merged
HumanBean17 merged 2 commits into
masterfrom
feat/brownfield-annotations-v2
May 6, 2026
Merged

migrate brownfield annotations to v2 shape#38
HumanBean17 merged 2 commits into
masterfrom
feat/brownfield-annotations-v2

Conversation

@HumanBean17
Copy link
Copy Markdown
Owner

Summary

  • migrate brownfield route annotations to v2 (CodebaseHttpRoute/CodebaseAsyncRoute) and add enum-typed outbound stubs for client/producer kinds
  • update AST extraction and pass6 Feign recovery so Feign declarations stay outbound clients while call-edge matching still resolves to inbound endpoints
  • refresh docs and tests to remove v1 CodebaseRoute* assumptions and validate the new direction-honest behavior

Test plan

  • pytest tests/test_brownfield_routes.py tests/test_brownfield_clients.py tests/test_cross_service_resolution_flag.py -q --tb=line
  • ruff check .
  • pytest tests -q --tb=line

Made with Cursor

Align brownfield route/client/producer annotations with the direction-honest v2 design, move Feign handling to outbound client extraction, and update docs/tests/fixtures to remove v1 Route annotation assumptions.

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

Review: PR #38 — migrate brownfield annotations to v2 shape

Verdict: Approved ✅

Faithful, conservative implementation of the v2 propose. All 8 acceptance criteria met. The Feign-as-outbound migration — the riskiest part — is implemented with minimal blast radius (transient RouteRow synthesis in pass6 instead of refactoring the matcher's signature). Tests stay green at 290/4 baseline; bank-chat-system manual evidence reproduces and confirms zero Feign rows in the Route table. Two non-blocking observations on doc/structure choices.

Scope discipline (out-of-scope checks)

Sentinel Status
ONTOLOGY_VERSION bump ✅ 0 hits — schema unchanged, correctly in scope
CREATE NODE TABLE / CREATE REL TABLE ✅ 0 hits — no schema additions
New @mcp.tool registrations ✅ 0 hits — tool surface unchanged
Client node / DECLARES_CLIENT / list_clients ✅ 0 hits — PR #37 territory not crossed
+ lines mentioning CodebaseRoute (v1) ✅ 1 hit — and it's the v1-detection stderr-warning per propose criterion #4
Producer projection ✅ 0 hits — async outbound parallel correctly punted

Scope is clean — this is a pure annotation-shape migration without schema or new-tool work, exactly as the propose specified.

Plan compliance — propose acceptance criteria

# Criterion from propose Verified
1 v2 stub source files exist + parse cleanly ✅ 6 stubs in tests/fixtures/brownfield_*_stubs/com/example/rag/ (HttpRoute, AsyncRoute, Client, ClientKind, Producer, ProducerKind + Routes/Clients/Producers containers); fields match propose exactly
2 graph_enrich recognises v2 simple names ✅ verified via test_brownfield_routes.py + test_brownfield_clients.py
3 Feign methods NOT emitted as Route nodes ast_java.py:2167if feign_iface: continue skips Route emission for Feign interfaces; tests/test_feign_not_exposer.py::test_feign_route_node_is_not_emitted asserts this explicitly
4 v1 simple names not recognised + stderr warning fires graph_enrich.py:1077-1082 emits [lancedb-mcp] v1 brownfield annotation detected; migrate to CodebaseHttpRoute / CodebaseAsyncRoute / CodebaseClient — message is verbatim per propose; reproduced manually with a @CodebaseRoute(...) fixture
5 Compile-time enum typing enforced ⚠️ Stubs are RetentionPolicy.SOURCE enums — typing is enforced at user-codebase javac time. No integration test pastes a clientKind = "rest_template" (string literal) and verifies compile-fail. Non-blocking — could be a follow-up test.
6 All docs reference only v2 names ✅ Zero @CodebaseRoute / CodebaseRouteKind / CodebaseRouteFrameworkKind in README, CODEBASE_REQUIREMENTS, AGENT-GUIDE, MANUAL-VERIFICATION-CHECKLIST. v2 names present in all four.
7 Test baseline holds ✅ 290 passed, 4 skipped — exact same baseline as master @ 83bf89e
8 Manual verification on bank-chat-system ✅ Reproduced — see below

Tests

290 passed, 4 skipped in 54.87s

Identical to the master baseline. test_brownfield_routes.py and test_brownfield_clients.py both rewritten to use the v2 shape; test_feign_not_exposer.py extended with three new assertions covering the new Feign-as-outbound flow.

Manual evidence reproduced

$ python build_ast_graph.py --source-root tests/bank-chat-system --kuzu-path /tmp/check_kuzu_v2 --verbose
[pass1] parsed 84 files in 0.25s: 92 types, 474 members, 0 parse errors
[pass4] Route extraction: emitted=11, exposes=11, exposes_suppressed_feign=0,
        skipped_unresolved=0, routes_resolved_pct=81.8, routes_from_brownfield_pct=0.0,
        by_framework={'spring_mvc': 9, 'kafka': 2}
[pass5] HTTP_CALLS: 2 edges, ASYNC_CALLS: 5 edges
[pass6] http_match={'phantom': 2}, async_match={'intra_service': 1, 'phantom': 4}
  • Pass4 emitted-routes count drops from 17 (pre-v2) → 11 (post-v2) — the 6 Feign http_consumer rows are gone, exactly as the propose intends.
  • by_framework={'spring_mvc': 9, 'kafka': 2} — no feign framework anywhere. ✅
  • Direct query of the built graph: [r for r in routes if r.kind=='http_consumer' or r.framework=='feign'] returns 0 rows. ✅
  • HTTP_CALLS / ASYNC_CALLS counts unchanged (2 / 5) — Feign caller resolution still works through the new @CodebaseClient(clientKind=feign_method) hint path.
  • v1 stderr warning reproduced manually with a @com.example.rag.CodebaseRoute(...) fixture: [lancedb-mcp] v1 brownfield annotation detected; migrate to CodebaseHttpRoute / CodebaseAsyncRoute / CodebaseClient. ✅

Notes that earned my trust

  1. Pass6 Feign hint recovery (build_ast_graph.py:1742-1767) is the right call. Synthesizes a transient RouteRow from member.decl.outgoing_calls (the new @CodebaseClient data path) when route_id is None. The synthetic RouteRow has id="" and never gets persisted — it just lets the rest of the matcher's downstream logic stay untouched. Minimal blast radius, faithful to the propose's "same data, new home" principle.
  2. ast_java.py:2166-2167if feign_iface: continue in the Route-emission loop is the cleanest possible cut: Feign interface methods take the OutgoingCallDecl path with client_kind="feign_method" (line 1757), never the Route path. No Route row, no EXPOSES edge, no double-counting.
  3. Defensive exposes_suppressed_feign counter retained at build_ast_graph.py:1361-1362. Even though Feign methods now skip the route-emission loop entirely, if a stale http_consumer row ever lands (e.g. via brownfield YAML overrides with kind: http_consumer set explicitly), the EXPOSES suppression still fires. Belt-and-braces.
  4. VALID_ROUTE_FRAMEWORKS correctly narrowed in java_ontology.py:27-30 to {spring_mvc, webflux} only. The transport-family entries (kafka, rabbitmq, etc.) move out — they're now extractor-internal, populated from listener-annotation context, not user input. Matches the propose's "framework is internal" stance.
  5. VALID_PRODUCER_KINDS added as a separate frozenset for the renamed producerKind field — clean separation from VALID_CLIENT_KINDS (now HTTP-only).
  6. YAML override schema correctly preserved. http_client_overrides.client_kind: kafka_send still appears in async_producer_overrides examples — out of scope per propose, untouched in this PR. Good discipline.

Observations (non-blocking)

  1. Stub directory naming kept as v1. Propose suggested renaming brownfield_route_stubs/brownfield_http_route_stubs/ and adding a new brownfield_async_route_stubs/. Implementation kept the single brownfield_route_stubs/ directory containing both CodebaseHttpRoute.java and CodebaseAsyncRoute.java. This is fine — simple-name matching doesn't care about directory layout, and one directory containing all Route stubs reads cleanly. Mild deviation worth flagging but no behavioural impact.

  2. Explicit "Direction matters" inbound-vs-outbound table missing from README. Propose called for "a fresh 'Direction matters' inbound-vs-outbound table that explicitly names Feign as outbound." The §3b/§3c rewrites do say "method-level inbound annotations" (line 402) and "outbound HTTP call sites" (line 456), so direction is conveyed inline — but a single comparison table would be the clearest form for the AMA agent. Suggest adding in a follow-up doc PR (alongside AGENT-GUIDE updates if the slash aliases or Recovery playbook mention Feign explicitly).

  3. No clientKind="string" integration test for compile-time enum-typing enforcement (acceptance criterion PR-A1: Route schema + literal extractor (B2a) #5). The stub-source enum types do enforce this at user-codebase compile time, but a dedicated regression test (paste a @CodebaseClient(clientKind = "rest_template") literal, verify it's classified as a string-literal annotation value rather than an enum reference) would make the criterion testable inside CI. Worth adding when the natural opportunity arises.

  4. Pass4 http_consumer defensive references survive. build_ast_graph.py:1291,1361,1773 and graph_enrich.py:821 still mention http_consumer as a valid kind. After this PR no greenfield path emits one, but the constants stay defensible (brownfield YAML can still set kind: http_consumer on route_overrides.fqn, and the transient hint-recovery row uses it). No action needed; flagging for future cleanup if/when the YAML override schema also drops http_consumer as a valid kind.

  5. Comment at build_ast_graph.py:1768-1770 is slightly stale. Says "Declared Feign client methods use http_consumer routes" — true historically, but post-v2 Feign methods don't have a persisted http_consumer route, only a transient one synthesized in pass6. Worth a one-line update to mention the synthesis. Minor doc-polish, not a defect.

Plan deltas needed

None. The propose is implemented as written. The Out-of-Scope items (no Client node, no list_clients tool, no Producer projection, no YAML schema changes, no Maven coordinate) are all respected.


Ready to merge. Next: PR #37 (list_clients propose) becomes implementable once this lands. Suggest queueing a planning doc for the Client node + tool work as the natural follow-up.

Add the directionality table to README, clarify pass6 Feign synthesis wording, and enforce enum-only parsing for client/producer kind annotations with regression tests for string-literal misuse.

Co-authored-by: Cursor <cursoragent@cursor.com>
@HumanBean17 HumanBean17 merged commit e3242ed into master May 6, 2026
@HumanBean17 HumanBean17 deleted the feat/brownfield-annotations-v2 branch May 10, 2026 21:18
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