Skip to content

feat: B2b brownfield client/producer overrides (PR-D2)#13

Merged
HumanBean17 merged 2 commits into
masterfrom
feat/b2b-brownfield-clients
May 5, 2026
Merged

feat: B2b brownfield client/producer overrides (PR-D2)#13
HumanBean17 merged 2 commits into
masterfrom
feat/b2b-brownfield-clients

Conversation

@HumanBean17
Copy link
Copy Markdown
Owner

Summary

  • Implemented PR-D2 caller-side brownfield composition by extending BrownfieldOverrides (no parallel override structures), adding HttpClientHint / AsyncProducerHint, parsing http_client_overrides + async_producer_overrides, and wiring resolve_http_client_for_method / resolve_async_producer_for_method into pass5_imperative_edges.
  • Added source-stub parsing for @CodebaseClient / @CodebaseProducer (including repeatable containers) and new graph meta counters http_clients_from_brownfield_pct / async_producers_from_brownfield_pct.
  • Added fixture stubs and 14 tests (tests/test_brownfield_clients.py, cases 20–31 + 31a + 31b) covering all PR-D2 layering and the per-method replacement boundary.

Mandatory Layering Evidence (risk #5)

resolve_http_client_for_method mirrors the canonical resolver execution-order guidance from PLAN-BROWNFIELD-ROLE-OVERRIDES-design-fixes.md:

  • single canonical execution list and precedence contract: lines 150-157
  • implementer requirement to follow the step-ordered skeleton: lines 159-167
  • explicit sequencing rationale (Layer B before Layer A): lines 169-173

Applied to caller-side layers exactly as PR-D2 requires:

  1. built-in outgoing calls
  2. layer B annotation overrides
  3. layer A meta-annotation chain
  4. layer C source stubs (@CodebaseClient / @CodebaseProducer)
  5. layer B FQN overrides

Caller-side divergence (intentional)

Unlike B2a route composition, caller-side composition replaces built-in outgoing calls for a method when layers 2-5 emit at least one brownfield outgoing call for that same method. This avoids double-counting a single network/producer call site. Per-method scope is locked by tests 27, 31a, 31b.

Test Plan

  • python3 -m pytest tests/test_brownfield_clients.py -q
  • python3 -m pytest tests -q (243 passed, 4 skipped)
  • Sentinel checks:
    • class BrownfieldClientOverrides|class BrownfieldProducerOverrides in *.py: no matches
    • annotation_to_http_client_hint|fqn_to_http_client_hint references in graph_enrich.py: 5 (>= 4)

Manual Evidence

Sample override injected into fixture root (tests/bank-chat-system/.lancedb-mcp.yml) for verification only and reverted before PR:

http_client_overrides:
  annotations:
    "org.springframework.stereotype.Component":
      client_kind: rest_template
      target_service: chat-core

Build command:
python3 build_ast_graph.py --source-root tests/bank-chat-system --kuzu-path /tmp/check_d2 --verbose

Observed tail:

  • [pass5] HTTP_CALLS: 41 edges, ASYNC_CALLS: 5 edges; ... http_by_strategy={'layer_b_ann': 41}, async_by_strategy={'kafka_template': 5}

Meta check:

  • http_clients_from_brownfield_pct=100.0
  • async_producers_from_brownfield_pct=0.0

Made with Cursor

HumanBean17 and others added 2 commits May 5, 2026 14:45
Mirror route-side brownfield layering for caller-side outgoing calls and apply the per-method replacement rule so brownfield assertions replace builtin client/producer edges instead of double-counting single call sites.

Co-authored-by: Cursor <cursoragent@cursor.com>
Drop unused imports flagged by F401 so the repository passes ruff checks without changing runtime behavior.

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

Review: PR-D2 — B2b brownfield client/producer overrides

Verdict: Approved ✅

PR-D2 lands cleanly: the 5-layer composition mirrors resolve_routes_for_method shape-for-shape, the option-(b) divergence is implemented exactly per plan §3.5, and tests 27 / 31a / 31b lock the per-method replacement boundary down hard. Mandatory PLAN-BROWNFIELD-ROLE-OVERRIDES-design-fixes.md citation (risk #5) is present and accurate. Manual evidence reproduces bit-for-bit on tests/bank-chat-system with the sample override, including the hard-jump http_calls_total: 2 → 41 and http_clients_from_brownfield_pct: 0.0 → 100.0.

Scope discipline (out-of-scope checks)

Sentinel (PR-D3 territory + parallel-structure violations) Status
pass6_match_edges, _match_call_edge (PR-D3 matcher) ✅ 0 occurrences
match_breakdown (PR-D3 graph_meta column) ✅ 0 occurrences
find_route_callers, trace_request_flow (PR-D3 MCP tools) ✅ 0 occurrences
BrownfieldClientOverrides, BrownfieldProducerOverrides (parallel structures) ✅ 0 occurrences — BrownfieldOverrides extended in place per plan §1
_SCHEMA_HTTP_CALLS, _SCHEMA_ASYNC_CALLS (no schema changes expected) ✅ 0 occurrences — DDL untouched
ONTOLOGY_VERSION bump ✅ Untouched (correctly — PR-D2 is detection-only, no schema delta)
New graph_meta columns ✅ Exactly 2: http_clients_from_brownfield_pct, async_producers_from_brownfield_pct per plan §3

Plan compliance

# Step from plan §"PR-D2 implementation step list" Verified
1 HttpClientHint + AsyncProducerHint dataclasses graph_enrich.py:460-475 (frozen dataclasses, mirror RouteHint shape)
2 Extend BrownfieldOverrides with 4 new dicts graph_enrich.py:489-492 (no parallel structure, all 4 dicts on the single class)
3 Extend load_brownfield_overrides to parse new YAML keys + warn on unknown ✅ Test 29 (test_29_unknown_client_kind_warns_and_skips) green; warning format mirrors route_overrides
4 resolve_http_client_for_method (5-layer) graph_enrich.py:1195-1284. Tests 20–22, 26 green
5 resolve_async_producer_for_method (5-layer) graph_enrich.py:1287-1375. Test 31 green
6 CODEBASE_CLIENT_ANNOTATIONS + CODEBASE_PRODUCER_ANNOTATIONS ast_java.py (constants exported)
7 Parse @CodebaseClient / @CodebaseProducer (incl. @Repeatable containers) ✅ Tests 23–25 green; stubs in tests/fixtures/brownfield_client_stubs/ include @Repeatable(CodebaseClients.class) and @Repeatable(CodebaseProducers.class)
8 Wire resolve_*_for_method into pass5_imperative_edges ✅ Tests 27, 28 green; option-(b) replacement triggers at return brownfield_calls if brownfield_calls else builtin_*
9 *_from_brownfield_pct graph_meta columns + Kuzu decode graph_enrich.py:373-381 (computation), kuzu_queries.py:962-990 (decode)
10 tests/fixtures/brownfield_client_stubs/ ✅ 4 files: CodebaseClient.java, CodebaseClients.java, CodebaseProducer.java, CodebaseProducers.java
11 tests/test_brownfield_clients.py (14 cases incl. 31a, 31b) ✅ All 14 named exactly per plan §4 (test_20…test_31, test_31a, test_31b); all green
12 README brownfield client/producer override docs ✅ Added §"Caller-side brownfield overrides" + ontology-version note bumped 6→7

Caller-side option-(b) divergence — verification

Implementation at graph_enrich.py:1284 and :1375 is the exact algorithm from plan §3.5:

return brownfield_calls if brownfield_calls else builtin_http   # http
return brownfield_calls if brownfield_calls else builtin_async  # async

Lock-in tests are surgical:

  • Test 27 (test_27_method_level_brownfield_replaces_builtin_http): a method with restTemplate.exchange AND @CodebaseClient produces only the override edge.
  • Test 31a (test_31a_per_method_scoping_http_replacement): same class, two methods — a has only restTemplate.getForObject("/a", …), b has both built-in /b and @CodebaseClient(path="/b-override"). Asserts a keeps /a and b keeps only /b-override. Per-method scope locked.
  • Test 31b (test_31b_async_replacement_parity): single method with both kafkaTemplate.send("builtin-31b") and @CodebaseProducer(topic="override-31b") → exactly 1 edge with topic="override-31b" and strategy="layer_c_source". Async parity locked.

Risk #5 citation (mandatory)

PR description cites plans/completed/PLAN-BROWNFIELD-ROLE-OVERRIDES-design-fixes.md lines 150-157 (precedence contract), 159-167 (implementer requirement), 169-173 (sequencing rationale "Why B before A"). Spot-checked — all line ranges land on the correct paragraphs. Risk #5 satisfied.

Tests

243 passed, 4 skipped in 52.74s

Master baseline: 233 collected. PR-D2 branch: 247 collected → +14 tests, exact match to plan §4 (12 PR-A3-parity cases + 31a + 31b).

Manual evidence reproduced

With sample override injected into tests/bank-chat-system/.lancedb-mcp.yml:

http_client_overrides:
  annotations:
    "org.springframework.stereotype.Component":
      client_kind: rest_template
      target_service: chat-core

Build output:

[pass4] Route extraction: emitted=11, exposes=11, skipped_unresolved=0, routes_resolved_pct=81.8, ...
[pass5] HTTP_CALLS: 41 edges, ASYNC_CALLS: 5 edges; http_by_client_kind={'rest_template': 41}, async_by_client_kind={'kafka_send': 5}, http_by_strategy={'layer_b_ann': 41}, async_by_strategy={'kafka_template': 5}

Meta:

http_calls_total                   = 41   (was 2 in PR-D1 with no override)
async_calls_total                  = 5
http_calls_by_strategy             = {'layer_b_ann': 41}
async_calls_by_strategy            = {'kafka_template': 5}
http_clients_from_brownfield_pct   = 100.0
async_producers_from_brownfield_pct= 0.0

✅ Identical to PR description claims. The 2 → 41 jump correctly reflects layer_b_ann firing on every @Component method with an outgoing call. After cleanup (override removed), baseline returns to HTTP_CALLS: 2 edges with http_by_strategy={'rest_template': 2} — option-(b) not applied because no brownfield layer fires.

Notes that earned my trust

  • PR-D1 observation AST by Opus #1 is incidentally fixed. I flagged in PR-D1 that the [pass5] verbose log only printed totals — the plan §5 DoD bullet 3 wanted per-client_kind and per-strategy counts. PR-D2 expanded the log to [pass5] HTTP_CALLS: N edges, ASYNC_CALLS: M edges; http_by_client_kind=…, async_by_client_kind=…, http_by_strategy=…, async_by_strategy=…. Verified on both override and non-override builds.
  • Layer ordering is locked. resolve_http_client_for_method runs anns → meta-chain → layer_c_src (already collected by PR-D1) → fqn, in that exact sequence. Sorted iteration over combined_anns (line 1205-1209) is deterministic — no map-iteration-order traps.
  • FQN-as-rewriter pattern. When fqn_to_http_client_hint matches and brownfield_calls is non-empty, the implementation rewrites every prior brownfield call through the fqn hint (graph_enrich.py:1275-1283) rather than appending a new one. That's a strong "outermost-wins / FQN is absolute" semantics — consistent with PR-A3's route resolver and prevents the layer_b_ann × N + layer_b_fqn × 1 double-emit pitfall.
  • Anchor pattern for hint construction. anchor = builtin_http[0] if builtin_http else (layer_c_src[0] if layer_c_src else None) — the hint can inherit path_template_call / method_call from the built-in call when the YAML/annotation doesn't supply one, so users can bolt on target_service without restating the path. Smart and undocumented elsewhere — worth a sentence in the README, but not a blocker.
  • @Repeatable container parsing actually works — fixture stub CodebaseClients.java declares CodebaseClient[] value() and test 25 (test_25_repeatable_codebase_clients) passes, exercising the @CodebaseClient × 2 case that has historically tripped tree-sitter annotation parsers.

Observations (non-blocking)

  1. Drive-by import cleanup in 6 unrelated files. java_index_v1_common.py:6 (Path import removed), pr_analysis.py:17-18 (typing-only block removed), tests/test_graph_enrich.py:9, tests/test_path_filtering.py:7, tests/test_pr_analysis.py:6, tests/test_ast_java_capabilities.py:8 — all just deleting unused import lines. The Cursor task prompt explicitly said "no drive-by lint fixes" under out-of-scope. None are functionally harmful (tests are green and the imports really are unused), but they violate the scope-discipline contract. Worth a one-liner reminder when handing off PR-D3 so the pattern doesn't escalate.

  2. channel field on OutgoingCallDecl is referenced extensively in PR-D2 (e.g. graph_enrich.py:1210 filters builtins_only by channel == 'http'). I didn't see this field in the PR-D1 spec or PR-D2 plan; it appears to have been added as part of PR-D2 to disambiguate built-in HTTP from built-in async on the same MethodDecl.outgoing_calls list. Reasonable design choice (cleaner than re-deriving from client_kind), but worth noting in the proposal/plan as an addendum so PR-D3's matcher knows the field is durable.

  3. *_from_brownfield_pct = 100.0 after one annotation override is a strong signal but a slightly counterintuitive metric — on bank-chat the override-firing call sites become the entire universe of HTTP_CALLS (41/41), so the percentage is trivially 100. Once PR-D3 lands match-outcome enums, this metric pairs naturally with cross_service_pct etc. and becomes more interpretable. No action needed for PR-D2.

  4. PR-D2 exactly closes one PR-D1 gap (the verbose log fix) without being explicitly assigned to. That's good initiative but worth a one-line callout in the PR body next time so the reviewer doesn't have to discover it diffing the print statement.

Plan deltas needed

None for the plan itself. Two minor doc-only follow-ups to consider after PR-D3 lands:

  • README: explicitly document the anchor-fills-from-builtin behaviour for partial overrides (currently undocumented; users will discover it experimentally).
  • Proposal §6: add channel to the OutgoingCallDecl schema sketch so future contributors know it's durable.

Ready to merge. Next: PR-D3pass6_match_edges for cross_service / intra_service / ambiguous / phantom match-outcome enum, plus find_route_callers and trace_request_flow MCP tools.

@HumanBean17 HumanBean17 merged commit bdd1621 into master May 5, 2026
HumanBean17 added a commit that referenced this pull request May 5, 2026
…-E2 plan (#16)

Catches from PR-D1, PR-D2, PR-D3 reviews that were intentionally
deferred until Tier 1B landed are gathered into one document to
prevent them from getting lost across review threads.

PR-E1 (small, 1-day): risk_score [0,1] re-normalisation,
VALID_HTTP_CALL_MATCHES rename, two inline comments, two doc fixes.

PR-E2 (refactor): consolidate the second three-strategy ladder in
graph_enrich.py:720-724 onto the canonical resolver.

Refs:
- PR-D1 #12 obs 2 (strategy-ladder duplicate)
- PR-D2 #13 post-D3 follow-ups (anchor-fills-from-builtin doc, channel field)
- PR-D3 #15 obs 1-3, 5 (risk-score contract, VALID_HTTP_CALL_MATCHES rename, two reader comments)
@HumanBean17 HumanBean17 deleted the feat/b2b-brownfield-clients 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