chore(gate): re-record the SQL column-literal baseline after team-analytics.ts converted - #2880
chore(gate): re-record the SQL column-literal baseline after team-analytics.ts converted#2880gsxdsm wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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. Comment |
Greptile SummaryThe PR re-records the SQL column-literal ratchet after the current count in
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| scripts/lib/sql-column-literals-baseline.json | Updates the baseline to the current three literal sites, restoring the gate while preventing the removed sites from regrowing unnoticed. |
Reviews (2): Last reviewed commit: "chore(gate): re-record the SQL column-li..." | Re-trigger Greptile
…lytics.ts converted A conversion took packages/core/src/team-analytics.ts from 6 SQL column-literal sites to 3 without re-recording, so check-sql-column-literals failed and took the whole merge gate with it — blocking every PR, not just a non-blocking lane. This is the workflow the tool prescribes: its own failure message says to re-record when a count goes DOWN, in the same commit. The allowance is removed rather than left open for regrowth. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
75f32f4 to
4f77976
Compare
|
Superseded by #2878, which re-recorded the same baseline entry ( Closing rather than rebasing. Worth noting for whoever tracks this: #2878's own title is "re-record the SQL baseline — main is red after #2864" — an independent worker hitting the identical failure from a different converting PR, at the same time I hit it from |
…of failing the gate (#2888) ## Why `check-sql-column-literals` runs inside `pnpm test:gate` — the **blocking** lane. It hard-fails when a count *drops*, so a single converting PR that doesn't re-record takes down the gate for **every worker in the program** until someone fixes the baseline by hand. That is not hypothetical. It is happening on `main` right now (`team-analytics.ts` 6 → 3, fixed by #2880), and it is the **second** instance of the shape — the lifecycle census hit it from a merge wave that dropped eleven files at once. ## The census already resolved this exact trade-off From `docs/testing.md`, on why the census stopped hard-failing on a drop: > "the drop is almost never the failing author's to fix ... A permanently-red gate is a bigger hole than a stale allowance, because it gets ignored and then nothing is guarded at all." That reasoning applies here **with more force**, because the census is *not* in the blocking lane and this check *is*. Same failure mode, higher cost, opposite policy — this aligns them. ## What changes A drop now rewrites the baseline downward, reports what it lowered, and exits 0: ``` [check-sql-column-literals] baseline TIGHTENED — fewer literals than it allowed packages/core/src/team-analytics.ts: allowed 6, now 3 The baseline has been rewritten downward. COMMIT IT so the allowance cannot be regrown into; in CI this write is discarded with the runner, which is why the gate is green and not silent. ``` **The rise check is untouched.** "No new SQL column literals" is the ratchet's actual purpose and still fails hard. The stale-allowance concern the old comment raised is real and is preserved: the rewritten file must be committed, and in CI the write is discarded with the runner — so the gate goes green rather than silently passing a stale allowance, exactly as the census does. ## Verified in both directions | scenario | result | |---|---| | drop (`team-analytics.ts` 6 → 3, the live case) | **tightens, exit 0** | | rise (a literal added to a zero-allowance file) | **fails, exit 1** — `task-age-staleness.ts: 1 SQL column literal(s), baseline allows 0` | The rise probe needed a zero-allowance file: adding one literal to `team-analytics.ts` keeps it at 4 against an allowance of 6, which is correctly *not* a rise. Worth noting because it is an easy way to conclude the guard is dead when it is working. ## Relationship to #2880 #2880 fixes the **instance** — it re-records the current drift so the gate goes green now. This fixes the **class**, so the next conversion doesn't take the gate down again. They are independent and either can land first; if #2880 lands first, this becomes a no-op on a matching baseline. ## Verification - `pnpm test:gate` — exit 0 with this change applied - `pnpm lint` — clean No changeset: gate tooling, not published behaviour. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The merge gate is red on main; this fixes it
pnpm test:gateexits 1 — and this is the blocking lane, so it is currently stopping every PR in the program, not just a non-blocking suite.A conversion took
team-analytics.tsfrom 6 SQL column-literal sites to 3 without re-recording the ratchet's baseline.The whole diff
One line. Nothing else moved — 28 sites across 14 files, otherwise unchanged.
Why committing it is the prescribed workflow
The check says so in its own failure output:
Leaving the stale
6keeps an allowance open for three SQL column-literals to regrow into, which is exactly what the ratchet exists to prevent.It cannot hide a regression. The check fails on a rise; only a drop is re-recordable, and the new number is lower.
This is the second instance of a recurring pattern
#2844 was the same failure on the lifecycle-column census: a conversion landed, the count dropped, the baseline was not re-recorded, and
mainwent red for whoever ran next. That one was a non-blocking lane. This one is the merge gate, so the cost is higher — every worker's PR is blocked until it lands.Both ratchets prescribe re-recording in the converting commit. Worth flagging to whoever owns conversion review that this is now a repeat, not a one-off: the two ratchets fail in different lanes and neither failure names the PR that caused it.
Verification
pnpm test:gate— exit 0 (was exit 1)check:lifecycle-columns) — exit 0No changeset: baseline state for internal tooling, not published behaviour.
🤖 Generated with Claude Code