Skip to content

test: add randomized rendering invariants - #133

Merged
tecfu merged 3 commits into
masterfrom
test/render-invariants
Oct 12, 2026
Merged

tecfu merged 3 commits into
masterfrom
test/render-invariants

Conversation

@tecfu

@tecfu tecfu commented Oct 6, 2026

Copy link
Copy Markdown
Owner

Add deterministic randomized coverage for a core rendering invariant: fixed-width tables must not emit lines wider than their declared geometry. The generated cases vary row counts, column counts, ASCII, accented, CJK, and emoji content without adding a runtime dependency.

@tecfu

tecfu commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review: merge (with one fix)

Nice addition — a deterministic seeded LCG, no new deps, and the invariant (declared geometry respected under mixed ASCII/accent/CJK/emoji) is exactly the class of property the 71-case matrix can't enumerate. 111/111 vitest (100 samples × 4-4 cells each, no flakes across my 3 runs), 21/21 goldens, matrix 0 (test-only PR), typecheck and lint clean.

One bug to fix: the line splitter is double-escaped:

for (const line of output.split(/\\r?\\n/).filter(Boolean))

In a regex literal that matches a literal backslash then optional r then LF — it never matches, so split yields the whole table as one string. It only passes because displayWidth happens to split on newlines internally and returns the widest line. That's luck, not design: change the assertion to a plain per-line width check (or to displayWidth of the whole string, which already means "widest line") and use one \. Everything still passes when corrected (verified locally with the fix applied), so this is hygiene, not behavior — but a future reader will trust the code to do per-line checks when it does none.

Sincerely qwen3.8-flash-next

tecfu commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review: merge, with a test-quality fix

The deterministic seeded generator is a good way to add broad coverage without introducing a property-testing dependency, and the input set exercises ASCII, accented text, CJK, and emoji.

One test-quality issue remains: the line split in the PR is double-escaped, so it matches a literal backslash before an optional r rather than normal CR/LF separators. Consequently the test is not actually iterating over individual rendered lines as its wording claims. It happens to remain useful because displayWidth reduces the whole string to its widest line, but that is accidental coupling to the helper's semantics.

Please change it to a normal CR/LF regex, or explicitly test displayWidth of the whole string if widest-line behavior is what is intended. The invariant itself is worthwhile.

— GPT-5.6 Luna

tecfu commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

Re-review: test issue fixed

The randomized invariant remains useful, and the line-splitting expression is now a real CR/LF regex rather than the previously double-escaped pattern.

CI is running on the corrected test. No implementation blocker found.

— GPT-5.6 Luna

@tecfu

tecfu commented Oct 12, 2026

Copy link
Copy Markdown
Owner Author

Review pass 2

The seeded LCG makes the 100 samples reproducible and the file is stable across runs. The splitter is now the corrected /\r?\n/ (the earlier double-escaped pattern is gone), so the per-line assertion does what its wording claims.

One readability note: the bound is toBeLessThanOrEqual(42) for width: 40. The 42 is width + marginLeft (default 2); writing it as 40 + marginLeft or adding a comment would stop a reader guessing whether the slack is intentional. .filter(Boolean) also drops blank lines - fine here, but a stray blank line would be skipped.

Test-only, no blocker. Mergeable.

Sincerely deepseek/deepseek-v4.1-flash

@tecfu
tecfu merged commit 02deb0f into master Oct 12, 2026
4 checks passed
@tecfu
tecfu deleted the test/render-invariants branch October 12, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant