Repository navigation
docs(generate): add EGM contracts for cmd and cmd/generate - #79
Merged
Merged
Conversation
Member
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0daaa4227
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ThomasK33
added a commit
that referenced
this pull request
Oct 29, 2026
WriteHelpersJen repeated emitUnion's variant naming, but it seeded
usedTypeNames with schema definition names only. WriteTypesJen also
registers names for inline object properties before it names union
variants, so a variant whose preferred name was already taken got a
fallback name in types_gen.go while helpers_gen.go kept the preferred
name: the constructor built the wrong or a missing type and the package
did not compile.
emitUnion now records each oneOf variant it emits (field, type,
discriminator, required keys, properties), and WriteHelpersJen builds
the types once and creates constructors from that record. The second
copy of the naming logic is gone, so the two emitters cannot disagree.
WriteTypesJen keeps its signature and now renders buildTypesJen's file.
TestWriteHelpersJen_UsesUnionVariantTypes failed before this change
("helpers_gen.go sets PickRequest.Item to &PickItem{...}, but the field
has type *PickRequestItem") and passes now. No variant in the current
schema collides, and the regenerated *_gen.go files are byte-identical.
Found by Codex review on #79.
_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28e9c792c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ThomasK33
added a commit
that referenced
this pull request
Oct 29, 2026
…tracts Review round 2 on #79 and a read-only audit of all seven generator contracts found sentences that stated a rule more broadly than the code: - WriteHelpersJen now uses meta, so a nil meta panics. - generateNestedTypeName resolves taken names; panics are for exhausted names, taken VariantN names, and failed registration checks. - A bad flag exits with status 2; later errors panic. - load: key conflicts error only for wires that are not stable; only refs to copied definitions are rewritten; promoted definitions include new ones; the copy shares Type, Const, Default, and Enum values. - ir: the fallback is keyed by side and wire, and other sides get BindUnknown. - emit: map-order independence assumes one key per wire; parent properties merge only into inline object variants; const injection is for object variants; Validate checks only fields whose own type is string or array (not $ref-typed ones such as sessionId). - util: SplitCamel does not split before a leading underscore. - cmd: -schema and -out set one directory each. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Member
Author
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
ThomasK33
force-pushed
the
egm/docs-generate
branch
from
October 29, 2026 17:36
766431c to
56abf9c
Compare
Add README contracts (Elephant-and-Goldfish Model) for cmd/, cmd/generate/, cmd/generate/internal/, and its emit, ir, load, and util packages. Each package contract lists its public interface, the invariants the code enforces, its side effects, and a verification command. Every entry cites the file and symbol it comes from. Fresh-context read-only checkers compared every contract with the code (drift report), and a second round checked the fixes. The emit contract describes the map-order and phantom-variant behavior after the previous commit. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
- Validate is generated only for object schemas among the *Request, *Response, and *Notification types (AgentResponse, ClientResponse, ExtRequest, ExtResponse, and ExtNotification have none). - WriteHelpersJen now builds its constructors from the variants that emitUnion records, so the contract no longer says the two emitters repeat the naming logic. - os.WriteFile and os.MkdirAll apply 0o644 and 0o755 only when they create a file or directory. An existing one keeps its permissions. - hasSchema stats schema/schema.json only when schema/meta.json exists. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
…tracts Review round 2 on #79 and a read-only audit of all seven generator contracts found sentences that stated a rule more broadly than the code: - WriteHelpersJen now uses meta, so a nil meta panics. - generateNestedTypeName resolves taken names; panics are for exhausted names, taken VariantN names, and failed registration checks. - A bad flag exits with status 2; later errors panic. - load: key conflicts error only for wires that are not stable; only refs to copied definitions are rewritten; promoted definitions include new ones; the copy shares Type, Const, Default, and Enum values. - ir: the fallback is keyed by side and wire, and other sides get BindUnknown. - emit: map-order independence assumes one key per wire; parent properties merge only into inline object variants; const injection is for object variants; Validate checks only fields whose own type is string or array (not $ref-typed ones such as sessionId). - util: SplitCamel does not split before a leading underscore. - cmd: -schema and -out set one directory each. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
ThomasK33
added a commit
that referenced
this pull request
Oct 29, 2026
Fixes three latent defects in `cmd/generate/internal/emit`. No variant in the current schema triggers either one, and regenerating leaves every `*_gen.go` file byte-identical.
1. Map order. `emitUnion` and `WriteHelpersJen` took the fallback discriminator as the first `const` property found by ranging over a map, and `emitUnion` collected the `const` injections for `MarshalJSON` the same way. A variant with two or more `const` properties could produce different output on each run. Both now scan properties in sorted key order (`sortedPropertyKeys`, `firstConstPropertyKey`).
2. Phantom variants. `emitUnion` emits no type for a variant with no `$ref`, no properties, no `null` type, and no title, but `WriteHelpersJen` still emitted a constructor for it, which referenced a type that does not exist. Both now use `isPhantomVariant`.
3. Helper names. `WriteHelpersJen` repeated `emitUnion`'s variant naming, but it never saw the names that `WriteTypesJen` registers for inline object properties first. A variant whose preferred name was taken got a fallback name in `types_gen.go`, while `helpers_gen.go` kept the preferred name, so the package did not compile. `emitUnion` now records each `oneOf` variant it emits, and `WriteHelpersJen` builds the types once (`buildTypesJen`) and creates its constructors from that record. The second copy of the naming logic is gone.
**Review focus:** `buildTypesJen` in `types.go` and the shorter `WriteHelpersJen` in `helpers.go`. Helpers now trust `emitUnion`'s record (field, type, discriminator, required keys, properties), so check that the record holds what the old helper code computed. The byte-identical regeneration says it does for the current schema.
**Evidence**
- The new tests in `union_test.go` failed before the fix (`types_gen.go differs between runs`, `helpers_gen.go builds undeclared type PickVariant2`, and `helpers_gen.go sets PickRequest.Item to &PickItem{...}, but the field has type *PickRequestItem`) and pass after it, 10 of 10 runs with `-count=10`. The third defect was found by Codex review on #79.
- `cd cmd/generate && go test ./...` passes.
- `cd cmd/generate && env -u GOPATH -u GOMODCACHE go run .` leaves `git diff -- '*_gen.go'` empty.
Second PR of a 4-PR stack. It touches only `cmd/generate/internal/emit` and does not depend on the examples change below it.
---
_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
ThomasK33
force-pushed
the
egm/docs-generate
branch
from
October 29, 2026 17:38
56abf9c to
cd2b249
Compare
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.
Adds README contracts in the Elephant-Goldfish Model (EGM) format for
cmd/,cmd/generate/,cmd/generate/internal/, and theemit,ir,load, andutilpackages. Each package contract lists the public interface, the invariants the code enforces, its side effects, and a verification command, and each entry cites the file and symbol it comes from.cmd/andcmd/generate/internal/are short index pages that link to their children.Review focus: these files are claims about the code. The
emitandloadinvariants are the densest, so check a sample there against the code. Theemitcontract describes the sorted scans and the phantom-variant skip from the PR below.Evidence: fresh-context read-only checkers compared every contract with the code in a drift review. It found 36 findings across the 13 contract files of this stack, and all were fixed. A second round of fresh checkers confirmed the fixes, and the four small problems it found were fixed too.
mdformat --checkpasses, and every relative link resolves.make check(treefmt withmdsh) was not run locally becausemdshis not installed here: CI runs it.Third PR of a 4-PR stack. Documentation only.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh