propose: v2 brownfield annotations — split @CodebaseRoute, enum-typed kinds#36
Conversation
… kinds Direct outcome of feedback during the first real-project rollout: - User reached for @CodebaseRoute(framework=kafka, …) for an outbound Kafka producer (right concept, wrong annotation). The asymmetry of the v1 set (one inbound annotation across HTTP+async, two outbound annotations split by transport) was the proximate cause. - User asked why clientKind is a string when the valid set is a small enum. Typos fail silently with stderr warning. - User asked why @CodebaseRoute is unified across HTTP+async when client/producer are split. v2 design (per user direction: breaking change OK, no migration): - Split @CodebaseRoute into @CodebaseHttpRoute and @CodebaseAsyncRoute - All 'kind'/'framework' fields become typed enums in the annotation set - @CodebaseClient.clientKind: String → CodebaseClientKind enum (required) - @CodebaseProducer.clientKind → producerKind (rename) + CodebaseProducerKind enum - Drop the optional cluster-name 'broker' field on both @CodebaseAsyncRoute (Java method-name collision with the new enum 'broker()') and @CodebaseProducer (uniform cleanup; resolver does nothing with it on either side) No code change in this propose; planning + implementation are separate. Test baseline unchanged: 290 passed, 4 skipped.
…roker
Review feedback exposed three more problems with the first v2 draft:
1. `framework` on @CodebaseHttpRoute is never used by the resolver.
Verified by grepping build_ast_graph.py / graph_enrich.py:
- Only used as a list_routes filter and routes_by_framework
cosmetic count.
- Never branched on by the call-edge matcher.
Drop it; the extractor still populates Route.framework
internally from source annotations.
2. `broker` on @CodebaseAsyncRoute is fully derivable from `kind`
(bijection: kafka_topic⇒kafka, rabbit_queue⇒rabbitmq, etc.).
Drop both `broker` and `kind` from the user-facing annotation;
the kind is inferred from the listener-annotation context.
3. The really important one: v1 @CodebaseRoute(kind=http_consumer)
was an OUTBOUND Feign declaration (caller-side declaration of a
remote endpoint), but @CodebaseRoute was advertised as inbound.
The annotation lied about its direction. Move Feign declarations
to @CodebaseClient(clientKind=feign_method, …) where they
honestly belong.
Net result: v2 inbound annotations carry only path+method or topic.
Outbound annotations carry kind enum + destination hints. Two
enums live (CodebaseClientKind, CodebaseProducerKind), down from
five in the earlier v2 draft.
Resolver flow change: pass6 hint-recovery walk now consults
@CodebaseClient records on the caller member instead of the
caller's http_consumer route. list_routes stops returning Feign
declaration rows — covered by a follow-up propose for a
list_clients MCP tool.
No code change in this propose; tests unchanged at 290 passed,
4 skipped.
Update — propose tightened (commit a144c91)Three additional design issues surfaced during review and the propose now incorporates all three fixes. Major rewrite, not an addendum. 1.
|
kind |
broker |
|---|---|
kafka_topic |
kafka |
rabbit_queue |
rabbitmq |
jms_destination |
jms |
stream_binding |
stream |
User-facing broker field dropped. Internally derived from listener-annotation context (@KafkaListener ⇒ kafka_topic, etc.). Falls back to kafka_topic for brownfield-only methods.
3. The big one: direction contradiction — Feign moves to @CodebaseClient
@CodebaseRoute(kind=http_consumer) represented a Feign declaration — a caller-side contract for a remote endpoint. That's outbound. But @CodebaseRoute was advertised as the inbound annotation. The annotation simultaneously claimed both directions depending on kind.
The honest model:
| Annotation | Direction | Owns the URL? |
|---|---|---|
@CodebaseHttpRoute |
inbound | yes — this method serves it |
@CodebaseAsyncRoute |
inbound | yes — this method consumes from this topic |
@CodebaseClient |
outbound | no — declares an outbound call to a remote URL |
@CodebaseProducer |
outbound | no — publishes to a remote topic |
Feign interface methods carry path+method known at compile time, but they're outbound (the implementation lives in the target service). They belong on @CodebaseClient(clientKind=feign_method, …), not on a "Route".
Net shape — clean 2×2
// inbound HTTP — just say where you listen
@CodebaseHttpRoute(path = "/users/{id}", method = "GET")
// inbound async — just say what topic you consume
@CodebaseAsyncRoute(topic = "user-events")
// outbound HTTP — Feign declaration, RestTemplate, WebClient
@CodebaseClient(clientKind = CodebaseClientKind.feign_method,
targetService = "user-service",
path = "/users/{id}", method = "GET")
// outbound async
@CodebaseProducer(producerKind = CodebaseProducerKind.kafka_send,
topic = "user-events")Total enum count: 2 (CodebaseClientKind, CodebaseProducerKind) — down from 5 in the earlier draft. Total annotations: 4. Mental model: inbound = "where do I listen?", outbound = "what kind of call and where does it go?".
Resolver flow change
HTTP_CALLS edges still go Symbol → Route (a http_endpoint Route on the target service). Internally, pass6's hint-recovery walk (build_ast_graph.py:1741–1770) needs to consult @CodebaseClient(clientKind=feign_method) records on the caller member instead of the caller's http_consumer route — same data, new storage location.
Behavioural change visible to the AMA agent: list_routes no longer returns Feign declaration rows. A follow-up PR proposes a list_clients MCP tool to cover that gap.
Tests
290 passed, 4 skipped — unchanged baseline (propose still doc-only)
Remaining open question
The earlier draft's three open questions are all resolved by the rewrite. One remains worth flagging:
- Should
@CodebaseClient.clientKindever become inferable? v2 keeps it required (the partial-override use case wants a channel hint even when path/method/target are missing). Future enhancement section flags this as a v3 candidate once we have real-project feedback.
…utes (#37) Companion propose to the v2 brownfield annotations PR (#36). After v2 lands, Feign declarations leave the Route table — they're correctly modeled as outbound @CodebaseClient annotations rather than inbound routes. This leaves a real workflow without an entry point: "show me every outbound HTTP call this service makes". Pre-v2, agents reach for list_routes(framework=feign), which is wrong on three counts (only Feign, conflates direction, returns nothing post-v2). The propose adds: 1. A new Client graph node table storing outbound-client declarations (one row per @CodebaseClient annotation; Feign methods synthesised from source). 2. A new DECLARES_CLIENT(Symbol → Client) rel table mirroring the existing EXPOSES(Symbol → Route) edge. 3. A list_clients MCP tool with filters symmetric to list_routes (microservice, client_kind, target_service, path_prefix, method). The HTTP_CALLS(Symbol → Route) edge stays unchanged — Client is additional caller-side metadata, not a replacement. Pass6's hint recovery walk retargets from the caller's http_consumer route to the caller's DECLARES_CLIENT → Client path. Same data, new home. Three follow-up tools sketched but punted: - get_client_by_path (symmetric with get_route_by_path) - find_client_callers - find_client_target_route A parallel Producer node + list_async_producers tool is also flagged as future work for the async outbound side. ONTOLOGY_VERSION bump: 9 → 10. No code change in this propose; doc-only.
Summary
Propose v2 of the brownfield annotation set, addressing three pain points raised during the first real-project rollout:
@CodebaseRoute(framework=kafka, …)to register an outbound Kafka producer — right concept, wrong annotation. The asymmetry of v1 (one inbound annotation covering both transports; two outbound annotations split by transport) is the proximate cause.@CodebaseClient.clientKindis aStringeven though the valid set is a 5-element frozenset. Typos fail silently with a stderr warning the user usually never sees.@CodebaseRouteis unified across HTTP and async while the outbound side is split — the annotation set is internally inconsistent.Per project policy ("breaking changes allowed, no production users yet"), v2 ships as a single breaking change. No deprecation phase.
v2 design — clean 2×2
@CodebaseHttpRoute@CodebaseAsyncRoute@CodebaseClient@CodebaseProducerClass-level (
@CodebaseRole,@CodebaseCapability) — unchanged.Key shape changes
@CodebaseHttpRoute:frameworkenum narrowed to{spring_mvc, webflux, feign};kindenum narrowed to{http_endpoint, http_consumer}.pathandmethodpromoted to mandatory.@CodebaseAsyncRoute:brokerenum{kafka, rabbitmq, jms, stream}(renamed fromframework);kindenum{kafka_topic, rabbit_queue, jms_destination, stream_binding}.topicmandatory.@CodebaseClient.clientKind:String→CodebaseClientKindenum (required). Async values removed (they belong on the producer).@CodebaseProducer:clientKind→producerKind(rename —clientKindwas a v1 misnomer; a Kafka send is a producer not a client).producerKindis now aCodebaseProducerKindenum, defaulting tokafka_send.brokerfield dropped on both@CodebaseAsyncRoute(Java method-name collision with the new enumbroker()) and@CodebaseProducer(uniform cleanup — the resolver carries it through but no downstream tool filters on it).What's in this PR
A single propose document — no code changes. Planning and implementation are separate phases.
propose/BROWNFIELD-ANNOTATIONS-V2-PROPOSE.md— full propose with problem statement, v2 annotation set (with stub source), per-annotation diff table, file-by-file impact list, acceptance criteria, out-of-scope items, and three open questions.Tests
Unchanged from the master baseline at
d62b48c(PR #34 + PR #35) — propose is doc-only.Open questions inside
brokerfield collision + cluster-name fields dropped — recommended drop on both async annotations; reintroduce later ascluster()if a real use case emerges.producerKindrename — recommended; v2 is the only time it's free.@CodebaseHttpRoute.frameworkbe optional? — recommended keep mandatory (the propose's whole thrust is "say what you mean, get a compile error if you don't").Out of scope (not for v2)
clientKind@CodebaseScheduledTask)Next steps (if approved)