chore: enforce CI quality gates -- flake8 blocks, ESLint added, security blocks (OPE-55)#262
Conversation
…ity scan blocks (OPE-55) Backend: - Added .flake8 config: checks F (pyflakes) + E9 (runtime) errors only, ignores style-only rules (whitespace, indentation, line length) - Removed 44 unused imports and 4 unused variables via autoflake - Fixed f-string without placeholders in dna_extractor.py - Removed unused global declaration in user_limits.py - Removed redundant 'pass' from exception classes and abstract methods - Added metrics re-export with noqa comment in dependencies.py - flake8 now passes with ZERO errors Frontend: - Added ESLint + plugins to devDependencies (eslint, @eslint/js, globals, eslint-plugin-react-hooks, eslint-plugin-react-refresh, typescript-eslint) - Added 'lint' and 'lint:fix' scripts to package.json CI workflow: - Removed '|| true' from flake8 -- lint failures now block merges - Added 'Lint (ESLint)' step to frontend job - Moved lint before build (fail fast) - Changed Trivy exit-code from '0' to '1' -- CRITICAL/HIGH vulns block - Removed continue-on-error from trufflehog secret scanning - Security scan now conditional on path changes (not every push) 289 tests pass. Zero flake8 errors.
|
@DevanshuNEU is attempting to deploy a commit to the Dev's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds CI linting (flake8, ESLint, TypeScript checks), enhances security scans (Trivy, trufflehog) with conditional execution, introduces backend Flake8 config and frontend ESLint deps/scripts, and performs widespread import/unused-code cleanup across backend modules. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Repo as Repository
participant Backend as Backend job
participant Frontend as Frontend job
participant Trivy as Trivy scanner
participant Trufflehog as Trufflehog
GH->>Repo: push / PR event
GH->>Backend: run install (incl. flake8) -> lint -> tests -> build
GH->>Frontend: run install -> typescript check -> eslint -> tests -> build
Backend->>Trivy: run container/image scan (fail on HIGH/CRITICAL)
Backend->>Trufflehog: run secrets scan
Frontend->>Trivy: conditional scan if frontend changed
Frontend->>Trufflehog: conditional secrets check
Trivy-->>GH: report / exit non-zero on issues
Trufflehog-->>GH: report findings
GH-->>Repo: status checks updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/services/dna_extractor.py (1)
826-826: Remove the no-op list comprehension.Line 826 computes a list and discards it, so this check currently has no effect.
♻️ Proposed cleanup
- # Check for test directory structure - [d for d in ['tests', 'test'] if (repo_path / d).exists()]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/dna_extractor.py` at line 826, The list comprehension "[d for d in ['tests', 'test'] if (repo_path / d).exists()]" is a no-op because its result is discarded; replace it with a meaningful check such as using any(...) to test presence (e.g. if any((repo_path / d).exists() for d in ['tests','test']): ...) or assign the list to a variable (e.g. tests_dirs = [d for d in ['tests','test'] if (repo_path / d).exists()]) and use that variable where needed; reference the existing repo_path and exists() calls to locate and update the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 109-110: Remove the job-level conditional that gates the entire
security job on backend/frontend changes (the "if: ${{
needs.changes.outputs.backend == 'true' || needs.changes.outputs.frontend ==
'true' }}" line) so the security job always runs; instead, keep any path-based
gating at the step level (e.g., inside the Trivy/scan step) so only the scanner
step skips when irrelevant, ensuring other security checks and secret scans
still execute for all commits.
In `@backend/.flake8`:
- Line 2: The flake8 config currently sets max-line-length = 120 but excludes
E501 by using "select = E9,F,W6" (and only listed E501 in extend-ignore), so
line-too-long checks aren’t enforced; update the config to include E501 in the
selected codes (e.g., add E to the select or include E501 explicitly) or remove
the restrictive "select = E9,F,W6" so the default E codes (including E501) are
checked, and keep "max-line-length = 120" as the enforced limit.
---
Nitpick comments:
In `@backend/services/dna_extractor.py`:
- Line 826: The list comprehension "[d for d in ['tests', 'test'] if (repo_path
/ d).exists()]" is a no-op because its result is discarded; replace it with a
meaningful check such as using any(...) to test presence (e.g. if any((repo_path
/ d).exists() for d in ['tests','test']): ...) or assign the list to a variable
(e.g. tests_dirs = [d for d in ['tests','test'] if (repo_path / d).exists()])
and use that variable where needed; reference the existing repo_path and
exists() calls to locate and update the logic.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.github/workflows/ci.ymlbackend/.flake8backend/dependencies.pybackend/main.pybackend/routes/auth.pybackend/routes/github.pybackend/routes/repos.pybackend/routes/search.pybackend/routes/ws_playground.pybackend/services/auth.pybackend/services/dependency_analyzer.pybackend/services/dna_extractor.pybackend/services/exceptions.pybackend/services/indexer_optimized.pybackend/services/indexing_events.pybackend/services/input_validator.pybackend/services/rate_limiter.pybackend/services/repo_manager.pybackend/services/search_enhancer.pybackend/services/search_v2/summary_generator.pybackend/services/search_v3/code_graph_ranker.pybackend/services/search_v3/embedding_provider.pybackend/services/search_v3/query_understanding.pybackend/services/search_v3/search_engine.pybackend/services/style_analyzer.pybackend/services/supabase_service.pybackend/services/user_limits.pyfrontend/package.json
💤 Files with no reviewable changes (6)
- backend/services/input_validator.py
- backend/routes/github.py
- backend/services/search_v3/embedding_provider.py
- backend/routes/ws_playground.py
- backend/services/auth.py
- backend/services/exceptions.py
… comprehension 1. .flake8: removed dead extend-ignore (select already filters). Clarified that style codes are intentionally excluded, only bugs checked. 2. dna_extractor.py: removed discarded list comprehension on line 826 that checked for test directories but never used the result. Skipped: removing security job path gate (trufflehog already scans diff only, Trivy on docs-only PRs wastes CI minutes for zero value). 289 tests pass. Zero flake8 errors.
…ing issues to warnings The existing eslint.config.js used reactHooks.configs.flat.recommended which doesn't exist in eslint-plugin-react-hooks@5.x. Rewrote config using tseslint.config() flat config pattern with explicit plugin registration. 92 pre-existing issues (43 no-explicit-any, 34 no-unused-vars, etc.) downgraded from error to warn. ESLint now passes with 0 errors, 92 warnings. These will be fixed incrementally -- not in a CI gates PR. Verified locally: - bun run lint: 0 errors, 92 warnings (exit 0) - bun run tsc --noEmit: clean - bun run build: clean - bun run test: 13 passed - Backend: 289 passed, 0 flake8 errors
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/eslint.config.js (1)
31-31:varsIgnorePatternnot set alongsideargsIgnorePattern.Variables prefixed with
_(e.g.,_unused) will still trigger warnings; only function-argument names are silenced. AddingvarsIgnorePattern: '^_'is the conventional pairing.♻️ Proposed fix
- '@typescript-eslint/no-unused-vars': ['warn', { argsIgnorePattern: '^_' }], + '@typescript-eslint/no-unused-vars': ['warn', { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/eslint.config.js` at line 31, The rule configuration for '@typescript-eslint/no-unused-vars' only sets argsIgnorePattern and therefore still warns for variables prefixed with '_' (e.g., _unused); update the rule object for '@typescript-eslint/no-unused-vars' to include varsIgnorePattern: '^_' alongside argsIgnorePattern so both function arguments and variables prefixed with '_' are ignored. Locate the '@typescript-eslint/no-unused-vars' entry in eslint.config.js and add the varsIgnorePattern property to the same rule configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/eslint.config.js`:
- Line 31: The rule configuration for '@typescript-eslint/no-unused-vars' only
sets argsIgnorePattern and therefore still warns for variables prefixed with '_'
(e.g., _unused); update the rule object for '@typescript-eslint/no-unused-vars'
to include varsIgnorePattern: '^_' alongside argsIgnorePattern so both function
arguments and variables prefixed with '_' are ignored. Locate the
'@typescript-eslint/no-unused-vars' entry in eslint.config.js and add the
varsIgnorePattern property to the same rule configuration.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
frontend/eslint.config.js
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Problem
CI quality gates were fake:
|| true-- literally never failedexit-code: '0'andcontinue-on-error: trueFix
Backend lint (flake8)
.flake8config: checks pyflakes errors (unused imports, undefined names, shadowed vars)passstatementsflake8now runs WITHOUT|| true-- lint failures block mergesFrontend lint (ESLint)
bun run lintscriptSecurity scan
0to1-- CRITICAL/HIGH vulns blockcontinue-on-errorfrom trufflehog secret scanningCI flow improvement
Test
289 backend tests pass. Zero flake8 errors.
Closes OPE-55
Summary by CodeRabbit
Tests
Chores