fix(compiler): address build-time iteration review feedback (#511)#621
Conversation
Address Codex review findings on the merged build-time iteration work: - reverse/take now charge copied elements against the per-block budget, so repeated list-producing transforms cannot bypass the documented cap. - The literal splitter tracks Go raw strings (backticks), so a comma inside a raw-string list/object element no longer breaks parsing. - Recursive AST evaluation enforces the nesting-depth limit, so deeply nested parentheses/operators produce a diagnostic instead of risking stack overflow. - List/object number values serialize as canonical JSON (e.g. 01 -> 1) while preserving exact wide-integer literals. - Go build helper output is decoded with json.Decoder.UseNumber, so large integer IDs inside returned slices/structs keep full precision. - seq bounds are constrained to the safe-integer range so end-start cannot overflow int before the budget/limit check. Adds regression tests for each.
![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: d695a4396f
ℹ️ 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".
| decoder.UseNumber() | ||
| var raw map[string]any | ||
| if err := json.Unmarshal(output, &raw); err != nil { | ||
| if err := decoder.Decode(&raw); err != nil { |
There was a problem hiding this comment.
In the build-data runner path I checked, stdout is supposed to be exactly one JSON object while stderr is handled separately. Switching from json.Unmarshal to a single Decode accepts {"title":"ok"} debug or a second JSON object after the first and silently ignores the trailing bytes, so a helper that writes trailing stdout can produce a successful build with only the first object instead of surfacing corrupted helper output. After this decode, check that the decoder is at EOF (or otherwise reject non-whitespace trailing data).
Useful? React with 👍 / 👎.
1 participant
Follow-up to #613 (merged) addressing the Codex review findings on the build-time iteration work.
reverse/takecopied list elements without charging the budget, so repeated transforms bypassed the per-block caplen(result)againstenv.consumein both", so a comma inside a Go raw string (`a,b`) was treated as a separator and broke parsingexprScannernow tracks backtick raw stringsbuildEvalASTenforces the depth limit and descends viaenv.deeper()01serialized as invalid JSON[01]9007199254740993stays exact)float64, rounding large integer IDs inside returned slices/structsjson.Decoder.UseNumber; handlejson.Numberseq(-9e18, 9e18)overflowedintatend-startbefore the budget check, risking a panicseqoperands to the safe-integer rangeAll six are real correctness issues; each ships with a regression test (
TestBuildDataReviewRegressions,TestBuildFunctionOutputPreservesWideIntegers).go test ./internal/buildgen/passes;gofmt/go vetclean.