test(frontend): fix the red main - pay down the coverage regression from #274/#263/#280

✅ merged · #281 · open-legal-products/mike ← amal66/mike · opened 3mo ago by amal66 · merged 3mo ago by willchen96 · +600-13 across 5 files · ↗ on GitHub

From the PR description

Why

Main's Frontend build and tests CI 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:

  • #274 (folder-grouped tabular reviews) + #263 (db-pagination) - listTabularReviews / listTabularReviewIds query building (28 of the 64 uncovered branches), document_grouping on create/update, uploadReviewDocument, and the tabular chat/cell endpoints.
  • #280 (workflow slash triggers) - listWorkflows type 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.ts 27 → 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:

Metric Before After
Statements 52.14% 81.18%
Branches 62.57% 98.24%
Functions 34.67% 55.64%
Lines 52.41% 79.18%

Floors ratcheted, per the config's own rule

vitest.config.mts says 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:coverage fully green (158 tests, thresholds met), npm run lint 0 errors, npx tsc --noEmit clean.

Noticed while testing (not fixed here - happy to file issues)

  • mikeApi.ts:1062 - if (pagination?.limit) drops limit: 0 / offset: 0 (falsy check).
  • Multipart uploads throw plain Error instead of MikeApiError, so isMfaRequiredError/status handling can't see upload failures.
  • uploadReviewDocument is a non-atomic upload-then-PATCH; a PATCH failure orphans the uploaded document.
  • deleteTabularReviewsWithConcurrency with concurrency: NaN silently no-ops (zero workers).

🤖 Generated with Claude Code

Our analysis

Restore frontend coverage ratchet — read the full analysis →

Think the analysis missed something the PR description covers?

Commits in this PR (1)

SHA Subject Author Date
ac9a7859 test(frontend): pay down the coverage regression from #274/#263/#280 with lib tests Amal 2026-08-03 ↗ GitHub
commit body
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 #255 (2266446) before three merges
landed untested code inside the gated scope (src/app/lib/**):

- #263 (db-pagination) + #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.
- #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 #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>

Capture this PR into my fork

Download a Markdown prompt that tells Claude how to port every commit in this PR into your working tree. Run it via claude -p < capture-pull-281.md from inside the repo you want the changes in.

⬇ Download capture-pull-281.md