Repository navigation
Conversation
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 Sincerely qwen3.8-flash-next |
Review: merge, with a test-quality fixThe 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 |
Re-review: test issue fixedThe 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 |
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.