quality-debt: PR #1000 review feedback (high)

[marcusquinn]

Unactioned Review Feedback

Source PR: #1000
File: general
Reviewers: coderabbit
Findings: 1
Max severity: high

---

HIGH: coderabbit (coderabbitai[bot])

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @.agents/scripts/quality-sweep-helper.sh:
- Around line 908-918: The SQL invocations interpolate $limit directly which
allows non-numeric values; update the CLI handlers that build queries (the
blocks using db "$SWEEP_DB" ... LIMIT $limit and the related commands
cmd_sonarcloud_query and cmd_dedup) to validate and sanitize $limit before use:
ensure $limit is a positive integer (e.g. reject or normalize values that do not
match ^[0-9]+$), set a safe default or exit with an error on invalid input, and
only pass the validated numeric value into the SQL string so no non-numeric or
;-separated payload can reach sqlite3.
- Around line 770-780: The API endpoint used to fetch repository issues is
missing the required "/search" suffix, so update the endpoint variable in the
quality-sweep helper (the local variable named endpoint in the block that calls
codacy_api_call) from
"/analysis/organizations/${provider}/${org}/repositories/${repo}/issues" to
include "/search" at the end; keep the surrounding error handling (the
codacy_api_call invocation, response variable, the db update using SWEEP_DB and
run_id, and the page_count extraction from response) unchanged so subsequent
parsing of response.data and pagination fields will work with the Codacy v3 API.

🧹 Nitpick comments (2)

.agents/scripts/quality-sweep-helper.sh (2)

872-921: Significant duplication — consider parameterizing query/summary/export by source.

cmd_codacy_query, cmd_codacy_summary, cmd_codacy_export, and cmd_codacy_status are near-identical copies of their cmd_sonarcloud_* counterparts, differing only in the source='codacy' filter and label strings. With CodeFactor and CodeRabbit integrations on the roadmap (t245.3+), each new source would require copying all four functions again.

A single parameterized set of functions accepting source as an argument would eliminate ~200 lines of duplication and make future integrations trivial:

♻️ Sketch of a parameterized approach

-cmd_codacy_query() {
-    ...
-    local where="WHERE source='codacy'"
-    ...
-}
-
-cmd_sonarcloud_query() {
-    ...
-    local where="WHERE source='sonarcloud'"
-    ...
-}
+cmd_source_query() {
+    local source="$1"; shift
+    ...
+    local where="WHERE source='$(echo "$source" | sed "s/'/''/g")'"
+    ...
+}

Then in routers:

# cmd_codacy
query) cmd_source_query "codacy" "$@" ;;

# cmd_sonarcloud
query) cmd_source_query "sonarcloud" "$@" ;;

---

1085-1085: Non-deterministic f.message in GROUP BY query.

The dedup_query selects f.message without aggregation while grouping by file, line, type. SQLite allows this (returning an arbitrary row's value), but the message shown could be from any contributing source. For JSON/CSV output (lines 1103, 1106) which includes message, this could be confusing if the SonarCloud and Codacy messages differ significantly.

Consider wrapping it in GROUP_CONCAT(DISTINCT f.message) or selecting the message from the highest-severity row. That said, for a dedup overview, showing any representative message is likely acceptable — so this is just a heads-up.

View comment

---

Auto-generated by quality-feedback-helper.sh scan-merged. Review each finding and either fix the code or dismiss with a reason.

[johnwaldo]

Investigated — the files referenced in this issue are now in supervisor-archived/ or archived/ directories and are no longer active code. The review feedback applies to archived scripts that have been superseded.

Recommend closing as won't-fix (archived code).

[johnwaldo]

Recommend closing: The target file quality-sweep-helper.sh has been moved to scripts/archived/. This is no longer active code.

[marcusquinn]

Completed via PR #3968.

• Model: sonnet (anthropic/claude-sonnet-4-6) — pulse supervisor merge
• Attempts: 1
• Duration: PR merged by pulse supervisor

[github-actions]

Completed via PR #3968. merged to main.

[marcusquinn]

Closing — resolved by PR #3968 (merged).