fix: allow candidate-level stale reference suppressions#67
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces candidate-level suppressions for the stale-reference rule, allowing users to narrow down suppressions to specific missing reference candidates rather than suppressing all missing references in a file. This includes updating the documentation, validation scripts, fixtures, and the core suppression matching logic in skills/lucid/scripts/lucid.py. Feedback was provided to optimize the apply_suppressions function; the current implementation performs a linear scan over all suppressions for each finding, resulting in O(N * M) complexity, which can be optimized to O(N + M) by grouping suppressions by (rule, path) in a dictionary lookup.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| active: list[dict[str, Any]] = [] | ||
| suppressed: list[dict[str, Any]] = [] | ||
| for finding in findings: | ||
| suppression = suppression_lookup.get((finding["rule"], finding["path"])) | ||
| suppression = next( | ||
| ( | ||
| item | ||
| for item in suppressions | ||
| if finding_matches_suppression(finding, item) | ||
| ), | ||
| None, | ||
| ) |
There was a problem hiding this comment.
The current implementation of apply_suppressions performs a linear scan (O(M)) over all suppressions for every single finding, resulting in an O(N * M) time complexity.
By grouping the suppressions by (rule, path) in a dictionary lookup, we can reduce the search space to only the relevant suppressions for each finding, optimizing the average-case lookup time to O(1) per finding and overall complexity to O(N + M).
| active: list[dict[str, Any]] = [] | |
| suppressed: list[dict[str, Any]] = [] | |
| for finding in findings: | |
| suppression = suppression_lookup.get((finding["rule"], finding["path"])) | |
| suppression = next( | |
| ( | |
| item | |
| for item in suppressions | |
| if finding_matches_suppression(finding, item) | |
| ), | |
| None, | |
| ) | |
| suppression_lookup: dict[tuple[str, str], list[dict[str, str]]] = {} | |
| for item in suppressions: | |
| suppression_lookup.setdefault((item["rule"], item["path"]), []).append(item) | |
| active: list[dict[str, Any]] = [] | |
| suppressed: list[dict[str, Any]] = [] | |
| for finding in findings: | |
| candidates = suppression_lookup.get((finding["rule"], finding["path"]), []) | |
| suppression = next( | |
| ( | |
| item | |
| for item in candidates | |
| if finding_matches_suppression(finding, item) | |
| ), | |
| None, | |
| ) |
Summary
candidatematching forstale-referenceentries inlucid.ignore.jsonTesting
python3 scripts/validate-skill.pypython3 scripts/validate-no-dangerous-io.pypython3 scripts/validate-evals.pypython3 scripts/validate-docs.pypython3 scripts/validate-package-skill.pypython3 skills/lucid/scripts/lucid.py verify --root . --strictpython3 skills/lucid/scripts/lucid.py audit --root . --format terminal