[shivamhwp]

Note

🤖 Claude Fable 5.2 on behalf of Shivam

Problem

A read-through of the whole test suite against docs/internals/testing.md found tests the rules say not to write: a case repeated at a second layer or in a second it, duplicate table rows, and assertions that only repeat a constant or fixed copy. They add run time and review noise and catch nothing the remaining tests miss.

Change

Deletions only: 39 audit entries in 33 test files (896 lines). No surviving test changed, apart from removing imports, a mock and table rows that the deletions left unused. Each removed test is one of:

• Covered by a test that stays. The surviving test runs the same production path and asserts at least as much. Where its inputs differ from the removed test's, the difference cannot change the branch taken; those cases are marked below.
• Restates the source. It repeats a literal default, a fixed string, or a return type.
• Cannot fail. Its mock or fixture is never read by the code under test.

No removed test was the only one to run a function, the only check of a guard, the only test of one branch of a choice, or the only proof of a past fix.

One production line changes: sortThreadsForListV2 in apps/mobile/src/features/threads/threadListV2.ts loses its export, because the removed test was its only importer and knip fails on unused exports. It is still used inside the module.

Every removal and what covers it

Server: orchestration and adapters

• AcpAdapterV2.test.ts "clamps per-turn subsets that exceed their parent counters": the pure case "clamps subsets that exceed their parents and drops unset counters" uses the same sample; adapter wiring stays covered by "reports per-turn usage unavailable on a continuation turn that never prompts".
• AcpAdapterV2.test.ts "reports per-turn usage unavailable when the agent omits usage": the pure unavailable case plus the same continuation test.
• AcpAdapterV2.test.ts "passes per-turn usage samples through": the pure case with every counter set, plus the continuation test, which checks each prompt gets its own sample. Different numbers, same branch: the adapter passes the usage object through whole (packages/provider-acp/src/server/adapter.ts).
• AcpAdapterV2.test.ts "finalizes a settled turn held open for background work when interrupted": "carries a live subagent lineage across an interrupt so the next turn can complete it" has the same setup, asserts the same interrupt, then more.
• ClaudeAgentSdkQueryRunner.test.ts "reports the CLI's root refusal when it exits before answering": "keeps other startup stderr out of the turn failure" asserts the refusal is reported, with other stderr around it. Detection is a substring check, so the extra stderr does not change the branch.
• ClaudeAdapterV2.test.ts row "compressed-bomb": the fixture builder gave it the same bytes as the "object-stream" row, with the same assertion.
• ProjectionStore.test.ts "limits turn-start history to the requested runs, including an empty selection": TurnStartReads.test.ts makes the same four reads and asserts exact items.
• CodexAdapterV2.test.ts row "sharing-code-mention": CodexManagedErrors.test.ts asserts the classifier ignores this exact string; rows "chatgpt-sharing-prose" and "exhausted-500" keep the adapter path.

Server: everything else

• CodexDeveloperInstructions.test.ts "describes Markdown media support in the runtime context in both modes": a regex on fixed copy, run in one mode. The test that checks retired wording stays out is kept.
• EventNdjsonLogger.test.ts "shares one thread writer across native and canonical streams": "keeps shared store views non-owning when one adapter closes" writes both streams to one thread file and asserts both serialized events. It uses different event ids and closes the native logger between the writes.
• AntigravityAcpSupport.test.ts rows undefined, {}, { image: false }: the table "keeps attachments path-only when native prompt capabilities are absent" includes a GIF with undefined, {}, and all three capabilities false, so its image capability values match the removed rows. The { image: true } row is kept.
• consoleStdin.test.ts row "file": the test callback ignores the row value, so it was the "pipe" row twice.
• AgentSessionScanner.test.ts "drops injected Codex instructions while keeping the visible user event": "uses the canonical Codex event when its turn has generated response context" exercises the same canonical-turn filtering, with different instruction and conversation text, one more injected record, and a title assertion.
• 016_CanonicalizeModelSelections.test.ts row thread-legacy-options: identical to thread-claude; it carried no legacy options.
• PullRequestService.test.ts "reports a file pushed to since it was cleared as changed": "tracks environment viewed files by revision and refuses a baseline it cannot read" and "leaves a mark alone when the host could not say what the head has of it" assert the same dismissal.
• externalLauncher.test.ts "keeps scanning after the caller is interrupted and shares that scan": the launcher forwards to makeStaleWhileRevalidate, whose own test "shares one first scan that an interrupted caller does not cancel" covers the case; "memoizes editor discovery and refreshes in the background" keeps the launcher on the cache.

