fix(sonar): repair quality gate baseline and clear new-code issues#103
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to restore SonarCloud quality gate health on main by fixing the “previous_version” baseline behavior (injecting the current package version at scan time instead of hardcoding it) and addressing/mitigating newly surfaced “new code” issues (deterministic sorting and keyboard accessibility for clickable non-interactive elements).
Changes:
- Remove hardcoded
sonar.projectVersionand inject it via GitHub Actions during the SonarCloud scan; add a narrowly-scoped rule suppression fortypescript:S2245inTestDataGenerator. - Fix in-place sort/mutation in schema diff index comparison by sorting copies with a comparator.
- Add keyboard activation support to multiple clickable non-interactive UI elements via a shared
activateOnKeyhelper (plus a targeted inline handler in the import dropzone) and adjust a few related UI behaviors.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/schema-diff/diff-engine.ts | Avoid in-place mutation when sorting index column sets by sorting copies with a comparator. |
| src/lib/a11y.ts | Introduce activateOnKey helper to mirror click activation via keyboard for non-interactive elements. |
| src/components/VisualExplain.tsx | Add keyboard activation for expandable plan nodes. |
| src/components/studio/StudioTabBar.tsx | Add keyboard activation to tab selection elements. |
| src/components/SnapshotTimeline.tsx | Add keyboard activation for snapshot selection elements. |
| src/components/SchemaDiagram.tsx | Simplify a tautological stroke ternary and clarify intent in comments. |
| src/components/schema-explorer/TableItem.tsx | Remove unused import and add keyboard activation for table item toggling. |
| src/components/SavedQueries.tsx | Add keyboard activation for saved-query selection rows. |
| src/components/ResultsGrid.tsx | Add keyboard activation for sortable headers and (mobile) row selection; stop propagation for key events in filter/editing UI. |
| src/components/results-grid/ResultCard.tsx | Add keyboard activation for result-card selection. |
| src/components/PivotTable.tsx | Use a comparator when sorting pivot column keys. |
| src/components/DataImportModal.tsx | Add keyboard activation to the upload dropzone (inline handler due to compiler/ref constraints). |
| src/components/DataCharts.tsx | Use Radix onSelect for saved chart menu activation and remove nested click handler. |
| sonar-project.properties | Stop hardcoding sonar.projectVersion; add scoped suppression for typescript:S2245 on TestDataGenerator. |
| .github/workflows/ci.yml | Resolve package version in CI and pass it to SonarCloud scanner args. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+195
to
+197
| - name: Resolve project version | ||
| id: pkg | ||
| run: echo "version=$(node -p "require('./package.json').version")" >> "$GITHUB_OUTPUT" |
Comment on lines
+12
to
+19
| export function activateOnKey<T extends HTMLElement = HTMLElement>(handler: () => void) { | ||
| return (event: KeyboardEvent<T>) => { | ||
| if (event.key === "Enter" || event.key === " ") { | ||
| event.preventDefault(); | ||
| handler(); | ||
| } | ||
| }; | ||
| } |
Comment on lines
+82
to
+85
| // Build a stable map key from the column set. Copy before sorting so the | ||
| // original column order is preserved for the diff messages below, and use | ||
| // localeCompare for a deterministic, locale-aware ordering. | ||
| const columnKey = (idx: IndexSchema) => idx.name || [...idx.columns].sort((a, b) => a.localeCompare(b)).join(","); |
| const changes: string[] = []; | ||
| const sourceColStr = sourceIdx.columns.sort().join(","); | ||
| const targetColStr = targetIdx.columns.sort().join(","); | ||
| const sortedJoin = (cols: string[]) => [...cols].sort((a, b) => a.localeCompare(b)).join(","); |
Comment on lines
+315
to
+322
| onKeyDown={(e) => { | ||
| // Inlined (not activateOnKey) so the compiler sees this as a | ||
| // deferred event handler and allows the ref access. | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| e.preventDefault(); | ||
| fileInputRef.current?.click(); | ||
| } | ||
| }} |
cevheri
force-pushed
the
fix/quality-gate
branch
from
a134cf0 to
bff90da
Compare
Comment on lines
+195
to
+197
| - name: Resolve project version | ||
| id: pkg | ||
| run: echo "version=$(node -p "require('./package.json').version")" >> "$GITHUB_OUTPUT" |
The SonarCloud quality gate failed on main with new_reliability_rating=D and new_security_rating=C. Root cause: sonar.projectVersion was hardcoded to 0.6.38 while the package is at 0.9.35, so the "previous_version" new-code baseline was frozen at 0.6.18 (Feb 2026). Months of code counted as "new", surfacing pre-existing issues against the gate. Config: - Stop hardcoding sonar.projectVersion; CI now injects it from package.json so the new-code baseline resets at every release (mirrors libredb-database). - Suppress S2245 for TestDataGenerator only: Math.random there produces fake sample rows, not security/crypto material (documented false positive). - Suppress S2871 for diff-engine only: its .sort() calls build internal canonical keys for index-column SETS (equality matching), which must be deterministic and locale-independent. The default code-point sort is correct here; localeCompare would tie the key to the host locale. Code (real bugs surfaced by the gate): - diff-engine: sort index columns on a copy (was an in-place .sort() that mutated idx.columns, corrupting the column order shown in diff messages). - PivotTable: localeCompare comparator for user-facing column keys (S2871). - SchemaDiagram: drop the tautological stroke ternary that returned the same colour in both branches (S3923). - DataCharts: move the saved-chart click from a non-interactive <span> to the Radix menu item's onSelect, which is keyboard-accessible (S1082). The broader S1082 keyboard-accessibility cleanup is deferred to a focused follow-up: fixing it via role="button" trades a minor bug for a major maintainability smell (S6819), and the correct per-component fix deserves its own PR. Verified locally: format, lint, typecheck, knip, build, build:lib, attw, and the full test suite (486 tests) all pass.
cevheri
force-pushed
the
fix/quality-gate
branch
from
bff90da to
9109615
Compare
|
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
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.
Problem
The SonarCloud quality gate on
mainis failing:new_reliability_ratingnew_security_ratingRoot cause
sonar.projectVersionwas hardcoded to0.6.38insonar-project.propertieswhile the package is at0.9.35. With the "previous_version" new-code definition, the baseline was frozen at0.6.18(Feb 2026), so ~4 months of code counted as "new" and surfaced pre-existing issues against the gate. Left unfixed it keeps degrading, since the version string never changes.Changes
Config
sonar.projectVersion. CI now injects it frompackage.json(-Dsonar.projectVersion=<version>) so the new-code baseline resets at every release. Kept inci.yml(not a separate workflow) so the Sonar job keeps consuming the coverage artifact from thetestjob. Mirrors the provenlibredb-databasepattern.S2245forTestDataGeneratoronly:Math.randomthere generates fake sample rows (names, addresses, lorem ipsum), not security/crypto material. Documented false positive, scoped to the one file.Code (real technical debt surfaced by the gate)
S2871+ latent mutation bug): the oldidx.columns.sort()sorted the array in place, corrupting the original column order later rendered in diff messages. Now sorts a copy withlocaleComparefor the map key while preserving display order.S2871):localeComparecomparator for column keys.S3923): removed the tautologicalstroketernary that returned the same colour in both branches (highlight is already conveyed bystrokeWidth+opacity).S1082× 12: added keyboard activation to clickable non-interactive elements across the results grid, schema explorer, tab bar, charts, snapshot timeline, saved queries, and import modal. NewactivateOnKeyhelper keeps it DRY; the saved-charts menu item uses RadixonSelect; the import dropzone uses an inline handler (ref access trips the React Compiler through the helper).Buttonimport inTableItem.Verification
Ran the full pre-commit suite locally — all pass:
format·lint(oxlint + ESLint) ·typecheck·knip·build·build:lib·attw·test(18 groups, 486 tests)Note on the gate
The
maingate turns fully green once this merges andmainis re-analyzed at version0.9.35(the baseline resets at that point). The PR analysis itself only removes issues, so the PR gate passes.