Corpus parity: formatter-enum expansion, raw:false flag, try/catch for binary chains#6
Corpus parity: formatter-enum expansion, raw:false flag, try/catch for binary chains#6kevinelliott wants to merge 2 commits into
Conversation
… success-conditional descriptions
The new TS corpus runner (acars-decoder-typescript, 289 samples) failed
41 samples on first run, splitting into three root-cause classes. This
commit fixes all three at the spec/emitter level:
Class B — formatter enum too narrow (10 samples, 44-family):
The v1 enum forced specs to map month/day/eta/off/on/in/fuel-remaining
through generic `timestamp`/`fuel`, flattening distinct legacy items
(MSG_MON, MSG_DAY, ON "Landing Time", ' FUEL_REM') into TIMESTAMP/FOB
items. The unit tests never caught it; the corpus deep-equal did.
- schema: formatter_call.type adds eta/out/off/on/in/day/month/
departure_day/arrival_day/fuel_remaining/remaining_fields
- ir.ts: FormatterCall union extended to match
- emit-typescript: methodMap additions + `remaining_fields` emission
(`if (data.length > N) ResultFormatter.unknownArr(result,
data.slice(N))` — the Label_44_Base.addRemainingFields pattern,
closing the bulk-port agent's documented trailing-fields caveat)
- specs 44/{ON,IN,OFF,ETA,POS}: items now use the precise types;
44/POS drops its fuel item entirely (legacy sets raw.fuel_in_tons
with NO formatted item — the field's auto-raw emit covers it)
Class A — success-conditional descriptions (30 samples; arinc_702,
16/AUTPOS, H1/Paren):
Legacy plugins built on defaultResult() leave description "Unknown"
on failed decodes and set the real description only on success
(in-body or via Arinc702Helper). The generated wrapper set the spec
description unconditionally. Fixed by setting those three specs'
descriptions to "Unknown"; the existing hatches already set the real
description on their success paths (verified), so successful samples
are unaffected.
Class C — uncaught throw in binary chains (1 sample, H1/OHMA):
Legacy wrapped base64→inflate→decode chains in try/catch and failed
gracefully; the generated chain threw. emit-typescript now wraps the
decode body in try/catch (→ failUnknown) whenever the parse steps
include a binary kind (ascii85/base64/deflate/text_decode/hex_decode).
Rust/C are unaffected: their helpers return empty buffers rather than
throwing.
Runtime (TS): remainingFuel now tolerates undefined/NaN (mirrors the
legacy parseFuel isNaN guard and the currentFuel precedent) so
when-gated spec fields can call it unconditionally.
All 68 specs validate; TS target regenerates clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fields like Label_44_POS's flight_level_raw exist only to feed a later field/formatter; the legacy plugin never stored them in result.raw. The auto-raw emit was adding a divergent raw key (caught by the last 2 corpus failures). 'raw: false' suppresses the auto-emit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 59 minutes and 33 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 83908caabd
ℹ️ 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".
| | "eta" | ||
| | "out" | ||
| | "off" | ||
| | "on" | ||
| | "in" |
There was a problem hiding this comment.
When ads-gen is run with --target rust or --target c, these shared formatter types are now accepted by the schema/IR and used by the label 44 specs, but codegen/src/emit-rust.ts and codegen/src/emit-c.ts still only map the old formatter names and otherwise emit TODO formatter comments. Because those same emitters suppress auto-raw for fields referenced by formatters, generated Rust/C decoders for these specs will silently omit the formatted items and the canonical raw values for fields such as ETA/out/off/on/in/month/day/fuel_remaining rather than matching the updated TypeScript output.
Useful? React with 👍 / 👎.
| # Intermediate value feeding `altitude`; legacy never stored it in raw. | ||
| raw: false |
There was a problem hiding this comment.
For --target rust and --target c, this new raw: false flag is lowered into the shared IR but the Rust/C emitters' skipAutoRaw checks still only look at consumedByFormatter, so regenerating Label_44_POS in those targets will continue to write the intermediate flight_level_raw into result.raw. This makes the new spec flag target-dependent and preserves the parity bug the flag is intended to fix whenever the non-TS codegen paths are used.
Useful? React with 👍 / 👎.
1 participant
Summary
The new TS corpus runner (acars-decoder-typescript@031f096) deep-equals 289 production-extracted samples against the generated decoder. First run failed 41 — all real parity gaps the unit tests never caught. This PR fixes every one at the spec/emitter level; the corpus now passes 289/289 byte-for-byte.
Three root-cause classes
1. Formatter enum too narrow (10 samples, 44-family)
The v1 enum forced specs to express month/day/eta/off/on/in/fuel-remaining as generic
timestamp/fuelitems, flattening distinct legacy items (MSG_MON,MSG_DAY,ON/"Landing Time",' FUEL_REM') intoTIMESTAMP/FOB.formatter_call.typeaddseta/out/off/on/in/day/month/departure_day/arrival_day/fuel_remaining/remaining_fieldsremaining_fieldsemission (if (data.length > N) ResultFormatter.unknownArr(result, data.slice(N))— closes the bulk-port agents' documentedaddRemainingFieldscaveat)44/{ON,IN,OFF,ETA,POS}updated to the precise types;44/POSdrops its fuel item (legacy setsraw.fuel_in_tonswith no formatted item)2. Success-conditional descriptions (30 samples: arinc_702, 16/AUTPOS, H1/Paren)
Legacy
defaultResult()-style plugins leavedescription: "Unknown"on failed decodes and set the real description only on success. Specs now declare"Unknown"; the hatches' success paths already set the real description (verified).3. Uncaught throw on malformed binary input (1 sample, H1/OHMA)
Legacy wrapped base64→inflate chains in try/catch → graceful fail. The TS emitter now wraps the decode body in try/catch (→
failUnknown) whenever parse steps include a binary kind. Rust/C unaffected (their helpers return empty buffers rather than throwing).Plus: field-level
raw: falseflagIntermediate fields (e.g.
44/POS'sflight_level_raw) that feed later fields but were never stored inrawby the legacy plugin can now suppress the auto-raw emit.Runtime (TS):
remainingFueltolerates undefined/NaNMirrors the legacy
parseFuelisNaN guard and thecurrentFuelprecedent.Verification
Why this matters
This is the corpus doing exactly its job: the 407 unit tests passed throughout while 41 samples diverged. Byte-parity claims for Rust/C in Stage 3 now have a proven, executable definition.
🤖 Generated with Claude Code