Reach a 100% Stryker mutation score for wpd-codec - #1246
Merged
Merged
Conversation
format.ts, errors.ts, codec.ts, bytes/base64.ts, bytes/view.ts, stream/eol.ts, and stream/furniture.ts had no dedicated test file at all; every mutant in them was either unreachable or only incidentally touched by integration tests that never isolated a specific branch. Adds direct unit coverage for each, including exact boundary cases (int16At's 0x7fff/0x8000 sign flip, sliceAt's off-by-one bounds, isSingleByteEol's range edges, readFurnitureClaim's six subgroup codes) and a partial-byte-match case for hasWordPerfectOrCompoundHeader that no existing fixture (always a full match or a full mismatch) ever exercised. Also covers build-wpd.ts's own synthetic-file builder directly (buildWpdFile's file-size stamping and reserved-long fidelity, variableFunction/eolFunction's default-argument and size arithmetic) and compound-file.ts's [MS-CFB] writer through a real read-back via archive-codec, including the big-stream and mini-stream boundary at 4096 bytes that every existing container test happens to stay under. Removes build-wpd.ts's own redundant encryption-word write: the buffer already starts zero-initialised, so writing 0 to it was always a no-op. Adds two page.ts tests isolating a stated length of zero from a stated width of zero, which the existing "no size" test always set both to zero at once and so could never tell apart.
Every reader in container/ had boundary conditions no existing test
distinguished from a neighbouring, coincidentally-identical outcome:
- documentAreaEnd's fallback-to-buffer's-own-end path was never
proven against a file whose header {file size} is genuinely valid
and shorter than the buffer, only against the two cases that force
the fallback -- so a version that always returned bytes.length
would have passed every existing test.
- hasWordPerfectFileId's own length guard was provably redundant
(indexing a Uint8Array past its end always answers undefined, which
never equals a real file-ID byte) and is removed; a partial-match
case now isolates every() from some().
- readPrefixPackets' index-count and packet-type-0 branches, and
readGeneralWpTextBlocks'/readTypefaceName's own arithmetic, were
exercised only by inputs where an unrelated later check produced
the identical fallback result regardless of the mutated branch.
readGeneralWpTextBlocks' firstBlockOffset < 0 guard is removed for
the same reason as hasWordPerfectFileId's: the value is an unsigned
16-bit read and can never be negative.
- readDocumentSummary's date-zero check, group-size/cursor arithmetic,
and per-tag text/keyword assignment were only ever tested with every
triggering condition true at once, or with text that already carries
its own null terminator (masking any inflated word-count mutation);
new cases isolate each clause and use un-terminated raw word data
so an inflated budget would visibly read into a neighbouring group.
- decryptWpdDocument's and applyWpdStandardEncryption's own
empty-password guards were never called with an empty password
directly; container.ts's identical, redundant re-check of the same
condition readFileHeader already performs is removed.
- normaliseWpdPassword's Latin-1 boundary (0x00FF handled, 0x0100
refused) and its error message's exact hex formatting had no direct
test.
Two remaining boundary comparisons (readStyleBeginBlock's start < 0,
covered separately; summary.ts's group-header-fits-exactly check,
container.ts's file-size <= boundary) are proven equivalent in place
with a Stryker disable comment, since every input where the stricter
and looser comparison disagree already converges on the same result
through a check right beside it.
…, table, tokenise decodeSingleByteCharacter and decodeAsciiSet had no direct boundary tests at either edge of their ASCII ranges (0x20/0x21 and 0x7f/0x80), and decodeWordString's own high-byte offset and null-terminator check were only ever exercised by pure-ASCII multi-character strings, whose high byte is always 0 regardless of which word it actually belongs to -- masking a wrong offset entirely. A non-ASCII word now proves each word reads its own high byte. ole.ts: readGraphicsChildIds' and readOleDescriptor's own length guards were exercised only with either a full match or a wild overrun, never at the exact byte-count boundary; readOleDescriptor's OLE2 branch two arithmetic mutations on its word-count budget are proven equivalent in place (decodeWordString's own bounds check always stops at the packet's true end regardless of an inflated budget, since there is never real data past bytes.length to read). style.ts: readSystemStyleNumber's redundant undefined check is removed (returning value directly already answers undefined when the byte is absent); styleSemanticsFor's heading and not-indented ranges had no test one past their own stated upper bound; readStyleBeginBlock gains the same start < 0 dead-code removal as prefix.ts's firstBlockOffset, plus a pid-count overrun case and direct coverage of a packet too short to hold its own pid count at all. table.ts: readEmbeddedSubfunctions' own overrun check is removed as redundant -- the end-gate check immediately after it already reads undefined for any offset the overrun check would have caught, and never equal to a real code. Its while-loop boundary and the cell formula length-word boundary are proven equivalent for the same reason (the one iteration or branch they gate is already a no-op or already rejected elsewhere). nearestPercentType's tie-breaking comparison is proven equivalent: no achievable shade byte produces a percentage exactly equidistant between two adjacent table entries. readCellFill's "believed unreachable" foreground-undefined branch, already documented as unreachable in its own comment, is marked accordingly rather than chased with a test that cannot exist. tokenise.ts: every WpdFormatError message's exact wording (hex case, padding) is now asserted directly rather than only matched by a loose regex, and two message paths (a variable-length function overrunning the document area, and an end-gate mismatch) had no test constructing them at all.
…acked Its buffer is always a fresh, plain Uint8Array, but the declared return type was the wider Uint8Array (ArrayBufferLike), which only readCompoundFile's own stricter Uint8Array<ArrayBuffer> parameter type exposed once a test called it directly instead of only ever routing the bytes through openWpdDocument's looser signature.
Same gap as compoundFileWithStream: its buffer is always a fresh, plain Uint8Array, but the declared return type was the wider Uint8Array (ArrayBufferLike), which archive-codec's writeCompoundFile rejects now that a test passes this fixture to it directly instead of only ever routing it through this package's own looser-typed helpers.
walkOverrideBlocks' loop bound (bit >= 5, not just the three bits this reader names) mattered only when an unread bit's own block carries a corrupted size -- no existing test set a low, unread bit at all, so the walk never had a chance to notice one lying about its extent. readContentType's and readPositionOverride's PID-flags-skip guards (bits 15/14, "is there room for these two bytes") are redundant and removed: neither reads anything through that skip, so an insufficient buffer either surfaces at the very next read that actually touches the buffer, or -- with nothing after it -- leaves every optional field undefined, which the public API (readBoxContent's own frame and contentType checks) cannot distinguish from this function having been rejected outright. The three remaining minimum-length checks (contentBlock.length, positionBlock.length, and the block-size-field overrun check) have their own <= boundary proven equivalent in place for the identical reason: the one length where < and <= disagree already fails every downstream consumer's own guard regardless. The horizontal- and vertical-position sub-blocks' own "is this offset absolute from the page edge" checks, and the width/height sub-blocks' own presence checks, had no case isolating a non-absolute type or an unset bit whose neighbour was set -- a full position override exercising all six optional sub-blocks together with distinct values now proves every cursor advance lands on its own bytes rather than a neighbour's, and two dedicated cases prove a bit that was never set can never resolve a phantom width or height from bytes that belong entirely to the sub-block actually present.
…'s JPEG scanner readTableFormula's token table names far more codes than the existing tests ever built: the space-count (25), plain cell/range references (27/28), the floating point constant (30, its own 8-byte double and byte-string spelling), the user argument reference (31) and function call (32), the three no-payload attribute markers (42/43/44), and every individual absolute-reference flag bit across both the ranged (48-63) and cell (64-67) code families -- the one existing range- reference test sets two flag bits at once, which cannot tell any one of the four apart from another. columnLetters' own base-26 rollover had no test past a single letter, so a version that stopped after one pass regardless of the remaining value would still have passed every existing case. cellReferenceText's negative-row/column guard is only reachable at all through a signed cell-number read, which nothing previously constructed. readLengthPrefixedByteString's own decoded text is discarded by its only caller (code 30 checks definedness alone, using the double it already read instead) -- the two mutations that only change what text it builds, never whether it builds one, are marked equivalent in place rather than chased with an assertion nothing can observe. image.ts had no dedicated test file at all: scanPng's chunk walk and the entirety of scanJpeg (marker-segment walking, fill-byte runs, standalone markers, SOS-to-EOI entropy scanning) were exercised only incidentally, through one integration test carrying a single well-formed PNG. Adds direct coverage for both scanners' malformed and boundary inputs (a chunk or segment whose stated length exactly exhausts the remaining buffer, an EOI landing on the buffer's own final two bytes, a stuffed FF00 inside entropy data that must not be mistaken for EOI, and the standalone-marker classification for TEM and the RST0-RST7 range) and the PNG-vs-JPEG precedence when a packet carries both signatures.
…h width and height A first pass at box.ts's position-override tests set a horizontal or vertical sub-block alone, with neither width nor height also present -- frame is only ever computed once both are defined, so a bug in the horizontal/vertical type check or an insufficient-room guard produced the identical frame: undefined result whether or not the bug fired, masking it entirely. Every such case now also carries width and height, and two new cases isolate positionResolved's && from || by resolving exactly one of x/y rather than both or neither. Also fixes three Stryker disable comments placed on the line introducing a multi-line expression rather than the line the mutated operator actually sits on (ole.ts's byte-string word budget, and table.ts's percent-tie comparison and its own unreachable-throw message) -- Stryker only honours a disable comment immediately above the mutated node's own line, so these were silently not suppressing anything. style.ts and table.ts each had one further boundary this session's earlier pass missed: a text-block header that exactly fills its own packet (style.ts), and a two-byte embedded-subfunction region whose non-zero deletable size overruns it while a zero one wouldn't -- both now have a test isolating the boundary from the same-length, different-value case that previously produced an identical, masking result either way. compound-file.ts's remaining survivors are all genuinely equivalent and are now marked as such: fields archive-codec's own reader never reads back at all (the colour flag, sibling pointers, the minor version, and three declared-count fields the reader instead derives by walking self-terminating chains), values whose bytes are invariant under either endianness (0 and 0xFFFFFFFF), an off-by-one loop bound whose stray write is always overwritten by the very next real write that follows it in this builder's own call order, and a directory entry's own name (the reader identifies the root by object type alone).
wpg.ts's signature scan, characterization walk, colour dispatch, group bookkeeping, and geometry boundaries had no tests isolating their own branches. formula.ts's string-length boundaries and range/cell-reference edges were only exercised by fixtures too short to distinguish a boundary from a downstream failure. image.ts's scanners lacked tests for a lying IEND length, a bare EOI byte, and the restart/SOI standalone boundaries.
…pound-file image.ts's chunk/segment overrun checks and indexOf's loop bound have exact ties or out-of-range reads that converge on the identical refusal regardless of which comparison operator gates them. compound-file.ts's declared-count and colour-flag writes are never read back by archive-codec's reader, and several DIFAT/FAT fill values are byte-order-invariant or immediately overwritten by a later write -- each is annotated with the specific proof rather than left as an unexplained gap.
…valent readCharacterization's geometryAt bound is either the -1 refusal sentinel or at least 2 (cursor + 2), so it can never sit exactly at 0 -- the < vs <= tie is unreachable. Its two callers (readTextBlockFrame, readPrimitiveVector) both need a positive number of further bytes after a successful characterization, so the outer record-walk loop and readPolyline's own count- field check converge on the identical refusal at their own exact ties regardless of which comparison operator gates them.
…e gaps buildRun's strike/fontFamily/sizePt/color spread guards, flushParagraph's abandoned-note reporting, the font-face-change resolution chain (missing packet, wrong packet type, unreadable typeface), page-geometry change reporting, the per-axis margin subgroup branches, style-resolution-depth exhaustion, restoreFormattingSnapshot's own attribute-set loop, and outline numbering's level-already-set guard and depth floor all had no test isolating their own branch from a fixture that also happened to trip a different check.
…pace typecheck The page-geometry-message test destructured only diagnostics from an IIFE returning both document and diagnostics; document was never read, and the unused binding was enough to trip the root-level cross-package typecheck even though the package-local tsc run passed. Removing the wrapper entirely is simpler than keeping an unused return value around.
…g.ts readCharacterization used a -1 sentinel mixed into an otherwise-valid result to mark a transformation-flag refusal, forcing both callers to separately check geometryAt < 0 alongside the normal undefined check; it now returns undefined directly like every other refusal, so callers only ever test one thing. The main record walk's own outer bound was redundant with the loop body's first check, so it collapses to a plain for(;;); the group-closing while loop's redundant length guard is gone since the optional chain already answers false on an empty array; and readPolyline's manual count/point room checks become byteAt's own natural bounds-check throw, caught once around the loop.
…untested decodeBoxWpg and applyBoxGroup's WPG branch (a decoded graphic becoming a nested drawing embeddedObject, a refused wpg1/encrypted/malformed graphic reporting its own exact message, a decoded graphic with no trustworthy box frame, and partial decodes naming their skipped record types) had no test reaching them at all -- an image box whose content packet resolves to a WPG payload is the only path into this code, and nothing constructed one.
readLengthPrefixedByteString built a string its only caller (code 30) immediately discarded, checking only definedness -- it is now skipLengthPrefixedByteString, which validates and advances the cursor without building that string at all, throwing (via sliceAt) rather than returning a sentinel on truncation; the caller wraps it in a try/catch, using sliceAt instead of a raw DataView for the double itself so a cursor over a subarray still bounds-checks correctly. Code 28's and the 48-63 range reference's own "read the end cell only if the start cell succeeded" ternary is gone: readCellNumber's own insufficient-bytes check never advances the cursor, so a redundant second read from the same position is safe, and always attempting it needs no conditional. Codes 48-63 and 64-67 derive their own flag bits with code & 0x0f and code & 0x03 rather than code - 48/64 -- masks on the exact bits those flags are ever read through, not an offsetting subtraction. Cases 41, 42, and 43 share one case-label group instead of two textually identical bodies.
…e keys The header/footer/watermark spread guards on the tree-form section only ever get compared through toEqual against fixtures that either supply the furniture or never check for its absence, so a key present with an undefined value would look identical to the key being omitted entirely. An explicit Object.hasOwn check proves the key itself is absent, not just undefined-valued.
Member
Author
|
Status update: substantial progress since the last measured baseline (66.07%). container/ and most stream/ leaf modules (box, characters, ole, style, table, tokenise) are at 100%. This session added dedicated coverage for wpg.ts's signature scan, characterization walk, colour dispatch, group bookkeeping, and geometry boundaries; formula.ts's string-length and range/cell-reference edges; image.ts's PNG/JPEG scanner boundaries; and read.ts's attribute spreads, note/style/font-face resolution, page-geometry reporting, and the previously-untested box-embedded WPG graphic lift (decodeBoxWpg / applyBoxGroup's WPG branch had zero coverage before this). A concurrent session on this same branch also landed structural refactors removing several disable-comment patterns from image.ts, compound-file.ts, wpg.ts, formula.ts, and table.ts entirely (replacing them with idioms that don't expose a mutable bounds comparison in the first place). Both approaches are compatible -- full local suite ( I could not get a clean final Next step for whoever picks this up: re-run |
…uards readPositionOverride's need(n) helper checked room for a whole sub-block (5 bytes for horizontal positioning, 3 for the other three) ahead of each read, but the guard was never observably different from the throw uint16At/byteAt already raises for an out-of-range read: a buffer too short even for the walk's own reads throws in exactly the place the guard rejected it, and a buffer long enough for cursor to legitimately reach a later bit's own reads is, by construction, already long enough to satisfy every earlier bit's own need, since cursor only ever advances by each bit's full declared width. Removes need() and its four call sites, relying solely on the try/catch walkOverrideBlocks and readContentType already use for the same reason.
…ount guard readStyleBeginBlock's own room guard, ahead of its PID-count read, duplicated the throw uint16At already raises for a packet too short to carry it, so wraps the read in try/catch and drops the explicit length check the same way box.ts and prefix.ts already do. The text-block header's own room guard (afterPids + TEXT_BLOCK_HEADER_SIZE > packet.length) stays: it checks room for all four LONG fields the header declares, even though this function only ever reads three of them, so a packet with room for exactly those three real reads but not the fourth would otherwise slip past the guard and return a begin block read from a too-short packet. Adds a test proving that boundary, plus one proving afterPids is computed by adding the pid list's own byte cost rather than subtracting it.
…face guards readGeneralWpTextBlocks' sizesEnd > bytes.length guard checked room for every size word its own loop was about to read, but the guard's threshold (4 + blockCount * 2) is exactly the offset the loop's last iteration already requires, so a bytes.length short of it makes that iteration throw in precisely the place the guard rejected it. Removes the guard and the now-unused sizesEnd variable, and wraps the block count and first block offset reads in try/catch for the same reason box.ts and style.ts already do. readTypefaceName's own Math.min(nameLength, packet.length - TYPEFACE_NAME_OFFSET) bound is redundant for the same reason ole.ts's was: decodeWordString already stops the moment it runs off the real end of packet, regardless of how many words it is asked for, so nameLength alone is exactly as safe a bound as intersecting it with the packet's own remaining length. Its own too-short-for-the-length-word guard is replaced by a try/catch around the read, matching the same pattern.
…up header guard readDocumentSummary's own room guard ahead of each group's header read duplicated the throw uint16At already raises for a packet too short to carry it. Wraps the three header reads in try/catch and ends the walk the same way the size check just below it already does, rather than checking length explicitly first.
fileSize > documentAreaOffset && fileSize <= bytes.length disagreed from a strict < only when fileSize === bytes.length exactly, and at that exact boundary both branches returned the same number, so no input could ever observe which comparator ran. Replaces the upper bound with Math.min(fileSize, bytes.length), which carries the identical fallback without emitting a comparison whose boundary case has no effect.
applyWpdStandardEncryption's empty-password guard already rejects before its loop ever runs, so the loop's own undefined check on normalised[relative % normalised.length] (forced by noUncheckedIndexedAccess) was unreachable from every real caller and its own thrown message had no test able to observe it either way. Extracts the cyclic lookup into passwordByteAt, exported for this package's own tests, so the throw is proven genuine by a direct test supplying an empty array instead of sitting inlined and unreachable.
…string read ole.ts's OLE 2 stream name has no separate length prefix of its own to bound the read to -- it wants "the rest of this buffer, read as a word string" -- so it computed (bytes.length - payloadOffset) / 2 as a maxWords bound, but decodeWordString already stops at the first null word or the moment a read runs off the buffer's real end regardless of how large a maxWords it is given, making any such computed bound unobservable. Adds UNBOUNDED_WORDS, a named sentinel for exactly this "no separate bound" case, and uses it in place of the arithmetic.
readCellFill's own background shade is always a Uint8Array byte read (0-255), so nearestPercentType's own out-of-range guard was unreachable from every real caller and its thrown message had no test able to observe it either way. Exports the function for this package's own tests and adds a direct call with shade 256 to prove the message genuine, rather than leaving it inlined and unreachable.
…'s root entry archive-codec's own reader (src/cfb/read.ts) reads the root directory entry directly as entries[0] and never walks into it through the directory tree's own name/nameLength validation -- only a non-root entry, reached via root.child, ever has its name read back. Splits writeDirectoryEntry into writeDirectoryEntryFields (every entry) and writeDirectoryEntryName (called only for the one real, non-root stream), so the root's own call has no name argument, and therefore no unread string literal, to carry at all. Also replaces the separately materialised, zero-padded miniStream byte array with a plain numeric pool size: archive-codec's reader bounds the mini-stream pool to the root entry's own declared size, so that size must stay rounded up to a whole mini sector even though the real stream data inside it is shorter, but the file's own backing buffer already starts fully zeroed, so writing the caller's raw, unpadded stream produces the identical bytes a separate padded copy would have. Adds a test proving every unused header DIFAT entry is still padded with FREESECT rather than the buffer's own zero fill -- archive-codec's reader walks all 109 header entries unconditionally and would otherwise misread a zero as 108 more, bogus FAT sectors.
…ange reads bytesMatchAt read through plain bracket access, which answers undefined rather than throwing for a position too close to the buffer's own end -- safe, but it made every "does this still fit" bound around it (indexOf's own give-up check, scanImagePayload's outer loop bound, its two per-signature room checks) unobservably redundant with that silent false, since going past the true bound could never produce anything but a harmless extra no-op attempt. Switches bytesMatchAt to byteAt, which throws, and restructures both callers around that: indexOf drops its own give-up check entirely, since its one real caller (scanJpeg's entropy-data search) already wraps the whole walk in a try/catch that answers undefined identically either way. scanImagePayload wraps only the longer PNG_SIGNATURE check in its own try/catch (JPEG_SOI, checked second, is left unguarded so its own throw -- the shortest either signature could ever need -- correctly ends the whole scan once reached), and drops the separate arithmetic bound in favour of letting the reads bound themselves. Also exports bigEndianUint32At/bigEndianUint16At for a direct test proving each byte's own place value: every PNG/JPEG fixture this module's own tests otherwise construct keeps chunk/segment lengths small, so a wrong sign or operator on an upper-byte term would otherwise go unobserved (0 combined any way is still 0).
Real gaps, closed with tests that isolate each boundary: - readCountField's 1-byte/extended split at exactly 0xFE. - readCharacterization's edit-lock descriptor overrunning a record with no Object ID to catch it first, misreading the next record's own bytes as geometry. - readWpgRectangle's square-corner threshold for rx and ry independently, rather than only ever exercising both at once. - a rounded rectangle actually carrying a stroke, not just every existing fixture's own absent one. - clamping ry's own corner radius to half the frame's height, alongside the existing rx/width assertion. - a Start WPG record of exactly 13 bytes proceeding to its own extent check, rather than surviving via a second, later Start WPG record the correct code's whole-function return never even reaches. - a Text Block whose own frame failed to resolve still swallowing its declared members. - a stroke key's own absence, not just an undefined value toEqual cannot tell apart from a present-but-undefined one. Genuinely redundant checks, removed and replaced with the throw their own reads already raise, or with the sibling check already covering them: readCharacterization's flags-word and Object-ID room guards (matching every other throw-based read in this package), its own trailing geometryAt bound (every one of its callers already refuses identically past their own record's end), the record walk's leading cursor bound, and the top-level recordStart validation's upper half (the walk's own leading read already refuses identically past the buffer, verified by removing it and confirming the full suite still passes) and its lower half (the fixed 26-byte header can never assemble into a record the critical fields it already validates don't already rule out). Also drops readCharacterization's now-redundant recordEnd parameter, always passed as bytes.length by both callers.
Diagnostic message assertions, added to tests that already trigger each code but never checked its exact text: MergeFieldSpansParagraphs, TableFlattened, OutlineNumberRegenerated, HeaderFooterDropped (both call sites), NoteDropped, MergeCodeDropped, BoxDropped, BoxContentUnresolved, CrossReferenceFlattened, UnmappedCharacter, and OleObjectDropped. Real behavioural gaps, closed with new tests: flushParagraphIfContent flushing when pending text is empty but a run has already been split off it; a paragraph's heading level staying with the first structural style it captured rather than a second one opening later in the same paragraph; a cell's own alignment landing only on its paragraph blocks, not a page break sharing the cell; TableFormulaUnresolved and CellFillBlended not firing for a plain cell with neither; and a plain paragraph, cell, and row carrying none of their own optional keys at all when nothing sets them. Also simplifies effectiveStyle to Array.prototype.findLast, removing a hand-rolled reverse index walk whose own arithmetic had no input that could ever observe a wrong starting index: any position a later bit's own read could legitimately reach was already within the correct range regardless.
readEmbeddedSubfunctions extracted CELL_FORMULA_SUBFUNCTION's data as everything between its two code gates, which still included the leading and trailing two-byte length words either side of the tokenised formula. readTableFormula parses from offset 0 with no length-word skip of its own, so a table cell's formula token stream always began with a length byte instead of a real token, and could essentially never resolve. Slice out just the token region (offset 3, spanning the declared length) when the subfunction is CELL_FORMULA_SUBFUNCTION, matching stream/formula.ts's own comment that its input already arrives "isolated from its own six-byte length framing".
…gle-byte formatting functions Add tests exercising code paths in read.ts that no test previously reached: the "ignore" End-of-Line mapping (subfunction 0, Beginning of File), which has no single-byte spelling and is reachable only through the group 0xD0 multi-byte form; the "hardEndOfColumn" mapping's own diagnostic message, asserted verbatim; and five single-byte functions (invisible return in line, auto-hyphen at end of line, dormant hard return, soft end of center align, hard end of center align), each asserted against the specific text or paragraph-split behaviour its own WPFF-quoted comment describes. Also add a test proving the End of Text to Skip handler clamps skip depth at zero: an extra, unmatched End of Text to Skip must not drive the depth negative, since a later Start would then raise it to zero instead of one and let that region's own text leak into the document instead of being dropped.
…gaps effectiveStyle: proves the innermost-scope walk skips past a transparent style with no structural semantics to reach a structural one further out, not just the reverse nesting already covered. appendText: proves a paragraph's list membership, once captured from its own first style, survives a later style in the same paragraph that sets a heading instead. Tables: covers a well-formed cell never reporting a truncated attribute list; a table definition that closes with no rows at all being dropped; a fixed row height set by an earlier cell surviving a later cell in the same row stating none; Table Off still closing a cell whose pending text is empty but whose runs or already-flushed blocks are not; an already-open table closing (and its enclosing paragraph flushing) when a new Table Definition arrives; and a Table Column function after Define Table End being ignored. Also asserts a plain cell's own paragraph carries no `alignment` key at all. Font face changes: proves the packet-type check actually gates the read (a non-descriptor packet whose bytes would decode as a valid typeface must not be read as one); a failed change leaves a previously set font family in place rather than clearing it; a change splits the run so earlier text keeps its own (absent) font family; and the missing-prefix-packet diagnostic carries its exact message. Page geometry: proves a portrait form reports no landmark orientation diagnostic; a page geometry change is reported once across more than one later change, and not at all when the same value repeats; and a page or column margin function whose subgroup names neither margin is not misapplied as the other one. Single-byte functions: proves a code with no named case in applySingleByteFunction's own switch falls through its default case, contributing neither characters nor structure. Table formula: strengthens the resolved-formula test to also prove no unresolved-formula diagnostic fires once the formula does resolve.
…boundary tests pendingListLevel is only ever compared against undefined, never enumerated or spread conditionally on its own presence, so the separate `level !== undefined` guard before setting it was provably redundant: assigning it an undefined level when the level itself could not be read is indistinguishable from leaving it untouched. Replaced the two-part condition with `??=`. Also adds tests proving the style resolution depth guard refuses exactly the boundary-crossing (17th) recursion rather than the one after it, that the depth counter returns to its starting value after each style scope rather than accumulating across sibling (non-nested) scopes, and that an unrelated function sharing the paragraph-number-display group does not end suppression it did not start.
Note markers: proves a genuinely empty marker falls back to a generated numeral computed from notes already carried plus one (not minus one), and that a real, non-empty, multi-run marker wins over that fallback rather than being replaced by it -- joined with no separator across its runs. Note body reading: adds the exact could-not-read diagnostic message for a note whose body packet exists but is not General WP Text, alongside the already-covered could-not-resolve case for a missing prefix ID. Merge fields: proves a field instruction split across more than one run joins with no separator, not just a single-run instruction that can't distinguish a join from a no-op. plainTextOf: proves it reads only paragraph blocks -- a page break folded in between two paragraphs is skipped entirely rather than read as one missing a `runs` field -- and joins runs within a paragraph with no separator and separate paragraphs with a newline.
decodeBoxWpg: proves the child-packet walk keeps trying subsequent Graphics Cached File Data children after one fails to decode as WPG, rather than stopping (successfully or not) at the first. applyBoxGroup: proves a decoded WPG's own drawing block is preceded by a flush of whatever paragraph text came before the box, and that its skipped-record diagnostic names more than one record type joined by ", " rather than concatenated with no separator. decodeWpgGraphic's own shapes array: proves a WPG carrying a Text Block/Text Data pair alongside a vector primitive surfaces both its shapes and its vectors, not just whichever the previous tests happened to exercise.
applyBoxGroup: proves a linked-text box (content type 2) is lifted the same way a plain text box is, not just falling through some other path; adds the exact box-content-unresolved and box-frame-unresolved diagnostic messages for both the image and text-like branches when the named packet cannot be read as the right type or the box states no trustworthy width and height; and proves preceding text is flushed into its own paragraph before an image block or a text-like box's own embedded document, rather than merged into it.
decodeSingleByteCharacter's own domain (1..127) is exhaustively covered by its shorthand table (1..32) plus its literal ASCII range (33..127), and the tokeniser only ever mints a "character" token for a byte already restricted to exactly that domain. Verified this by enumerating every byte in 1..127 in a new characters.test.ts case: none decode to undefined. Replaced applyToken's graceful "report and substitute U+FFFD" handling for this case -- which could never actually run -- with assertDefined, a small local type-guard that throws loudly if the proven invariant is ever wrong instead of silently working around a real possibility. assertDefined is exported and directly tested, since nothing in a real document can otherwise exercise its own undefined branch. Also adds tests for: an unrelated paragraph-group subfunction not being misread as a justification change, and a variable-function group with no named case contributing nothing.
readWpd: simplifies away an early "nothing to add" return that produced a value indistinguishable from the general path's own result once attachments and notes are both empty -- a shallow copy of assembled has the same own keys and values as assembled itself, and nothing afterwards mutates it. Adds a test proving readWpd actually threads its own password option through to its own openWpdDocument call, and a test proving its own header/footer/ watermark spreads fire when the document declares them, not just that they stay absent when it doesn't -- the existing absence-only test could never distinguish a guard that always evaluates false from one that reads the real condition. readMetadata: proves the Extended Document Summary packet is found by its own packet type, not just whichever packet sits first in the index. Table end-of-stream handling: proves an unfinished row's own already-closed cells still become a real row when the document area ends before any row-closing boundary, rather than vanishing along with the whole table (which only happens when it holds zero rows). Character and fixed-function fallbacks: proves the visible U+FFFD replacement character actually appears in a run's text, not just that the diagnostic fires, and that a fixed-length function code with no named meaning (Undo, 0xF1) contributes neither a character nor an attribute change.
Font size and colour changes: proves a change splits the run so earlier text keeps its own (absent) size or colour, mirroring the same split already proven for font face changes; proves a font size change is ignored when its non-deletable data is too short to hold the size word, and when the size decodes to exactly zero points. applyCharacterGroup: proves a subgroup this reader names no case for falls through its default case, contributing nothing.
…d.ts applySingleByteFunction, applyCharacterGroup, and applyVariableFunction each ended their own switch with a default case whose only statement was `return;` -- redundant since it was already the switch's last case, so falling through it reaches the enclosing void function's own end with identical behaviour. Removed the three now-pointless returns and kept each case's own explanatory comment. Also extracts assertDefined's one real call site's message into UNREACHABLE_CHARACTER_MAPPING_MESSAGE, a fixed exported constant rather than a per-byte template literal, so its exact wording stays testable on its own terms even though no real document byte can ever reach the throw it guards. New tests close the remaining gaps: a hard return not falling through into the next case's column-break diagnostic; a font size change read correctly at exactly the size word's own length (not one byte short of it); an unrecognised box content type never treated as text-like even when a readable General WP Text packet happens to sit at its prefix ID; a fixed-length function's own attribute-code gate proven with a byte that would visibly turn bold on if misread; an unrelated D7 subfunction not abandoning a footnote already open; and the exact could-not-resolve and could-not-read header/footer/watermark diagnostic messages, including that neither failure leaves a header key behind.
A fixed-length function code the reader treats as having no attribute meaning still reached decodeAttributeByte if its own gate were ever removed, and since its own code is neither ATTRIBUTE_ON nor ATTRIBUTE_OFF, misreading it would always take the "off" branch -- silently deleting whatever active attribute its data byte happens to number, regardless of which real code turned that attribute on. A data byte of 0 (no active attribute to delete) couldn't observe this; turning bold on first, so the deletion has something real to remove, does.
Mearman
marked this pull request as ready for review
September 12, 2026 20:32
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. |
Mearman
enabled auto-merge (rebase)
September 12, 2026 22:12
Contributor
|
🎉 This PR is included in version 3.5.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Second package in the wider push to get every package's own mutation score to 100% (see #1244 for excel-number-format, the first). Work in progress: measured baseline 66.07% (breakThreshold 64), currently raising container/ and several stream/ modules toward 100% before tackling the larger read.ts, stream/wpg.ts, stream/formula.ts, and stream/image.ts.
Same methodology as excel-number-format: isolate each boundary condition with its own narrow test rather than relying on integration coverage that happens to also pass under a mutated branch, simplify genuinely redundant guards (an unsigned read that can never be negative, a length check whose only reachable boundary already converges on the same result via a neighbouring check), and mark true equivalents with a
// Stryker disable next-line <Mutator>: <reason>comment plus a written proof rather than chasing an unwritable test.Not ready for review yet -- breakThreshold stays at 64 until every remaining survived/no-coverage mutant is resolved and the score is confirmed stable across repeated runs.
Test plan
pnpm test/pnpm typecheck/pnpm lintfor wpd-codec, green so farnpx stryker run stryker.config.tsat 100.00%, confirmed stable across repeated runs