Resolve SonarCloud BLOCKER findings + safe CRITICAL S1192 slice - #365
Merged
Conversation
… S1192
Scope: every BLOCKER-severity finding, plus the CRITICAL S1192
(duplicated string literal) findings in production code -- excluding
the code-generator modules (fbs-gen/proto-gen), which are an interim
in-house toolchain not worth polishing further (see
adr/0017-in-house-fbs-proto-codegen and the project's own generated-code
disclaimer). CRITICAL S3776 (cognitive complexity) findings are
deliberately NOT touched here: ~85 of them, several in hot-path
decode/encode code with complexity in the 40s-60s (BitpackedEncodingDecoder,
PcoEncodingDecoder, DictFilter, PrimitiveEncodingEncoder) -- these need
careful, individually-reviewed refactors with benchmarks, not a blind
autonomous sweep, and are a natural follow-up PR. S115 (TimeUnit's
PascalCase enum constants) is also left alone: renaming a public enum
used throughout the codebase and by downstream consumers is a breaking
API change that needs an explicit decision, not something to fold into
a lint-cleanup PR.
BLOCKER fixes:
- fsst: LossyPerfectHashTable and ShortCodeTable both had a private
instance field named `slots` differing only in case from the
`SLOTS` size constant (S1845). Renamed the field to `table` in both
(encapsulated, no external API, hot-path logic untouched -- pure
identifier rename, verified against the full fsst test suite).
- Two Raincloud corpus tests' `corpusIsHydrated()` (S2699, "add an
assertion") were flagged for having no assertion, but assumeTrue
*is* the check here -- a visible skip marker when the corpus isn't
hydrated, not a pass/fail assertion, and there's nothing else to
assert. Suppressed with a comment explaining why, rather than
inventing a no-op assertion just to satisfy the rule.
- CalciteDemo.profileFullScan (S2699) is a profiling harness with a
real gap: it printed a row count but never checked it against what
was written. Added a genuine assertion (count == rows written) --
cheap, doesn't interfere with profiling, and catches a broken scan
before trusting the profile it produces.
CRITICAL S1192 fixes (production code only):
- SparseEncodingDecoder: "indices"/"values" role literals (used in
15 call sites combined) extracted to ROLE_INDICES/ROLE_VALUES.
- Predicate: the six single-value leaf records (Eq/Neq/Lt/Gt/Lte/Gte)
each null-check their `value` component against the same literal;
extracted to a shared VALUE constant on the sealed interface. The
two-component leaves (Between/And/Or) were already below the
duplication threshold and are left as split literals ("lo"/"hi",
"left"/"right") -- clearer than a single ambiguous shared constant.
- VortexInspectorTui: three DataState-rendering blocks (dictionary
preview, per-chunk stats, column data) repeated the same "loading"
suffix, numbered-entry format, and column-data label prefix;
extracted to LOADING_SUFFIX/ENTRY_FORMAT/DATA_COLUMN_PREFIX.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjU7zWghUiJauRxK58nyUs
dfa1
force-pushed
the
fix/sonar-blocker-critical
branch
from
September 3, 2026 17:11
b724ff9 to
5aea5de
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
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.
Summary
Triaged sweep of the SonarCloud backlog, scoped deliberately (see commit message for full reasoning):
fsstfield/constant name clashes (S1845, pure rename, hot-path logic untouched), two Raincloud corpus tests whereassumeTruegenuinely is the check (S2699 — suppressed with a comment, not gamed with a no-op assertion), and one real gap inCalciteDemo's profiling harness (added a genuine row-count assertion).SparseEncodingDecoder,Predicate,VortexInspectorTui— extracted to named constants, no behavior change.Explicitly out of scope (left for dedicated follow-up, not an oversight):
BitpackedEncodingDecoder,PcoEncodingDecoder,DictFilter,PrimitiveEncodingEncoder). These need careful, individually-benchmarked refactors, not a blind pass.TimeUnit's PascalCase enum constants) — renaming a public enum used throughout the codebase and by downstream consumers is a breaking API change needing an explicit decision.fbs-gen/proto-gen(the in-house codegen modules) — interim tooling, not worth polishing per existing project direction.Test plan
./mvnw verify -pl fsst -am/-pl reader -am/-pl cli -am/-pl calcite -am/-pl integration -am— all green./mvnw verify— full reactor build, all modules green./mvnw verify -pl integration -am -Dit.test=DocsConsistencyTest— living-docs claims still resolve🤖 Generated with Claude Code
https://claude.ai/code/session_01LjU7zWghUiJauRxK58nyUs