Shared packages

• contracts/settings.test.ts "stores the transcription model and resources on the environment": three literal defaults from settings.ts.
• client-runtime/state/threadSnoozed.test.ts "wakes early when the agent is blocked on the user": "is true for approvals, input, and failures" plus the effectiveSnoozed table rows.
• client-runtime/providerSkills.test.ts "keeps the skill alias when the provider also exposes it as a slash command": the function takes no slash commands; "shows one row when enabled skills share a name" passes an equivalent input.
• shared/orchestrationV2Timeline.test.ts two tests (four cases under describe.each): the table's cancelled row asserts the same two visibility results.
• shared/filePreview.test.ts two RTF rows: the Office table's "notes.rtf" row and the application/rtf MIME assertion.
• client-runtime/threadPullRequests.test.ts "falls back to the most recently updated terminal link": "selects the same terminal link as stable sorting across input orders" checks the full link in both orders.

Source control packages

• source-control-gitcafe/gitCafePullRequestJson.test.ts 93-byte fixture case: the "+" row takes the same branch with the same assertion; the byte count only checked the fixture.
• source-control-github/gitHubPullRequestJson.test.ts "quotes literal backslashes without interpreting them as escapes": "preserves spaces and literal backslashes in both rename paths" and shared/gitPatchPath.test.ts.
• source-control-github/gitHubPullRequestJson.test.ts typed payload shape for buildReviewerRequest: the return type already requires both arrays; the mixed case covers the filter.

Web

• terminal-links.test.ts "keeps Windows absolute paths with forward slashes unchanged": word-for-word copy of the case in shared/fileLinks.test.ts.
• composer-editor-mentions.test.ts five recognition cases: the same inputs in shared/composerInlineTokens.test.ts (one differs by a trailing word that the tokenizer does not read). The two cases that exercise the web wrapper are kept.
• markdown-links.test.ts three of four file-URL cases: the shared/fileLinks.test.ts table. One case stays to run the web adapter, and it is the only one with a non-empty hash.

Mobile and desktop

• mobile/threads/threadListV2.test.ts sortThreadsForListV2 case: the function only calls the shared sort, covered in client-runtime/state/threadSort.test.ts; the surviving buildThreadListV2Items tests still run the wrapper.
• mobile/home/useThreadListActions.test.ts second archive case: "rejects a concurrent archive request without sending another mutation" asserts the same request and refresh.
• mobile/diffs/nativeReviewDiffSource.test.ts single retry case: the string row of "recovers after two transient failures on a later load".
• mobile/pull-requests/pullRequestDiffPages.test.ts full notice string: a template restated; the terminology for both host kinds and the full GitHub string are still asserted.
• desktop/app/ClerkTokenStore.test.ts EXDEV case: it mocked renameSync, which the migration never calls, so it re-ran an existing case. Its vi.mock("node:fs") goes with it.
• desktop/app/ClerkTokenStore.test.ts permissions case: the second half of the first test.
• desktop/snapShot/DesktopSnapShot.test.ts Hyprland/PNG row: identical to the GNOME/PNG row; the backend label only changes inputs for niri and KDE.

Infra and evals

• infra/relay/agentActivity/FcmDeliveries.test.ts non-null alert case: agentActivityPolicy.test.ts builds the same rows and asserts which one is returned.
• evals/mcp-tool-use/run.test.ts three briefRoute rows: the same text and route appear elsewhere in the file.
• evals/mcp-tool-use/run.test.ts maxSteps >= 14: the fixture file sets 14.
• evals/mcp-tool-use/secret-grading.test.ts two rows: exact duplicates in the same tables.

Scope and approval

Maintainer-requested cleanup by Shivam, following the rules in docs/internals/testing.md ("What to test"). It removes tests only and changes no behavior. The audit listed 131 delete candidates. 29 were set aside up front (lower confidence, visual guards, or a row that has to move down a layer first). 63 were checked and left alone because they were the only test of a function, a guard or a branch, or the proof of a past fix. Three of those came back after review: tests that guard required device and preview instructions (orchestrationInstructions.test.ts, mcp/toolkits/device/handlers.test.ts, shared/preview.test.ts).

Verification

At ad63d25190, rebased onto main at a577e9404e:

