Skip to content

style: format the 11 files #29982 merged unformatted (fix red main) - #31186

Closed
brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/fix-main-format-29982
Closed

brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/fix-main-format-29982

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​290 $\color{#cf222e}{\Huge{\mathbf{−}}}$​97 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​193
Prod 6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​86 $\color{#cf222e}{\Huge{\mathbf{−}}}$​35 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​51

What broke

Every PR's static check fails on main right now. #29982 ("save every chat reply and tool output whole") merged 11 files that the formatter rejects. CI runs oxfmt --check . before typecheck and tests, so every PR merged with current main stops there (for example #30398's run 39474073984, job "static analysis and typecheck").

The open formatting fixes #30951 and #30953 cover files from #29042, not these.

What changed

Formatted exactly these 11 files with the repo's oxfmt, with no other edits:

  • src/main/acp/acp-tool-timeline.ts
  • src/main/codex/codex-structured-journal-translation-turn-lifecycle.test.ts
  • src/main/jsonl-rpc/timeline-final-tail.test.ts
  • src/main/jsonl-rpc/timeline-lane.ts
  • src/main/native-chat/agent-session-journal/journal-completed-content.test.ts
  • src/main/native-chat/agent-session-wire/agent-session-item-wire-form.test.ts
  • src/main/native-chat/agent-session-wire/agent-session-item-wire-form.ts
  • src/main/pi/rpc-messages.ts
  • src/main/pi/rpc-session.ts
  • src/renderer/src/components/native-chat/NativeChatToolOutput.test.tsx
  • src/renderer/src/components/native-chat/NativeChatToolOutput.tsx

oxfmt --check . on current main lists exactly these 11 files, and none other. Every file stays under its line limit after formatting.

User-facing change

None. Formatting only; no behavior changes.

Testing

  • oxfmt --check . passes on this branch.

  • CI on this PR is the check.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below (formatting only)

Plan

Run the existing repository formatter on the eleven files rejected on main, verify the normalized syntax trees are identical, then compare real chat tool output on main and the branch. Keep all runtime behavior unchanged.

Light-path classification

Review status

The same eleven-file formatting fix already merged as #31184 at 18:44 UTC on October 20. All eleven files are byte-for-byte identical between this branch and current main (36d13f14a5f3e4252043486af30fbe1bb92bc1d5). QA was stopped once that was confirmed. This PR is not marked ready; the lane owner has the duplicate for disposition.

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 54a5875c-3b93-4d64-ace9-f0a3e2050e5a
📥 Commits

Reviewing files that changed from the base of the PR and between 3e4ceb1 and bee8dbb.

📒 Files selected for processing: 11
  • src/main/acp/acp-tool-timeline.ts
  • src/main/codex/codex-structured-journal-translation-turn-lifecycle.test.ts
  • src/main/jsonl-rpc/timeline-final-tail.test.ts
  • src/main/jsonl-rpc/timeline-lane.ts
  • src/main/native-chat/agent-session-journal/journal-completed-content.test.ts
  • src/main/native-chat/agent-session-wire/agent-session-item-wire-form.test.ts
  • src/main/native-chat/agent-session-wire/agent-session-item-wire-form.ts
  • src/main/pi/rpc-messages.ts
  • src/main/pi/rpc-session.ts
  • src/renderer/src/components/native-chat/NativeChatToolOutput.test.tsx
  • src/renderer/src/components/native-chat/NativeChatToolOutput.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The changes reformat timeline and RPC declarations, calculations, and tests, along with journal, wire, and renderer code. The described limits, assertions, and runtime behavior remain unchanged. The NativeChatToolOutput test cleanup now explicitly unmounts the React root before resetting the read mock.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to bee8d

No actionable merge-blocking risk was identified; the formatting cleanup is ready for normal CI validation.

Pre-merge checks: Passed 2 · Failed 3

Failed Failed checks: 3 warnings

Check name Status Explanation Resolution
Linked Issues check Warning Open issue #30951 requires formatting wsl-orcad-connection.ts, wsl-orcad-pairing-identity.ts, and or cad-managed-tunnel-identity-restart.test.ts. It also requires moving the listed public types … Add the #30951 WSL formatting changes. If the formatted connection file exceeds 300 lines, move the specified types into wsl-orcad-connection-types.ts and re-export them without changing importers.
Out of Scope Changes check Warning The 11 stated changes format ACP, Codex, JSONL-RPC, native-chat, and PI files associated with the separate #29982 formatter failures. They do not implement #30951's WSL-file requirements. The PR descr… Separate the #29982 formatting changes into a PR scoped to that work, or change this PR's linked issue and scope so they match. Keep a PR for #30951 focused on its three WSL files and the conditional type extraction.
Description check Warning The description clearly explains the formatting-only scope, affected files, motivation, testing, and current review status. It does not follow the required template fully: it omits the ELI5, Why, Link… Add the missing template sections. Include an actual linked issue under Fixes #, write N/A with a reason under Visual Proof because there is no behavior or UI change, complete the ELI5 and Why sections, address the upstream-boundary and…
Passed Passed checks: 2 passed
Check name Status Explanation
Title check Passed The title identifies the formatting of 11 files and the goal of fixing the failing main-branch check. It is specific and related to the primary change.
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 11 files.
Full details: Linked Issues check

Explanation

Open issue #30951 requires formatting wsl-orcad-connection.ts, wsl-orcad-pairing-identity.ts, and or cad-managed-tunnel-identity-restart.test.ts. It also requires moving the listed public types and re-exporting them if formatting pushes wsl-orcad-connection.ts over 300 lines. This PR's stated 11 changed files do not include those WSL files. The PR description and issue identify the WSL formatter failures as still separate from this PR's fixes.

Resolution

Add the #30951 WSL formatting changes. If the formatted connection file exceeds 300 lines, move the specified types into wsl-orcad-connection-types.ts and re-export them without changing importers.

Full details: Out of Scope Changes check

Explanation

The 11 stated changes format ACP, Codex, JSONL-RPC, native-chat, and PI files associated with the separate #29982 formatter failures. They do not implement #30951's WSL-file requirements. The PR description also distinguishes these changes from the open WSL formatting fixes.

Resolution

Separate the #29982 formatting changes into a PR scoped to that work, or change this PR's linked issue and scope so they match. Keep a PR for #30951 focused on its three WSL files and the conditional type extraction.

Full details: Description check

Explanation

The description clearly explains the formatting-only scope, affected files, motivation, testing, and current review status. It does not follow the required template fully: it omits the ELI5, Why, Linked Issue/Fixes #, Visual Proof with the required N/A statement, Agent skill upstream boundary, Notes, and most checklist items.

Resolution

Add the missing template sections. Include an actual linked issue under Fixes #, write N/A with a reason under Visual Proof because there is no behavior or UI change, complete the ELI5 and Why sections, address the upstream-boundary and Notes sections, and complete the applicable checklist items. Clarify the AI Disclosure section if required for the contributor type.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

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

@brennanb2025

Copy link
Copy Markdown
Contributor Author

The formatting check failed after #29982 left eleven files unformatted, blocking PR static checks. This PR reformats those files without changing app behavior, but the identical fix already merged in #31184 at 18:44 UTC on October 20. All eleven files now match current main byte for byte.

The adversarial review found no behavioral defects and required no code fixes. Normalized syntax trees, JSX text and comments match the original base; every file matches the formatter’s output and remains under its line limit. The whole-tree formatting check passes, and the full PR workflow passed on bee8dbb52282c3ba857e3bd044ce569cc3881f09.

This review is not marked ready. The strict light-path criteria were not met, and QA was stopped before an Electron app or QA driver launched once the duplicate landing was confirmed. No screenshots or live behavior verification were completed. The QA ticket and workers were released; this PR remains open for the lane owner to resolve its duplicate status.

For Brennan’s review: #31184 already delivered the formatter repair. No app behavior difference is intended, and no human design judgment is needed in this diff. Main-versus-branch screenshots are unavailable because QA was stopped before launch.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Closing: superseded by #31184, which merged on 10-20 with the same formatting of the same 11 files (every file here is byte-identical to #31184's merged version).

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