corpora: validate doc, rtf, xlsx drawings, and compute against real-producer output - #1184
Merged
Conversation
Mearman
marked this pull request as ready for review
September 10, 2026 06:04
Member
Author
|
@codex security review |
Mearman
enabled auto-merge (rebase)
September 10, 2026 06:04
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ 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. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
scripts/generate-corpus.mjs authors construct-spanning flat-ODT and packaged-ODT sources (paragraphs, styled runs, headings, lists, tables with merges, an as-char inline picture, sections) and converts each headlessly through Writer's own MS Word 97 filter, so every corpus file is genuine application output; the regenerated corpus.test.ts asserts each fixture's expectations against the reader, and test/corpus/ stays gitignored per the family's convention. The corpus immediately exposed a real reader defect: the inline-picture locator assumed Word's OfficeArtInlineSpContainer nesting, which LibreOffice does not follow (its property-table record's recLen spans past the blip), so blips were silently dropped from every LibreOffice-produced document. The locator now scans for a validated blip record -- known type, known rgbUid instance, length inside the stream, payload starting with its format's own file signature -- which is spelling-independent and rejects wrapper bytes outright.
scripts/generate-corpus.mjs authors construct-spanning flat-ODT sources (styled runs, colour, headings, lists, tables, multiple paragraphs, alignment) and converts each headlessly through Writer's own RTF export filter, so every corpus file is genuine application output rather than hand-authored control words; the regenerated corpus.test.ts asserts each fixture's expectations against the reader through readRtfContent, and test/corpus/ stays gitignored per the family's convention. The corpus validated the reader cleanly across all eight fixtures -- no defects found -- turning the README's 'never been exercised against real application output' status line into a statement of what now is.
…orpus scripts/generate-xlsx-drawing-corpus.mjs authors flat-ODS sources with declared column widths and row heights and a cell-embedded picture at varied anchors, converting each headlessly through Calc's own Office Open XML export so both the drawing markup and the worksheet grid it resolves against are genuine application output; the regenerated corpus.test.ts asserts the anchor cell and frame against the authored geometry. The corpus exposed a real sizing defect: a twoCellAnchor editAs="oneCell" picture's frame was derived from the to-marker difference, which disagrees with the producer's own transform extent by the character-unit column-width approximation underneath -- the producer's exact-EMU transform now rules for the oneCell spelling, the to-marker difference for twoCell (ECMA's default and the resize-with-grid semantics). Calc never emits xdr:absoluteAnchor (it normalises every drawing to twoCellAnchor spellings), so that path keeps its hand-built fixtures -- stated in the module's own gate comment.
…following operand The unary-minus lowering only recognised a minus at the HEAD of a sequence, so a stated answer like T = -0.36 degraded its entire equality to one unparsed node (and a + -b likewise): the empty segment between the relation and the number tripped the operator-placement diagnostic instead of reading as the number's sign. Normalisation now rewrites every empty-segment-with-a-subtract-after-it into a negation flag on the segment the minus precedes -- parity-counted for --x -- leaving genuine placement errors (a = = b) to the existing diagnostic, and the leading-minus case folds through the same pass rather than its own special case. Found by the generated at-scale worked-example corpus (37 of its first 300 documents carried a negative stated answer).
…corpus scripts/generate-corpus.mjs regenerates 300 deterministic worked-example markdown documents over the mechanically-lowered arithmetic grammar (explicit \times/\frac/\sqrt compositions, never juxtaposition; single-letter symbols, since digit-bearing and multi-letter tokens lower to unparsed nodes and juxtaposition respectively), each stated answer computed by the generator's own parallel JS evaluation and rounded to 6 significant figures the way a textbook author rounds -- so a mismatch names a genuine lowering/evaluation defect, never a fixture typo. The regenerated corpus.test.ts runs the at-scale measurement through the identical readMarkdownContent -> lowerMarkdownMath -> runCorpus pipeline the starter corpus uses, asserting full coverage and writing the measured report to test/corpus/report.txt (300/300 matched, 0 gaps, 0 unresolved at introduction). The package's vitest config gains the explicit unit/corpus project split the family convention carries, which the configless default include would otherwise let leak into pnpm test.
…m the corpus generator
Mearman
force-pushed
the
fix/956-corpora
branch
from
September 10, 2026 06:10
fd82789 to
4aeebe3
Compare
Contributor
|
🎉 This PR is included in version 8.10.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: fd82789b06
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
|
|
||
| /** Scans forward from `from` for a validated blip record (see readInlinePicture's own locating note) -- every candidate header of a blip type must also carry a known rgbUid instance count, a length inside the stream, and payload bytes starting with its format's own file signature. */ | ||
| function findBlipRecord(data: Uint8Array, from: number): FoundBlip | undefined { | ||
| for (let at = from; at + RECORD_HEADER_SIZE <= data.length; at++) { |
There was a problem hiding this comment.
🛡️ Codex Security Review
When a service accepts attacker-supplied .doc files, this byte-by-byte search runs to the end of the entire Data stream for every U+0001 anchor. Anchors can reuse one sprmCPicLocation, so work is O(anchors × Data bytes): a 269 KB document with a 256 KiB Data stream and 64 anchors held readDocContent for 6.44 s; the parent took 0.53 ms for 16 equivalent lookups versus 1.65 s here. Bound the search to the referenced picture, cache by location, and add a work/abort budget. Parser resource amplification is in scope.
SECURITY.md reference: SECURITY.md:L15-L18
Useful? React with 👍 / 👎.
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
1 participant
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.
Closes #956: the four-package corpus workstream, each layer built from LibreOffice-produced output (soffice headless), each with a committed rerunnable scripts/generate-corpus.mjs, a vitest corpus project under pnpm test:corpus, and test/corpus/ gitignored per the family convention.
The corpus runs found and fixed three genuine defects:
rtf-codec validated cleanly on first run against its first-ever corpus (8 documents). document-compute.js's at-scale differential measurement: 300/300 worked-example documents matched at 6 significant figures -- 100.0%, 0 gaps, 0 unresolved (report committed at test/corpus/report.txt).
Provenance, stated in each README: every corpus is LibreOffice-produced -- no genuine Word-produced files exist on this machine outside private user data (excluded on privacy grounds). Each README says what a LibreOffice corpus does and does not prove; real-user corpora remain a local drop-in on top.
Closes #956.