• vp run check:changed origin/main passed against merge base a577e9404e: format, lint, knip, GraphQL checks, typecheck of 37 packages, the 34 test files beside the changes, and the React compiler check.
• vp test run on the 33 edited test files locally: 33 files, 2,582 tests passed.
• Each removal was checked against the test that covers it before deleting (Claude Opus 5.5), then every removal was reviewed again by a separate session that did not write the change (GPT-6.1 Sol). That review confirmed these 39 and asked for the three restores above.

Not run locally: the full suite and the browser tests. No *.browser.test.tsx file is touched.

Claude Fable 5.2, Claude Code via T3 Code. Removals by Claude Opus 5.5; review by GPT-6.1 Sol.

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Approved at 3acb056

Macroscope's review found this PR approvable — The assessed change makes a thread-sorting helper private without changing its implementation or internal use. No material runtime or product-default change is evident.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

This change removes or narrows tests across desktop, mobile, server, web, evaluation, client-runtime, contracts, shared, and source-control code. It also makes sortThreadsForListV2 module-local and changes one secret-grading test case.

Changes: Test coverage and visibility updates

Layer / File(s)	Summary
Desktop and mobile test updates apps/desktop/src/app/ClerkTokenStore.test.ts, apps/desktop/src/snapShot/DesktopSnapShot.test.ts, apps/mobile/src/features/*, apps/mobile/src/features/threads/threadListV2.ts	Removed or narrowed tests for token migration, screenshot persistence, diff retries, thread archiving, provider notices, and thread sorting. sortThreadsForListV2 is no longer exported.
Server CLI and adapter test updates apps/server/src/cli/consoleStdin.test.ts, apps/server/src/orchestration-v2/Adapters/*	Narrowed redirected-input and attachment scenarios. Removed tests for adapter usage mapping, interrupted turns, startup refusal text, and provider-error classification.
Server persistence and provider test updates apps/server/src/orchestration-v2/ProjectionStore.test.ts, apps/server/src/persistence/Migrations/*, apps/server/src/process/externalLauncher.test.ts, apps/server/src/project/AgentSessionScanner.test.ts, apps/server/src/provider/*, apps/server/src/pullRequest/PullRequestService.test.ts	Removed tests for history filtering, migration fixtures, editor discovery, transcript parsing, provider instructions and logging, image capability cases, and viewed-file revision changes.
Web editor and link test updates apps/web/src/composer-editor-mentions.test.ts, apps/web/src/markdown-links.test.ts, apps/web/src/terminal-links.test.ts	Removed selected mention parsing, file URI, and Windows path test cases.
Evaluation and shared-package test updates evals/mcp-tool-use/*, infra/relay/src/agentActivity/FcmDeliveries.test.ts, packages/client-runtime/src/*, packages/contracts/src/settings.test.ts, packages/shared/src/*	Removed or adjusted selected evaluation, alert, client-runtime, settings, preview, and timeline tests.
Source-control test updates packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts, packages/source-control-github/src/server/gitHubPullRequestJson.test.ts	Removed tests for patch output, reviewer request fields, and backslash quoting in generated patch paths.

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

Possibly related PRs

• pingdotgg/t3code#24276: Implements native review source retry behavior and provider notices for unavailable pull-request diffs, which are behaviors covered by tests removed here.

Merge Risk: Minimal · up to ad63d

This PR only trims tests and narrows one helper's visibility, so production behavior is unchanged. Merging carries little risk, apart from the reviewers' own judgement on whether each removed test duplicated other coverage.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: removing redundant or source-restating tests.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description is complete and follows the required Problem, Change, Scope and approval, and Verification sections. It explains the duplicate-test cleanup, identifies the single production visibility…

Full details: Description check

Explanation

The description is complete and follows the required Problem, Change, Scope and approval, and Verification sections. It explains the duplicate-test cleanup, identifies the single production visibility change, documents coverage rationale, reports focused test results, and states that the full suite and browser tests were not run.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: ad0b2a9
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9f486f1e-b7bc-4b16-93f5-1de605eebb4d

Commits

Reviewing files that changed from the base of the PR and between 3acb056 and ad63d25.

Files selected for processing: 4

• apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts
• apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts
• apps/server/src/process/externalLauncher.test.ts
• apps/server/src/pullRequest/PullRequestService.test.ts

Files with no reviewable changes: 4

• apps/server/src/process/externalLauncher.test.ts
• apps/server/src/pullRequest/PullRequestService.test.ts
• apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts
• apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts

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