Skip to content

fix(shared): preserve final quoted empty CSV records - #11425

Merged
maria-rcks merged 1 commit into
pingdotgg:mainfrom
Lucenx9:fix/preserve-empty-delimited-record
Sep 24, 2026
Merged

maria-rcks merged 1 commit into
pingdotgg:mainfrom
Lucenx9:fix/preserve-empty-delimited-record

Conversation

@Lucenx9

@Lucenx9 Lucenx9 commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

What changed

Keep the final quoted empty record in CSV and TSV table previews when the file has no trailing line ending. The parser now tracks the start of the source row instead of using decoded cell content to decide whether a final record exists.

Why

For name\r\nAlice\r\n"", the preview drops the last row and reports complete content. Adding a final CRLF makes that same row appear. A final record without a line ending is valid CSV, including a quoted empty field. See RFC 4180, section 2.

flowchart LR
    A[Final quoted empty record without a line ending] --> B[Before: empty decoded buffers]
    B --> C[Row omitted]
    A --> D[After: source row still present]
    D --> E[Row retained]
Loading

This changes only the shared parser and its focused tests. The web and desktop attachment and workspace table previews use it. Mobile does not currently render this table component. File contents, source view, wire contracts, product defaults, and preview limits are unchanged.

Verification

  • The six new regression tests fail on the base commit. All nine tests pass with the fix.
  • vp test run packages/shared/src/delimitedPreview.test.ts --maxWorkers=1 --no-file-parallelism
  • vp lint packages/shared/src/delimitedPreview.ts packages/shared/src/delimitedPreview.test.ts
  • vp fmt --check packages/shared/src/delimitedPreview.ts packages/shared/src/delimitedPreview.test.ts
  • tsc --noEmit -p packages/shared/tsconfig.json passes. Existing Effect suggestions remain in unrelated files.
  • 504 generated round-trip cases pass across both delimiters, LF/CRLF/CR, BOM, quoted line breaks, empty fields, and the 100-row limit.
  • Fallow review scoped to packages/shared, one thread: low risk, no new APIs, dependencies, or diagnostic suppressions. No structural decisions flagged. Consumers outside that package were checked in source.

UI changes

Captured in the running T3 web app with Chromium at 2× device scale. The same draft attachment contains Participant\r\nAlice\r\nBob\r\n"", without a final newline. The before capture uses the parser from b1e223e2b0; the after capture uses caaf8daa44. No styles or components were changed for the captures.

Before: two data rows. The final empty record is missing.

Before: only Alice and Bob appear

After: three data rows. The empty row below Bob is retained.

After: the empty row below Bob is retained

Source view showing the quoted empty fourth line. Switching back to the table preserves all three data rows.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • No animation or timing changes, so a video is not applicable

Model: GPT-5 family. Harness: Codex.

Summary by CodeRabbit

  • Bug Fixes

    • Improved delimited data previews for BOM-prefixed input, empty records, trailing line endings, and quoted empty values.
    • Corrected row-limit and truncation handling to ensure previews include the appropriate remaining data.
  • Tests

    • Added coverage for empty input, BOM handling, row limits, trailing line endings, quoted empty records, and truncation behavior.

Copilot AI lite review requested due to automatic review settings September 12, 2026 14:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-12T14:08:58.185780Z caaf8da PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 12, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at caaf8da

Macroscope's review found this PR approvable — This focused parser fix preserves final quoted empty CSV/TSV records without changing parsing limits or introducing new capabilities. Regression tests cover the affected edge cases and confirm that empty input and trailing line endings remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 9da9efea-e684-4246-a27d-30b238a61656

📥 Commits

Reviewing files that changed from the base of the PR and between b1e223e and caaf8da.

📒 Files selected for processing (2)
  • packages/shared/src/delimitedPreview.test.ts
  • packages/shared/src/delimitedPreview.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

