test: property-check table formatter alignment under unicode content#595
test: property-check table formatter alignment under unicode content#595ndycode wants to merge 3 commits into
Conversation
Four fast-check properties over buildTable/buildTableRow with cell content mixing ASCII, CJK, and emoji so widths are exercised in display columns rather than UTF-16 units (ui-02): - every line of any table (header, separator, all rows) has exactly the layout's display width, for any column set, alignment mix, missing cells, or extra cells beyond the column count - content that fits is preserved verbatim with padding on the declared side - overflowing content truncates to a prefix of the original plus an ellipsis, never exceeding the column width even when a wide glyph cannot fill the final column - zero-width columns render empty and never leak an ellipsis that would desync the row from the header layout Companion to the property suites in #574/#575/#592-#594. https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
📝 WalkthroughWalkthroughproperty test suite validates ChangesTable formatter property tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes flagged observations
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/property/table-formatter.property.test.ts`:
- Around line 77-98: Add a symmetric property test for right-aligned truncation:
copy the existing test block that uses buildTableRow, arbCellText and
displayWidth but set columns: [{ header: "h", width, align: "right" }], strip
leading padding with visible = row.replace(/^ +/, "") (instead of trimming
trailing spaces), and assert the same properties: visible endsWith("…"), the
original value startsWith(visible.slice(0, -1)), and displayWidth(visible) <=
width to verify truncation + ellipsis is correct for right alignment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b0ba59fb-bdf9-4f95-b979-75cf5197b9fd
📒 Files selected for processing (1)
test/property/table-formatter.property.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,js,mts,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only with
"type": "module"in package.json; Node >= 18.17 required
Files:
test/property/table-formatter.property.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not use
as any,@ts-ignore, or@ts-expect-errorTypeScript assertions
Files:
test/property/table-formatter.property.test.ts
**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
OAuth callback server uses port 1455; do not hardcode OAuth ports—use existing constants/helpers
Files:
test/property/table-formatter.property.test.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (README.md)
**/*.{js,ts}: Use JSON format for machine-readable output in diagnostic and reporting commands (status, check, report, monitor, why-selected)
Implement interactive terminal dashboard with hotkeys (Up/Down for navigation, Enter for selection, 1-9 for quick switch, / for search, ? for help, Q for back)
Support device authentication flow via --device-auth flag and manual OAuth callback paste fallback via --manual flag for headless environments
Run npm version check during normal forwarded Codex startup and print upgrade notices only on interactive TTY or when CODEX_MULTI_AUTH_DEBUG=1 is set
Files:
test/property/table-formatter.property.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
test/**/*.test.ts: Write Vitest test suites with globals enabled (describe, it, expect)
Maintain 80%+ coverage threshold across statements, branches, functions, and lines
Use removeWithRetry() for Windows filesystem cleanup instead of bare fs.rm to handle EBUSY, EPERM, and ENOTEMPTY errors
Do not rely on dist/ in tests; use source files instead
Do not skip tests without justification
Relax lint rules for test files as configured in eslint.config.js
Files:
test/property/table-formatter.property.test.ts
test/**/property/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
Use fast-check for property-based testing in test files
Files:
test/property/table-formatter.property.test.ts
test/**
⚙️ CodeRabbit configuration file
tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.
Files:
test/property/table-formatter.property.test.ts
**
⚙️ CodeRabbit configuration file
**: # PROJECT KNOWLEDGE BASEGenerated: 2026-04-25
Commit: a87e005
Validated: 2026-06-10 against commit 98d9819 (repo audit; claims re-checked against the tree, content not regenerated)
Branch: main
Package version: 2.3.0-beta.3OVERVIEW
codex-multi-authis a Codex CLI-first OAuth account manager and optional forwarding wrapper for the official Codex CLI. The installedcodex-multi-authentrypoint handles account-management commands locally,codex-multi-auth-codexforwards official Codex commands through this package's wrapper when explicitly used, and runtime rotation can route live Responses traffic through a localhost account-rotation proxy by default. The plugin-host entrypoint remains exported for compatibility, but the primary product surface is the account manager, optional wrapper, storage, runtime proxy, and repair tooling.STRUCTURE
./ ├── scripts/ │ ├── codex.js # codex-multi-auth-codex wrapper, official CLI forwarder, shadow CODEX_HOME/runtime proxy setup │ ├── codex-multi-auth.js # standalone package CLI entrypoint │ ├── codex-routing.js # auth command and compatibility alias routing │ ├── codex-bin-resolver.js # official Codex binary discovery │ ├── codex-app-router.js # persistent localhost router for packaged Codex app bind │ └── codex-app-launcher.js # reversible user-level app launcher routing helper ├── index.ts # optional plugin-host runtime entry ├── lib/ # core runtime logic (see lib/AGENTS.md) │ ├── auth/ # OAuth flow, PKCE, callback server │ ├── runtime/ # Codex CLI/app integration helpers, app bind, live sync, runtime observability │ ├── request/ # request transform, SSE, failover, backoff │ ├── storage/ # path resolution, migrations, backups, restore, import/export │ ├── codex-cli/ # Codex CLI state sync and writer helpers │ ├── codex-manager/ # command modules and...
Files:
test/property/table-formatter.property.test.ts
🧠 Learnings (2)
📚 Learning: 2026-06-04T06:14:18.093Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/scheduling-strategy-config.test.ts:1-1
Timestamp: 2026-06-04T06:14:18.093Z
Learning: In ndycode/codex-multi-auth, do not flag explicit imports from "vitest" (e.g., describe, it, expect, beforeEach/afterEach, etc.) in test files as issues—even if the Vitest config sets `globals: true`. The repo’s established convention is to keep these imports for consistency with neighboring tests; removing them would make files outliers.
Applied to files:
test/property/table-formatter.property.test.ts
📚 Learning: 2026-06-04T06:14:24.975Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/runtime-rotation-proxy.test.ts:2478-2491
Timestamp: 2026-06-04T06:14:24.975Z
Learning: In ndycode/codex-multi-auth test files (e.g. `test/*.test.ts`), when creating V3 storage fixtures for accounts, it’s an intentional convention to use `as never` for deliberately minimal stored-account objects that only include `refreshToken`, `addedAt`, and `lastUsed`. Do not treat `as never` here as a type-safety problem: optional/other fields are expected to be populated by the runtime during execution, and the cast is used solely to keep the fixture minimal and consistent across existing tests.
Applied to files:
test/property/table-formatter.property.test.ts
🔇 Additional comments (5)
test/property/table-formatter.property.test.ts (5)
1-9: LGTM!
11-33: LGTM!
35-57: LGTM!
59-75: LGTM!
100-109: LGTM!
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…align Greptile P1: stripping leading spaces on a right-aligned row conflated alignment padding with spaces that belong to the truncated content (latent spurious failure under a different fast-check seed - e.g. ' a<CJK>' at width 3 right-aligned). Prefix fidelity now checks the left-aligned rendering of the same value, where the cell starts at column 0, while the right-aligned row keeps its width and trailing- ellipsis assertions. Verified at FAST_CHECK_NUM_RUNS=1000. Greptile P2: arbColumn and the fit property now generate undefined align too, exercising formatCell's default-left branch. https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
2 participants
Summary
lib/table-formatter.ts, the CLI table renderer whose alignment guarantee (ui-02) is "measure in display columns, not UTF-16 code units". Property generation is the right tool here: the interesting inputs are exactly the awkward ones — CJK doubles, emoji, wide glyphs straddling a truncation boundary, rows with missing or extra cells.What Changed
New
test/property/table-formatter.property.test.ts(4 properties overbuildTable/buildTableRow, cell content drawn from a mixed ASCII/CJK/emoji alphabet):left/right), padding computed in display columns.formatCelldocuments).formatCell).No SUT bugs found; the renderer's geometry held for every generated case.
Validation
npm test -- test/property/table-formatter.property.test.ts test/table-formatter.test.ts— 15/15 (new 4 + existing 11 untouched)npm run typecheck(also via pre-commit hook)npx eslint test/property/table-formatter.property.test.ts --max-warnings=0Docs and Governance Checklist
Risk and Rollback
test/property/suites (explicit vitest imports, plainfc.assert,fc.preto partition fit vs overflow).https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
Generated by Claude Code
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
adds a fast-check property suite for
lib/table-formatter.tscovering four invariants: lockstep line width, fit-preservation with padding, truncation to a prefix-plus-ellipsis, and zero-width column emptiness. both issues from the prior review are resolved — the right-aligned overflow prefix check now uses a left-aligned side-channel to avoid stripping content spaces, andarbColumnnow generatesundefinedalignment viafc.optionto exercise the default-left branch.expectedLineWidthformulaΣwidth + (n−1)correctly mirrorsArray.join(" ")even for zero-width columns, verified against the sut.leftRow), stripping only trailing padding withreplace(/ +$/, "")— the fix to the previously flagged spurious-failure risk.formatCellshort-circuit that prevents an ellipsis from inflating the row width by one column.Confidence Score: 5/5
additive test file with no production code changes — safe to merge.
the change is a single new test file; it makes no edits to production code, no filesystem i/o, no async/concurrency, and no token or credential handling. both issues raised in the prior round are addressed correctly. the four property invariants are logically sound against the sut.
no files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[arbCellText\nASCII / CJK / emoji chars] --> B[arbColumn\nheader · width 0-10 · align left/right/undefined] B --> C[arbOptions\n1-5 columns] C --> P1[Property 1: Lockstep layout\nevery line == expectedLineWidth] A --> P2[Property 2: Fit preservation\ndisplayWidth value <= width\nrow == pad+value or value+pad] A --> P3[Property 3: Truncation contract\ndisplayWidth value > width\nrow ends with … · prefix check via left-aligned side-channel] A --> P4[Property 4: Zero-width column\nrow == empty string] P1 --> SUT[buildTable / buildTableRow\nlib/table-formatter.ts] P2 --> SUT P3 --> SUT P4 --> SUT SUT --> FMT[formatCell\ntruncateToWidth / displayWidth\nlib/ui/display-width.ts]Reviews (2): Last reviewed commit: "test: validate overflow prefix on the le..." | Re-trigger Greptile