ci(frontend): ratchet the frontend migration metrics - #8491
ci(frontend): ratchet the frontend migration metrics#8491talissoncosta wants to merge 2 commits into
Conversation
Four migrations are in flight at once and none of them was measured, so a pull request could add to any of them without anybody noticing. Counting them shows that two are going the wrong way: `any` annotations have risen from 316 to 383 since Q4 2025, and files importing a Flux store from 82 to 91, while the TypeScript conversion itself has gone well. `quality-metrics.mjs` counts each migration and, given a baseline, fails when one moves the wrong way. Nothing is a target and no PR is blocked by debt it did not add, matching the changed-lines-only rule the lint job already follows. Everything except the type-check count comes from `git grep` against a tree-ish, so `quality-history.mjs` can count past quarters without checking them out or installing their dependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
front-end
Docker builds report
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds scripts that measure frontend migration metrics from Git trees and optionally count TypeScript errors. It adds baseline comparison and quarterly history generation. It stores baseline and historical metric data. It adds package commands, pull-request workflow validation, and documentation for the metric process. Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to This change adds CI enforcement for frontend migration metrics, but a tooling failure can falsely report TypeScript progress and the workflow may expose its token to pull-request-controlled code. These issues should be resolved before merge. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const count = (metric, ref) => { | ||
| if (metric.exts) { | ||
| // `ls-tree` only prefix-matches, so list the roots and filter here. | ||
| const suffixes = metric.exts.map((glob) => glob.replace('*', '')) |
✅ private-cloud · depot-ubuntu-latest-16 — run #20315 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20315 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Skipped testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss ✅ oss · depot-ubuntu-latest-arm-16 — run #20315 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #20315 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #20311 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20311 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #20311 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #20311 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8491 +/- ##
=======================================
Coverage 98.82% 98.82%
=======================================
Files 1622 1622
Lines 66691 66691
=======================================
Hits 65906 65906
Misses 785 785 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Visual Regression19 screenshots compared. See report for details. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 01109bb8-276e-48bf-af2b-6e956bb8b718
📒 Files selected for processing (7)
.github/workflows/frontend-pull-request.ymlfrontend/README.mdfrontend/package.jsonfrontend/scripts/quality-baseline.jsonfrontend/scripts/quality-history.jsonfrontend/scripts/quality-history.mjsfrontend/scripts/quality-metrics.mjs
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| working-directory: frontend | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v5 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,125p' .github/workflows/frontend-pull-request.ymlRepository: Flagsmith/flagsmith
Length of output: 2534
🤖 get_repo_knowledge executed:
get_repo_knowledge Flagsmith/flagsmith /tmp/coderabbit-repo-knowledge/flagsmith-flagsmith-a74d157a/learnings
Length of output: 8740
🏁 Script executed:
cat -n .github/workflows/frontend-pull-request.yml | sed -n '1,125p'Repository: Flagsmith/flagsmith
Length of output: 3199
🏁 Script executed:
printf '%s\n' '--- workflow ---'
sed -n '60,100p' .github/workflows/frontend-pull-request.yml
printf '%s\n' '--- workflow permissions and checkout options ---'
rg -n -C 3 'permissions:|pull_request|checkout@|persist-credentials|npm ci|quality:check' .github/workflows/frontend-pull-request.ymlRepository: Flagsmith/flagsmith
Length of output: 3366
Sensitive Data Exposure
Reachability: External
Exploitability: Moderate
CWE: CWE-522 — Insufficiently Protected Credentials
Disable checkout credential persistence for this job.
The workflow grants GITHUB_TOKEN contents: read permission and then runs repository-controlled npm commands. actions/checkout stores this token in the local Git configuration by default. A malicious pull request can read and exfiltrate it. Set persist-credentials: false under with; the job does not require authenticated Git operations.
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 77-77: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 77-77: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
Source: Linters/SAST tools
| - uses: actions/checkout@v5 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,115p' .github/workflows/frontend-pull-request.ymlRepository: Flagsmith/flagsmith
Length of output: 2534
Security Misconfiguration
Reachability: External
Exploitability: Difficult
CWE: CWE-829 — Inclusion of Functionality from Untrusted Control Sphere
Pin the added actions to immutable commit SHAs.
actions/checkout@v5 and actions/setup-node@v4 use mutable references. Replace all occurrences with approved full commit SHAs.
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 77-77: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 77-77: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 80-80: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
Source: Linters/SAST tools
| const QUARTER_STARTS = [ | ||
| '2025-10-01', | ||
| '2026-01-01', | ||
| '2026-04-01', | ||
| '2026-07-01', | ||
| '2026-10-01', | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Generate quarter starts instead of hardcoding them.
When the current date passes 2027-01-01, the final entry has no successor. The script then reads HEAD and writes its metrics under the stale 2026-Q4 label. Because quality:history persists this data to the tracked history file, the trend becomes incorrect until someone updates the list and regenerates it.
Derive the list from the first quarter through the current quarter.
♻️ Proposed refactor
-// Quarter starts, from the first quarter we want to report on.
-const QUARTER_STARTS = [
- '2025-10-01',
- '2026-01-01',
- '2026-04-01',
- '2026-07-01',
- '2026-10-01',
-]
+// First quarter we want to report on; later quarters are derived up to today.
+const FIRST_QUARTER_START = '2025-10-01'
+
+const quarterStarts = (from) => {
+ const [startYear, startMonth] = from.split('-').map(Number)
+ const now = new Date()
+ const starts = []
+ let year = startYear
+ let month = startMonth
+ while (
+ year < now.getUTCFullYear() ||
+ (year === now.getUTCFullYear() && month <= now.getUTCMonth() + 1)
+ ) {
+ starts.push(`${year}-${String(month).padStart(2, '0')}-01`)
+ month += 3
+ if (month > 12) {
+ month -= 12
+ year += 1
+ }
+ }
+ return starts
+}
+
+const QUARTER_STARTS = quarterStarts(FIRST_QUARTER_START)| } catch (error) { | ||
| return lines(String(error.stdout ?? '')).filter((line) => | ||
| line.includes('error TS'), | ||
| ).length | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Re-throw unparsed tsc failures.
When npx tsc fails before emitting error TS diagnostics, the catch returns 0. The --check path can then treat a failed type-check as progress and pass. Re-throw when the parsed error count is zero, while retaining the count for normal TypeScript diagnostic failures.
🐛 Proposed fix
} catch (error) {
- return lines(String(error.stdout ?? '')).filter((line) =>
+ const errors = lines(String(error.stdout ?? '')).filter((line) =>
line.includes('error TS'),
).length
+ if (errors === 0) throw error
+ return errors
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| return lines(String(error.stdout ?? '')).filter((line) => | |
| line.includes('error TS'), | |
| ).length | |
| } | |
| } catch (error) { | |
| const errors = lines(String(error.stdout ?? '')).filter((line) => | |
| line.includes('error TS'), | |
| ).length | |
| if (errors === 0) throw error | |
| return errors | |
| } |
Counting files made the Flux migration look like a regression when it was not. Splitting one Flux-reading component into five leaves the coupling untouched but triples the file count, and that is most of what happened since Q4 2025: `modals/CreateFlag.js` alone became four files. Imports are flat over the same period, 118 to 118, and splitting them by store shows why the total is a wash: ProjectStore is down 32 to 26 while AccountStore is up 53 to 60, so the progress on one is being cancelled by the other. AccountStore now gets its own row, since it is blocked on its own migration rather than on anybody's willingness. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
ci-cd
2 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
We have four frontend migrations in flight and none of them is measured, so a PR can add to any of them without anyone noticing. This counts them, records the trend by quarter, and fails CI only when one moves the wrong way.
Counting the last four quarters:
js_jsx_filests_tsx_filesany_annotationsts_suppressionsflux_store_importsaccount_store_importsproject_store_importsdark_selectorsraw_colour_hexThree things stand out.
The TypeScript conversion is going well but we are paying for it in
any. 143 JS files down to 106, 523 TS files up to 974, while explicitanyrose 316 to 383. Worth knowing when reviewing a conversion PR.Flux is a wash, and the two halves are moving in opposite directions. Total imports are flat at 118.
ProjectStoreis genuinely coming down (32 to 26), andAccountStoreis cancelling it out (53 to 60). That split matters, because AccountStore is blocked on its own migration rather than on anyone's willingness, so it gets its own row instead of hiding inside the total.Colours and
.darkare drifting down slowly as the token work lands, 342 to 274 and 48 to 45.What's in it
scripts/quality-metrics.mjscounts each migration.--check <baseline>compares againstscripts/quality-baseline.jsonand exits non-zero when a number moves the wrong way.ts_tsx_filesis the one metric where up is good, and it is handled accordingly.scripts/quality-history.mjsbuilds the quarterly table. Every metric except the type-check count comes fromgit grepagainst a tree-ish, so a past quarter is counted without checking it out or installing its dependencies.Migration metricsjob on frontend PRs runs the check and writes the table to the job summary.Choices worth flagging
Ceilings, not targets. A PR only fails if it makes a number worse, so nobody is blocked by debt they did not add. Same principle as the existing
Lint changed filesjob. If an increase is intentional, explain it in the PR and move the baseline in the same commit.Imports, not files. The first version of this counted files and made Flux look like a regression, because splitting one Flux-reading component into five triples a file count without deepening the coupling.
modals/CreateFlag.jsalone became four files. Anything measuring coupling counts imports; only the TS metric counts files, where the file is the unit of work.This is the first CI job to run
tsc. We had 949 type errors with nothing watching the number, which is largely how it got there. The job does not fail on the errors, only on the count rising.Adding a fifth metric later is one entry in the
METRICStable inquality-metrics.mjs.How did you test this code?
npm run quality:metricsagainstmain, and cross-checked every count by hand withgit grep/git ls-tree. The colour count also lines up with the separate DS audit'sraw-color-hexcategory (274 vs 292; the audit's scoping is slightly wider).npm run quality:checkpasses onmainwith the committed baseline. Hand-edited a copy of the baseline to fake both a rise influx_store_importsand a fall ints_tsx_files; both were reported asWORSEand the exit code was 1.npm run quality:historyreproduces the table above fromgit grepalone, with no checkouts. Verified the Flux reading separately by diffing the importing-file lists between the Q4 2025 commit andmain, which is what surfaced the file-count problem: 22 files added, 13 removed, and the removals are the pre-split originals of the additions.js-yaml, andnpx eslintis clean on both scripts.