parseDelimitedPreview now tracks the start of each row, including BOM-prefixed input, and uses remaining text to decide whether to emit a trailing row. Tests cover empty records, line endings, delimiters, BOM input, row limits, and truncation.

Changes

Delimited preview parsing

Layer / File(s) Summary
Row tracking and trailing-row validation
packages/shared/src/delimitedPreview.ts, packages/shared/src/delimitedPreview.test.ts
parseDelimitedPreview tracks row boundaries and emits trailing rows only when input remains. Tests cover comma and tab delimiters, quoted empty records, empty input, line endings, BOM input, row limits, and truncation.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: chrisdeeming

Merge Risk: ⚪ Minimal · up to 4e262

The parser change preserves final quoted empty records while retaining empty-input and trailing-line-ending behavior. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main fix: preserving final quoted empty CSV records. It is concise and specific.
Description check ✅ Passed The description explains what changed, why the fix is needed, affected consumers, verification steps, and UI evidence. It covers the required template sections and confirms that the change is focused.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@greatitself

Copy link
Copy Markdown

Independent validation of caaf8daa448d3eb4a9b45d9eb97d25363cff020c (merge-base b1e223e2b0d8).

Generated 2,400 valid CSV/TSV inputs with an independent serializer: 1–100 rows, LF/CRLF/CR separators, optional final separator and BOM, and quoted cells containing delimiters, quotes, newlines, and Unicode. Every input ends with an explicitly quoted empty record. All 2,400 matched the expected table on this revision; the merge-base lost the final row in 1,200 cases.

Reproduction scripts and recorded results — first.mjs; the numbered metadata file pins this PR’s tested revisions.

Model: GPT-6. Harness: Codex.

@Lucenx9
Lucenx9 force-pushed the fix/preserve-empty-delimited-record branch from caaf8da to 4e26213 Compare September 12, 2026 18:18
@maria-rcks
maria-rcks merged commit f1add18 into pingdotgg:main Sep 24, 2026
22 checks passed
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 24, 2026
## What's Changed
* fix(ci): shard release tests like pull request CI by @juliusmarminge in pingdotgg/t3code#13321
* fix(web): show previous worktree branch on second line by @Yash-Singh1 in pingdotgg/t3code#13314
* fix(providers): restore compatibility ranges for every harness by @juliusmarminge in pingdotgg/t3code#13328
* fix(preview): use the visible browser for new agent sessions by @Bil0000 in pingdotgg/t3code#13064
* fix(server): stop replaying old agent alerts on restart by @juliusmarminge in pingdotgg/t3code#13340
* fix(web): use a brain icon for the effort dropdown by @t3-code[bot] in pingdotgg/t3code#13309
* fix(desktop): SnapShot shortcut helper no longer adds a Dock icon on macOS by @Gigioxx in pingdotgg/t3code#13286
* fix(web): composer chip rings no longer clip at the editor edge by @flamboh in pingdotgg/t3code#13301
* fix(web): switches announce their real state to screen readers by @Leos-Khai in pingdotgg/t3code#11580
* fix(shared): preserve final quoted empty CSV records by @Lucenx9 in pingdotgg/t3code#11425
* feat(web): add an interactive 3D device workspace by @juliusmarminge in pingdotgg/t3code#12787


**Full Changelog**: pingdotgg/t3code@v0.0.43-nightly.20260923.2173...v0.0.43-nightly.20260924.2187

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.43-nightly.20260924.2187
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 24, 2026
Merges `pingdotgg/t3code` up to `78af372cf`: 27 commits on top of base
`aca3c87cd`. It branches from `main`; no earlier merge PR was open.

## Resolution

