docs: propose generating the wire types behind a facade - #12
Closed
bdchatham wants to merge 1 commit into
Closed
Conversation
The module tracks spec/openapi.json by hand across four files and 2643 lines. The snapshot is one event variant behind upstream today, and the suite is green, which is the defect: the conformance test proves the module declares no field the document lacks, and cannot prove the reverse. ADR 0001 proposes generating those types into internal/, where a path-mangled operation name cannot reach a consumer, and keeping the facade hand-written. spec/preprocess.py is the evidence: six transforms that make the generated types match the hand-written ones field for field, type for type and tag for tag, over ten types and 53 event variants, measured with oapi-codegen v2.8.0 against upstream 9d54826e. Status is Proposed. Nothing runs the script, no dependency is added, and no generated code lands here. The AGENTS.md rule against a generator stays as it is, with a pointer so the rule and the proposal do not silently disagree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Superseded by #13, which targets The script was dead code in the tree — nothing ran it, and its only job was to keep the spike reproducible. The ADR now says plainly that a throwaway spike produced the numbers and no code here carries it, so a reader is not left hunting. The generation work itself is what should stack on #11. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #11. Docs and one inert script — no code changes, no dependency, nothing wired.
What this asks
AGENTS.mdsays the module runs no code generator, and that re-adding one is a one-way door. That rule was set on good reasoning about a previous generator, which decided the shape of the public surface. This ADR asks whether the reasoning still holds when the generated code lands ininternal/, where a generated name cannot reach a consumer.Status is Proposed. Accepting or rejecting it is a separate decision from merging this PR, which only records the question and the measurements.
Why now
The module tracks
spec/openapi.jsonby hand acrosstypes.go,session_types.go,event.go, andenums.go— 2,643 lines.The snapshot is one event variant behind upstream today (
SessionTitleEvent, on main since9d54826e), and the suite is green. That is the defect, not an oversight:conformance_test.goproves the module declares no field the document lacks, and by design cannot prove the reverse. A new field is invisible.What was measured
A pre-process stage of six transforms, then
oapi-codegenv2.8.0:x-go-type: float64numberbecomesfloat32x-go-type-skip-optional-pointer*[]Tand*map[K]Von optional collectionsx-go-nameLlmModel,TotalCostUsd,McpStartupx-go-type: stringMcpServerStartup,SessionMcpStartupEventx-go-type: json.RawMessageConversationItem.dataResult against upstream
9d54826e:SessionResponse(46 fields),SessionListItem(27),ChildSessionSummary(21),AgentObject(14),UpdateSessionRequest(13),ConversationItem(7),ModelUsage(6),SkillSummary,ConversationDeleted,SessionForkRequest.Because the types are identical and the ten data types carry no methods, aliasing them is non-breaking — the public surface does not move, so this could land later as a patch.
What generation would not reach
stream.go, 579 lines.oapi-codegenignores the OAS 3.2itemSchemakeyword, and its typed response reads the whole body, which for an SSE stream never ends. This is the piece upstream addeditemSchemafor, and no Go generator reads it. The ADR's Consequences section lists the rest, including three new dependencies where there are none today.Related
Upstream
omnigent-ai/omnigent#5119asks for numericformatat the source. Worth knowing that it is not a dependency of this proposal —x-go-type: float64closes thefloat32defect locally. Accepting it upstream would retire three of the six transforms.Verification
Two pre-existing
valewarnings remain inAGENTS.mdat lines 8 and 32, outside this diff.ruffandblackare not installed here, sospec/preprocess.pyis unlinted — its output was verified byte-identical to the measured run instead.Review focus
The ADR's Consequences → Negative and Alternatives considered are the sections worth your attention. The positives are measured and dull; the costs and the rejected paths are where I would want to be wrong.