test(frontend): fix the red main — pay down the coverage regression from #274/#263/#280 - #281
Merged
willchen96 merged 1 commit intoAug 3, 2026
Conversation
…ucts#274/Open-Legal-Products#263/Open-Legal-Products#280 with lib tests WHY THIS MATTERS Main's "Frontend build and tests" job has been red for everyone: statements 52.14% vs the 54% floor and branches 62.57% vs 73%. A coverage ratchet only works if regressions are paid down with tests — if we lower the floor instead, the ratchet becomes a decoration and every future untested merge quietly erodes the suite. This commit restores green by testing the code that caused the drop, then re-arms the ratchet at the new level. WHAT HAPPENED The ratchet floors were measured in Open-Legal-Products#255 (2266446) before three merges landed untested code inside the gated scope (src/app/lib/**): - Open-Legal-Products#263 (db-pagination) + Open-Legal-Products#274 (folder-grouped tabular reviews) grew mikeApi.ts's listTabularReviews/listTabularReviewIds into query-string builders with seven conditional params each, added the document_grouping field, the uploadReviewDocument orchestration, and the tabular chat/cell endpoints — nearly all unexercised. Lines 1044-1456 were the bulk of the uncovered report. - Open-Legal-Products#280 (workflow slash triggers) leans on the workflow endpoints (listWorkflows feeds the slash menu), which were also untested. Because coverage is a global percentage over the gated files, adding untested statements/branches anywhere in scope dilutes the totals even though no tested line got worse. HOW THE FIX WORKS Extend the existing mikeApi.test.ts fetch/session mocking pattern to the regressed surface, asserting behavior (URLs, methods, exact payloads, error contracts), not just execution: - listTabularReviews/listTabularReviewIds: every pagination knob serialized under its snake_case name, scope="all" omitted (backend default), abort signals forwarded, and the ids query scoped identically to the list query — the invariant that keeps select-all-then-delete from deleting reviews the user cannot see. - createTabularReview/updateTabularReview: document_grouping (the Open-Legal-Products#274 field) passes through unchanged; PATCH sends only the given fields. - uploadReviewDocument: project vs standalone upload routing, and that the follow-up PATCH appends to existing document_ids instead of replacing them (the review-shrinking failure mode). - Multipart uploads: FormData with auth header only (a manual JSON content type would break the boundary), optional filename field, and the plain-Error-with-response-text failure contract. - Tabular chats/cells, workflow list/hide/unhide, query and payload defaults (getDocumentUrl version param, createChat "{}" body, parent_folder_id null-vs-undefined, empty error bodies). - supabase.ts: importing without env vars fails loudly at module load — the desired crash-at-startup behavior for a misconfigured build. - deleteTabularReviewsWithConcurrency: empty input short-circuits; concurrency<=0 clamps to one worker instead of silently deleting nothing. - utils.diceCoefficient: sub-bigram inputs score 0. Coverage moves from 52.14/62.57/34.67/52.41 (stmts/branch/funcs/lines) to 81.18/98.24/55.64/79.18. Per the ratchet's own rule ("floors only go up: when you add tests, raise them in the same PR"), the floors move to 79/96/53/77 — about two points under the new measurement, so one small innocent addition doesn't instantly re-redden main, while a real drop still fails CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
willchen96
approved these changes
Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Main's
Frontend build and testsCI job has been red since #274/#280 merged: all tests pass, but the vitest global coverage thresholds fail — statements 52.14% vs the 54% floor, branches 62.57% vs 73%. Because every PR runs the same job, every open PR currently shows a red check that its author can't fix. This PR pays the regression down the way a coverage ratchet is meant to be paid: by testing the untested code, not by lowering the floors.What was untested
The gap traces to recently merged frontend code in the gated
src/app/lib/**scope:listTabularReviews/listTabularReviewIdsquery building (28 of the 64 uncovered branches),document_groupingon create/update,uploadReviewDocument, and the tabular chat/cell endpoints.listWorkflowstype filter and the hidden-workflow routes that feed the slash menu.What this adds
35 net new behavior tests (123 → 158 across 22 files) — error contracts, pagination edges, query scoping, and payload defaults, not render-without-crashing filler:
mikeApi.test.ts27 → 55 tests: pagination/search query building, tabular CRUD incl.document_grouping, review-document upload, tabular chats +streamTabularChat, cell ops, multipart upload error contracts, workflow endpoints.supabase.test.ts(new): env-fallback branches; the import fails loudly without env.deleteTabularReviewsWithConcurrency.test.ts: empty input and concurrency-clamp edges.utils.test.ts: sub-bigram guard.Coverage in the gated scope:
Floors ratcheted, per the config's own rule
vitest.config.mtssays floors only go up: when you add tests, raise them in the same PR. Raised 54/73/32/52 → 79/96/53/77, ~2 points under the new measurement so a small untested change still fits but a #274-sized untested merge fails loudly at the PR instead of breaking main for everyone after merge.Verified
npm run test:coveragefully green (158 tests, thresholds met),npm run lint0 errors,npx tsc --noEmitclean.Noticed while testing (not fixed here — happy to file issues)
mikeApi.ts:1062—if (pagination?.limit)dropslimit: 0/offset: 0(falsy check).Errorinstead ofMikeApiError, soisMfaRequiredError/status handling can't see upload failures.uploadReviewDocumentis a non-atomic upload-then-PATCH; a PATCH failure orphans the uploaded document.deleteTabularReviewsWithConcurrencywithconcurrency: NaNsilently no-ops (zero workers).🤖 Generated with Claude Code