chore(governance): add scoped PR-size exception for PR #583 (#532) - #586
Conversation
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a base-ref-visible, tightly constrained PR-size exception for PR #583 so its reviewed architectural and localization changes can exceed the normal file-count ceiling without broadly weakening size enforcement. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (4)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📝 WalkthroughWalkthroughThe PR adds a configured size exception for PR ChangesPR size exception handling
Test count documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk:⚪ Minimal · up to This change adds the scoped PR 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
PR Summary
|
|
| Overall Grade | Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Docker | Sep 3, 2026 5:18p.m. | Review ↗ | |
| Python | Sep 3, 2026 5:18p.m. | Review ↗ | |
| Rust | Sep 3, 2026 5:18p.m. | Review ↗ | |
| Shell | Sep 3, 2026 5:18p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
There was a problem hiding this comment.
Review Summary
This PR adds a scoped exception for PR #583 to the pr-size-exceptions.json governance file. The structure follows the established pattern from previous exceptions.
Critical Issue Found
There's a logic error in the maxFiles configuration: it's set to 85 while the allowedPaths array contains exactly 81 entries. According to the PR description, this should be an "exact 81-file allowlist" with no wildcards. The 4-file gap between maxFiles and the actual path count would incorrectly permit files outside the intended allowlist.
Please fix this mismatch to ensure the exception enforces the exact file list as intended.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Uh oh!
There was an error while loading. Please reload this page.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
PR size is back within target — previous warning below is resolved. |
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Uh oh!
There was an error while loading. Please reload this page.
7136704 to
ddf9523Compareqnbs
commented
Sep 3, 2026
Fully recomputed from scratch, not carried forward. Squashed the prior 7 commits (6 incremental recompute attempts + a stray temp commit, none verified against a real rebase or actual gate behavior) into one, freshly derived by actually rebasing #583 onto current main (which now carries #562, #592, #594). New numbers: 65 governed files (84 incl. 19 generated locale bundles), 1611 meaningful lines, 14 commits — all lower than the previous 70/1753/21 estimate. Two things the rebase revealed that no prior estimate could have known:
Also fixed a genuine bug in Verified end-to-end locally, not just by inspection: built a synthetic commit carrying this exact registry entry on top of real Local validation: lint clean, typecheck clean, 57/57 tests green (including the new regression test), full |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
… check-pr-size.mjs exception-ceiling bug Recomputed entirely from a genuine rebase of #583 onto current main (which now carries #562, #592, and #594) rather than trusting the historical 70 files / 1753 lines / 21 commits figures the earlier commits on this branch carried forward. The rebase itself revealed two things the prior estimate could not have known: 1. #583 and #592 (the independent factory-reset persistence-admission fix, issues #591/#593) touch overlapping files -- app/listenerMiddleware.ts, services/factoryResetService.ts, services/crossProjectIndexService.ts, and their tests. Reconciled by layering both mechanisms inside wipeAllAppData(): #592's isFactoryResetInProgress()/coordinator-draining gate runs first (blocks new Redux-listener writes, drains in-flight ones), then #583's beginIdbReset() force-closes every other long-lived IDB connection the coordinators do not track. 2. PR #590 (merged earlier, unrelated) had already independently shipped the same locale-independent Settings/mobile-"More"-button navigation fix#583 originally introduced across five files (components/SettingsView.tsx, components/settings/SettingsModals.tsx, components/settings/DataSection.tsx, components/Sidebar.tsx, tests/e2e/helpers.ts). Parallel convergent evolution left #583's own changes to those files fully superseded -- zero net diff against current main -- so they are correctly absent from allowedPaths. Final measured diff: 65 governed files (84 incl. generated locale bundles), 1611 meaningful lines, 14 commits -- exact ceilings, no speculative headroom, computed directly via check-pr-size.mjs itself against the real rebased branch. That direct measurement also surfaced a latent bug in check-pr-size.mjs: when an exception's own ceiling legitimately exceeds TIERS.absolute (30 files/3000 lines/15 commits) -- the entire point of granting one -- evaluatePrSize() fell through to selectSeverity() against that fixed tier instead of treating the exception's own ceiling as authoritative, so a fully-satisfied wide exception still reported blocking:true. Neither #539 (maxFiles:30, at the absolute tier's own boundary) nor #564 (maxFiles:3, well under it) had ever exercised this path -- #583 is the first exception whose own scope is wide enough to expose it. Fixed to short-circuit on exception.entry directly, verified against a synthetic base commit carrying this fix plus the recomputed entry, diffed against the actual rebased #583 branch (exit 0, PR_SIZE_EXCEPTION=APPLIED). Added a regression test covering a wide exception ceiling that exceeds the fixed absolute tier. Squashes the prior five commits on this branch (four incremental "recompute" attempts plus a stray temp commit), none of which had been verified against a real rebase or the actual gate behavior.
ddf9523 to
e1f733bCompareThere was a problem hiding this comment.
Gates Passed 3 Quality Gates Passed
See analysis details in CodeScene
Quality Gate Profile:The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
Summary
Adds a scoped
config/pr-size-exceptions.jsonentry for PR #583 (issue #532 E2E startup/navigation nondeterminism), mirroring the precedent set by #576 for PR #564.check-pr-size.mjsloads the exception registry from the PR's base ref, so PR #583 cannot self-authorize this — it must land onmainfirst, then #583 rebases onto it so its own merge-base picks up this entry.Why #583 needs it
#583's review-convergence (CodeRabbit, cubic, Amazon Q, and direct maintainer review) surfaced real defects requiring a genuine architectural fix — a shared IDB reset-gate covering every long-lived connection in the app (9 modules). The gate fails closed: a registered connection-closer failure rejects
beginIdbReset()(after every closer, including the failing one, has still had its chance to run) sowipeAllAppData()aborts before any database deletion on an unproven teardown, and a closer registered while the reset is draining joins that same awaited barrier instead of racing ahead of it as fire-and-forget. A centralizedbeginIdbOpenAdmission()/isIdbOpenStillValid()pair closes a further gap: an open that starts after a reset already bumped the generation would otherwise still match at completion, so every opener also refuses to start a fresh open while a reset is in progress — and every reset closer now also invalidates its own module's pending open flight (not just the already-open connection), so the first post-reset caller starts a genuinely fresh attempt instead of reusing one already doomed to reject.deleteAllIndexedDBDatabases()usesPromise.allSettledso a fast-rejecting deletion can't release the gate while another is still outstanding, and only targets database names it can prove it owns (exactKNOWN_DB_NAMESplus theworldscript-localfirst-prefix), never assuming ownership just because enumeration returned a name. It also closes several stale-open-completion single-flight races via an identity token, a synchronousindexedDB.open()throw that could permanently memoize a rejected open promise, a test-reset helper that swapped the fake IndexedDB factory without releasing its cached connection, and a transient reset-denial NOOP that was indistinguishable from an intentional one.Since #583 was created, main has moved forward through PR #590 (locale-independent Settings navigation — independently shipping the same fix#583's own five UI/helper files originally introduced, leaving those five files at zero net diff now) and PR #592 (factory-reset persistence-admission/drain-coordinator fix, issues #591/#593 — overlapping several of #583's own files, reconciled by layering both mechanisms inside
wipeAllAppData()).Scope — recomputed from an actual rebase, not carried forward
Exact 65-file allowlist (84 incl. 19 generated
public/locales/*/bundle.json),maxNonExemptMeaningfulLines: 1611,maxCommits: 14— measured directly viacheck-pr-size.mjsagainst a real rebase of #583 onto currentmain, not estimated or copied from an earlier snapshot. (An earlier version of this PR carried 70/1753/21, computed before that rebase existed — superseded.)Review-thread state was not re-verified as part of this recompute and must be checked live against the current #583 PR before it merges, independent of this file-scope/line-ceiling exception.
This recompute also found and fixed a latent bug in
check-pr-size.mjsitself: when an exception's own ceiling legitimately exceedsTIERS.absolute(30 files/3000 lines/15 commits — the entire point of granting one),evaluatePrSize()fell through toselectSeverity()against that fixed tier instead of treating the exception's own ceiling as authoritative, so a fully-satisfied wide exception still reportedblocking:true. Neither #539 (maxFiles:30, at the tier's own boundary) nor #564 (maxFiles:3) had ever exercised this path — #583 is the first exception wide enough to expose it. Fixed with a regression test, verified end-to-end against the real rebased #583 branch (PR_SIZE_EXCEPTION=APPLIED outcome=within target).17 of the 65 files are
locales/*/sidebar.jsonwith only a trailing-newline diff — they already lacked a final newline onmain, and Biome's format-on-commit hook re-adds one the moment any of them is staged for any reason in this PR. This could not be reverted for real without skipping the pre-commit hook, so they remain in the allowlist with that diff.Test plan
jq empty config/pr-size-exceptions.json— valid JSONallowedPathsand the rebased fix(e2e): eliminate WelcomePortal startup/navigation nondeterminism (#532) #583 branch's actualgit diff --name-onlyoutput against currentmainpnpm run lint/ typecheck /pnpm run ci:prepush— cleancheck-pr-size.mjsfix