Six files conflicted, and each was resolved with the verdict
`preflight.mjs` printed:
- `AGENTS.md` (decide): the fork's Taste prose stays. Upstream's rule
change is folded into it: `shadcn/no-restyle` now fails lint, and a look
that belongs to one feature stays in that feature's component.
- `GitActionsControl.tsx`, `ProjectScriptsControl.tsx`,
`settings/ProjectActionsList.tsx`, `settings/SettingsSidebarNav.tsx`
(converged): these take upstream's markup that no longer restyles `ui`
components. The `FEATURES.openInEditor` gate, the
`editable`/`taskScoped` gates and the personal/admin settings-nav split
are re-applied at their new anchors.
- `pnpm-lock.yaml` (theirs): upstream's copy, re-derived with
`install.mjs`.

`projectScriptEditor.tsx` merged without a conflict but had no
path-policy entry, so it now has one: `project-script-editor`.

**Changes needed after the merge.** Upstream pingdotgg#13210 made
`shadcn/no-restyle` a lint error. That surfaced 65 violations, all in
fork-only files, and I fixed them at the call sites:
- `DialogPanel` in the Moatless settings dialogs keeps its own padding.
- Inputs use `font="mono"`.
- The sandbox tooltip uses `variant="code"`.
- The repository search field uses `InputGroup`.
- Muted captions in the sidebar filter are plain elements.
- The task-link empty state uses the `Empty` defaults.

`ui/textarea.tsx` gained the same `font` prop `Input` already has,
recorded as inventory entry `textarea-mono-font`. **Visual change to
review:** these panels now use the `ui` defaults for font size (13px →
14px) and dialog padding.

## Numbers
- Landed: 255 files, against 239 in the upstream range. The gap of 16 is
the lint fixes above plus `docs/fork/inventory.json`.
- Fork delta: 787 files.

## Verification
`verify.mjs` passed all 10 checks, including the full test step. The
unsupported-method derivation had nothing to add or drop.

## Feature classification

### Usable as-is
- Back/forward navigation with mod+[ and mod+] (pingdotgg#13212,
`AppSidebarLayout.tsx`, keybindings contract).
- Command palette sorts title matches by recent activity (pingdotgg#13219).
- The previous worktree branch shows on a second line in the branch
toolbar (pingdotgg#13314).
- A context-chip component, `ui` components owning their own variants,
composer chip rings no longer clip, a brain icon for the effort
dropdown, and accessible switch state (pingdotgg#13192–pingdotgg#13210, pingdotgg#13301, pingdotgg#13309,
pingdotgg#11580).
- CSV preview keeps a final quoted empty record (pingdotgg#11425).
- Mobile recovers from screen render errors (pingdotgg#13197); mobile version
bump.

### Unsupported in Moatless / needs implementation
- **Interactive 3D device workspace** (pingdotgg#12787,
`apps/web/src/components/device/**`). It sits behind the existing
`FEATURES.deviceHub` gate, and the `device.*` methods are still not
dispatched. It is covered by the existing device-hub gap.
- **Preview automation drives the visible browser for new agent
sessions** (pingdotgg#13064, `PreviewAutomationBroker.ts`,
`previewAutomation.ts`). Added to the existing preview-automation gap.
- **Mobile environment and provider update management** (pingdotgg#13302). It is
a paired-device surface, covered by the existing mobile gap.
- Desktop-only fixes (SnapShot Dock icon pingdotgg#13286, keyring test removal):
not applicable.

### Backend behavior to consider reproducing in Moatless
- **Don't replay stale agent alerts after a restart** (pingdotgg#13340,
`AgentAwarenessRelay.ts`). The relay publishes a completed or failed
thread only if its latest turn finished after the server started.
- **Normalise vendor-specific provider versions before the compatibility
check** (pingdotgg#13328, `providerCompatibility.ts`, `model-manifest.json`). It
strips Cursor's build hash and Antigravity's `agy_acp_server_` prefix,
and restores the ranges for every harness.

Both are recorded under "Runtime fixes upstream made to its own server"
in `docs/fork/gaps.md`. The tracker entry is in
`docs/fork/upstream-merge-log.md`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
Moatless task:
https://moatless.soaplabstest.com/tasks/0735cf38-d4cd-497a-87e7-ed2bc5c58ba6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants