ci: add pull-request quality checks#737
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe changes add repository quality automation and file-handling rules, update Biome scripts, adjust Windows native helper resolution and thumbnail queue handling, and tighten TypeScript types and Electron API test references. ChangesTooling and quality automation
Application maintenance
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
This was referenced Jul 9, 2026
This was referenced Jul 10, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/quality.yml (1)
42-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a format check step to CI.
The
format:checkscript (biome format .) was added topackage.jsonbut isn't exercised in CI. While the PR summary states formatting is not currently required, adding it as a non-blocking advisory step (like the i18n check) would surface formatting drift early without blocking PRs.💡 Suggested addition after the Lint step
- name: Lint if: ${{ !cancelled() && steps.install.outcome == 'success' }} run: npm run lint + + - name: Check formatting (advisory) + if: ${{ !cancelled() && steps.install.outcome == 'success' }} + continue-on-error: true + run: npm run format:check🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/quality.yml around lines 42 - 60, Add a non-blocking advisory CI step after the existing Lint step in the workflow, matching the install-success and cancellation conditions used by the other checks, and run the package.json format:check script to surface formatting drift without failing the job.electron/ipc/paths/binaries.ts (1)
56-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid duplicating
getNativeArchTag()andgetPrebundledNativeHelperPath()logic.Line 56 inlines the Windows arch-tag mapping that already exists in
getNativeArchTag()(lines 37–39), and lines 65–71 reconstruct the same prebundled path thatgetPrebundledNativeHelperPath(binaryName)already returns. If the arch-tag convention or prebundled directory layout changes, both copies must be updated in sync.Consider reusing the existing helpers:
♻️ Proposed refactor
export function resolvePreferredWindowsNativeHelperPath( helperDirectory: string, binaryName: string, ): string { - const windowsArchTag = process.arch === "arm64" ? "win32-arm64" : "win32-x64"; const buildOutputPath = resolveUnpackedAppPath( "electron", "native", helperDirectory, "build", "Release", binaryName, ); - const prebundledPath = resolveUnpackedAppPath( - "electron", - "native", - "bin", - windowsArchTag, - binaryName, - ); + const prebundledPath = getPrebundledNativeHelperPath(binaryName);Also applies to: 65-71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/ipc/paths/binaries.ts` at line 56, Remove the duplicated Windows architecture mapping and prebundled helper-path construction in the relevant binary-resolution logic. Reuse getNativeArchTag() for the architecture tag and getPrebundledNativeHelperPath(binaryName) for the prebundled path, preserving the existing behavior while centralizing these conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/quality.yml:
- Around line 42-60: Add a non-blocking advisory CI step after the existing Lint
step in the workflow, matching the install-success and cancellation conditions
used by the other checks, and run the package.json format:check script to
surface formatting drift without failing the job.
In `@electron/ipc/paths/binaries.ts`:
- Line 56: Remove the duplicated Windows architecture mapping and prebundled
helper-path construction in the relevant binary-resolution logic. Reuse
getNativeArchTag() for the architecture tag and
getPrebundledNativeHelperPath(binaryName) for the prebundled path, preserving
the existing behavior while centralizing these conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fa05f28c-05f6-448c-90dd-292d273cc4e2
📒 Files selected for processing (12)
.eslintrc.cjs.gitattributes.github/workflows/quality.ymlbiome.jsonelectron/ipc/paths/binaries.tselectron/ipc/register/assets.tspackage.jsonsrc/components/launch/SourceSelector.tsxsrc/components/launch/contexts/HudInteractionContext.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/audio/waveform/waveform.worker.tssrc/lib/exporter/streamingDecoder.test.ts
💤 Files with no reviewable changes (2)
- .eslintrc.cjs
- src/components/video-editor/VideoPlayback.tsx
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
1 participant
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.
Description
Adds a least-privilege code-quality workflow for pull requests and pushes to
main. It performs one clean Node 22 install, then runs TypeScript, Biome lint, and the full Vitest suite. Translation parity remains visibly advisory while the existing, mergeable #710 supplies the missing locale content.This also repairs the minimum baseline required by those checks: deterministic text/binary attributes, CSS Modules parsing, generated-output scanner ignores, duplicated Biome config, removal of the unused ESLint config, separate lint/format scripts, and eight mechanical typing/empty-callback lint findings.
Motivation
Recordly currently has no PR workflow that executes repository code. Type errors, lint regressions, and test failures can therefore reach
mainbefore the manual packaging workflows run.Type of Change
Related Issue(s)
Architecture remediation phase 0. Translation hard-gating follows #710; this PR deliberately does not duplicate that contributor's nine locale files.
Testing Guide
npx tsc --noEmit npm run lint npm testResults: typecheck passed; lint passed with 21 existing hook warnings; Vitest passed twice, with the concise run reporting 96/96 files and 841/841 tests. CSS Modules lint and workflow validation also passed.
Risk and non-goals
No user-visible runtime behavior is intended. Formatting is exposed as a separate check but is not required yet because current
mainneeds a dedicated format-only normalization. After merge, a maintainer still needs to make theRepository qualitycheck required in branch protection.Checklist
Summary by CodeRabbit
New Features
Improvements
Maintenance
Tests