From 220f5fc1ec20ea5821b2e193e6d953b7c23c8611 Mon Sep 17 00:00:00 2001 From: mARTin-B78 Date: Tue, 6 Oct 2026 03:03:23 +0200 Subject: [PATCH] feat(qa): group sonar issues per rule Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 5 ++++- architecture/sonar-triage.md | 4 ++-- scripts/sonar_triage.mjs | 42 ++++++++++++++++++++++++------------ tests/sonar_triage.test.mjs | 36 +++++++++++++++++++++++++++---- 4 files changed, 66 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6e838c..5b48e0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi --- ## [Unreleased] +### Changed +- QA: the Sonar triage script groups medium/large findings per rule instead of per (rule, file): one GitHub issue per rule with a file/line table (max 40 rows, 'and N more'; split into 'part n/m' beyond 40 locations); dedupe marker still carries all Sonar keys. + ### Changed - Conversation Playground: the round microphone button is red (pressed, "Stop microphone") while the mic is hot (live or recording) and blue ("Start microphone") when off; it returns to blue on stop, auto-stop and mic errors. The pulse ring only animates when reduced motion is not requested. - Conversation Playground: the metrics (Token Usage and STT / LLM first token / TTS first audio TTFB charts) now sit in the right-hand Latency column below Turn history instead of a second "Metrics" tab; the Conversation | Metrics tablist is removed. Charts gain axis titles ("Time (ms)", "Time"), a legend and three y-ticks. @@ -15,7 +18,7 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi - Conversation Playground: the metrics charts render at the column width (re-rendered on resize) so axis, tick and legend text is 11px instead of ~8px; a single point is centred with a larger dot. ### Added -- QA: `scripts/sonar_triage.mjs` (`npm run sonar:triage`), a local SonarQube triage loop (dry-run default): classifies findings small/medium/large/skipped via allow/skip lists, files one GitHub issue per (rule, file) with a dedupe marker, hands them to the multica `sonar-fixer` agent, and can auto-fix small ones as a local commit. See `architecture/sonar-triage.md`. +- QA: `scripts/sonar_triage.mjs` (`npm run sonar:triage`), a local SonarQube triage loop (dry-run default): classifies findings small/medium/large/skipped via allow/skip lists, files one GitHub issue per rule (file/line table, max 40 locations, larger rules split into 'part n/m') with a dedupe marker, hands them to the multica `sonar-fixer` agent, and can auto-fix small ones as a local commit. See `architecture/sonar-triage.md`. - Conversation Playground: Metrics view (Conversation | Metrics tabs) with Pipecat-style TTFB line charts for STT, LLM first token and TTS, plus session Token Usage (prompt/completion/total, "n/a" when the backend reports none). The stream now requests `stream_options.include_usage` (dropped and retried on 400/422) and forwards `usage` in the final stats event; the TTS value is now the time to the first audio chunk instead of including the remaining LLM generation time. - Conversation Playground: a Thinking on/off switch next to the Language Model (default off, setting `conversation_thinking`). Off sends `chat_template_kwargs.enable_thinking=false` (plus `think:false` for Ollama) with each reply request and never touches the system prompt; the empty-reply error now says to turn Thinking off. diff --git a/architecture/sonar-triage.md b/architecture/sonar-triage.md index 60df4bc..e152eaa 100644 --- a/architecture/sonar-triage.md +++ b/architecture/sonar-triage.md @@ -10,7 +10,7 @@ SonarQube /api/issues/search (GET, paged, OPEN/CONFIRMED/REOPENED) +----+-----------------------------+ SMALL MEDIUM / LARGE | (--apply --fix-small) | (--apply) - task file -> `claude -p` group by (rule, file) -> `gh issue create` + task file -> `claude -p` group by rule (<=40 locations) -> `gh issue create` -> ONE local commit (labels sonar, agent-fix; dedupe marker) `fix(sonar): ...` | `multica issue create --assignee sonar-fixer` @@ -28,7 +28,7 @@ SonarQube /api/issues/search (GET, paged, OPEN/CONFIRMED/REOPENED) `--dry-run` (default) | `--apply` | `--since-new-code` (default) | `--all` | `--json ` | `--max-issues N` | `--cap N` (GitHub issues per run, default 10) | `--fix-small` (needs `--apply`) | `--list-rules`. ## GitHub issues -One per (rule, file); title `[sonar] : in `; body holds Sonar keys, lines, rule link, effort, approach, acceptance. Hidden marker ``; an open `sonar` issue containing any of the keys suppresses a new one. Bodies contain paths/lines only, no code. If `gh auth status` fails the script reports and continues. +One per rule (all files); title `[sonar] : in N file(s)`; body has a table of file, line, Sonar key, message (capped at 40 rows, then 'and N more'), rule link, effort, approach, acceptance. A rule with more than 40 locations is split into numbered parts (`part 1/3`), 40 locations each. Hidden marker `` lists every key of the part; an open `sonar` issue containing any of the keys suppresses a new one. Bodies contain paths/lines only, no code. If `gh auth status` fails the script reports and continues. ## multica hookup After a GitHub issue is created the script runs `multica issue create --project --assignee sonar-fixer --description-stdin` (the multica issue links the GitHub URL; assigning starts the agent run). Override with `MULTICA_AGENT`, `MULTICA_PROJECT`. Failure is reported, not fatal. Agent `sonar-fixer` instructions: see the end of this file. diff --git a/scripts/sonar_triage.mjs b/scripts/sonar_triage.mjs index a61da81..68e572e 100644 --- a/scripts/sonar_triage.mjs +++ b/scripts/sonar_triage.mjs @@ -73,25 +73,39 @@ export function triage(issues, cfg) { const countBy = (xs, f) => xs.reduce((m, x) => (m[f(x)] = (m[f(x)] || 0) + 1, m), {}); -export function groupIssues(items) { - const g = new Map(); - for (const i of items) { const k = `${i.rule}\u0000${i.path}`; (g.get(k) || g.set(k, []).get(k)).push(i); } - return [...g.values()].map((is) => { - const keys = is.map((i) => i.key).sort(); - return { rule: is[0].rule, path: is[0].path, issues: is, keys, effort: is.reduce((s, i) => s + parseEffort(i.effort), 0), - title: `[sonar] ${is[0].rule}: ${(is[0].message || '').slice(0, 60).replace(/\s+/g, ' ')} in ${is[0].path}`.slice(0, 200) }; - }); +export const MAX_LOCATIONS = 40; + +// One group per rule; a rule with more than `size` locations is split into numbered parts. +export function groupIssues(items, size = MAX_LOCATIONS) { + const byRule = new Map(); + for (const i of items) (byRule.get(i.rule) || byRule.set(i.rule, []).get(i.rule)).push(i); + const out = []; + for (const [rule, all] of byRule) { + all.sort((a, b) => a.path.localeCompare(b.path) || (a.line ?? 0) - (b.line ?? 0) || a.key.localeCompare(b.key)); + const parts = Math.ceil(all.length / size); + for (let p = 0; p < parts; p++) { + const is = all.slice(p * size, (p + 1) * size); + const files = new Set(is.map((i) => i.path)).size; + const tag = parts > 1 ? ` (part ${p + 1}/${parts})` : ''; + out.push({ rule, part: p + 1, parts, issues: is, files, keys: is.map((i) => i.key).sort(), effort: is.reduce((s, i) => s + parseEffort(i.effort), 0), + title: `[sonar] ${rule}: ${(is[0].message || '').slice(0, 60).replace(/\s+/g, ' ')} in ${files} file(s)${tag}`.slice(0, 200) }); + } + } + return out; } export function issueBody(g) { + const shown = g.issues.slice(0, MAX_LOCATIONS); + const more = g.issues.length - shown.length; return [ ``, `Rule: [${g.rule}](${HOST}/coding_rules?open=${encodeURIComponent(g.rule)}&rule_key=${encodeURIComponent(g.rule)})`, - `File: \`${g.path}\` | Estimated effort: ${g.effort} min`, - '', '| Sonar key | Line | Message |', '|---|---|---|', - ...g.issues.map((i) => `| ${i.key} | ${i.line ?? '-'} | ${(i.message || '').replace(/\|/g, '\\|')} |`), + `${g.parts > 1 ? `Part ${g.part}/${g.parts} | ` : ''}${g.issues.length} location(s) in ${g.files} file(s) | Estimated effort: ${g.effort} min`, + '', '| File | Line | Sonar key | Message |', '|---|---|---|---|', + ...shown.map((i) => `| \`${i.path}\` | ${i.line ?? '-'} | ${i.key} | ${(i.message || '').replace(/\|/g, '\\|')} |`), + ...(more > 0 ? ['', `and ${more} more (keys are in the marker above).`] : []), '', '## Suggested approach', - 'Read the rule description linked above, fix each listed line at the root cause, keep behaviour unchanged unless the rule demands otherwise.', + 'Read the rule description linked above, fix each listed location at the root cause, keep behaviour unchanged unless the rule demands otherwise.', '', '## Acceptance', '- A re-scan clears the listed Sonar keys.', '- Follow CLAUDE.md and PROJECT_MAP.md; `node scripts/check-frontend.mjs` and the relevant unit tests pass (tests mock the API, import from tests/_guard.js).', @@ -133,7 +147,7 @@ function fixSmall(small, log) { const head = run('git', ['rev-parse', 'HEAD']).stdout.trim(); const dir = mkdtempSync(join(tmpdir(), 'sonar-small-')); const task = join(dir, 'task.md'); - const lines = groupIssues(small).flatMap((g) => g.issues.map((i) => `- ${i.rule} ${g.path}:${i.line ?? '?'} (${i.key}): ${i.message}`)); + const lines = groupIssues(small).flatMap((g) => g.issues.map((i) => `- ${i.rule} ${i.path}:${i.line ?? '?'} (${i.key}): ${i.message}`)); writeFileSync(task, `Fix ONLY these SonarQube issues, changing behaviour nowhere. Follow CLAUDE.md.\n${lines.join('\n')}\n\nThen run \`node scripts/check-frontend.mjs\` and \`node --test tests/*.test.mjs\`. If they pass, make exactly ONE local commit \`fix(sonar): \` ending with the line "Co-Authored-By: Claude Sonnet 5.5 ". Never push. Touch no other file; if an issue needs a design decision, skip it and say so.\n`); const tools = ['Read', 'Edit', 'Grep', 'Glob', 'Bash(node scripts/check-frontend.mjs)', 'Bash(node --test:*)', 'Bash(git add:*)', 'Bash(git commit:*)', 'Bash(git status:*)', 'Bash(git diff:*)']; const r = run('claude', ['-p', '--allowedTools', ...tools], `Read the task file ${task} and do exactly what it says.`); @@ -169,7 +183,7 @@ export async function main(argv, deps = {}) { if (a.json) writeFileSync(a.json, JSON.stringify(res, null, 2)); if (deps.noActions) return res; const groups = groupIssues([...res.medium, ...res.large]); - log(`\nMedium+ groups (rule, file): ${groups.length}`); + log(`\nMedium+ groups (per rule, max 40 locations): ${groups.length}`); ghIssues(groups, a.apply, a.cap, log); if (a.fixSmall) { if (a.apply) fixSmall(res.small, log); else log('fix-small needs --apply (dry-run).'); } return res; diff --git a/tests/sonar_triage.test.mjs b/tests/sonar_triage.test.mjs index 81cf3c2..c6338ae 100644 --- a/tests/sonar_triage.test.mjs +++ b/tests/sonar_triage.test.mjs @@ -27,13 +27,41 @@ test('shipped config loads and skips server.py bind', () => { const c = loadConfig(); assert.equal(classify(mk({ rule: 'python:S104', component: 'p:server.py' }), c).cls, 'skipped'); }); -test('grouping by rule+file and body marker', () => { - const g = groupIssues([mk({ key: 'b' }), mk({ key: 'a' }), mk({ key: 'c', component: 'p:core/y.py' })].map((i) => ({ ...i, path: i.component.slice(2) }))); +const P = (i) => ({ ...i, path: i.component.slice(2) }); +test('grouping per rule across files, marker keeps dedupe', () => { + const g = groupIssues([mk({ key: 'b' }), mk({ key: 'a' }), mk({ key: 'c', component: 'p:core/y.py' }), mk({ key: 'd', rule: 'python:S9' })].map(P)); assert.equal(g.length, 2); - const body = issueBody(g.find((x) => x.keys.length === 2)); - assert.match(body, //); + const one = g.find((x) => x.rule === 'python:S1481'); + assert.equal(one.files, 2); + const body = issueBody(one); + assert.match(body, //); + assert.match(body, /\| `core\/y.py` \| 3 \| c \|/); assert.ok(hasMarker([body], ['b'])); assert.ok(!hasMarker([body], ['zzz'])); + assert.doesNotMatch(one.title, /part/); +}); +test('rule with >40 locations is split into numbered parts', () => { + const many = Array.from({ length: 85 }, (_, n) => P(mk({ key: 'k' + String(n).padStart(3, '0'), component: `p:core/f${n % 7}.py`, line: n }))); + const g = groupIssues(many); + assert.deepEqual(g.map((x) => x.issues.length), [40, 40, 5]); + assert.match(g[0].title, /part 1\/3/); assert.match(g[2].title, /part 3\/3/); + assert.equal(new Set(g.flatMap((x) => x.keys)).size, 85); + assert.doesNotMatch(issueBody(g[0]), /and \d+ more/); +}); +test('body caps at 40 rows with "and N more" but marker lists all keys', () => { + const g = groupIssues(Array.from({ length: 45 }, (_, n) => P(mk({ key: 'k' + String(n).padStart(2, '0'), line: n }))), 100)[0]; + const body = issueBody(g); + assert.equal(body.split('\n').filter((l) => l.startsWith('| `core/x.py`')).length, 40); + assert.match(body, /and 5 more/); + assert.ok(hasMarker([body], ['k44'])); +}); +test('per-run cap limits created issues (dry-run)', async () => { + const out = []; + const issues = Array.from({ length: 12 }, (_, n) => mk({ key: 'z' + n, rule: 'python:R' + n })); + await main(['--dry-run', '--all'], { token: 't', cfg, log: (s) => out.push(s), fetchFn: async () => page(issues) }); + const txt = out.join('\n'); + assert.match(txt, /Medium\+ groups \(per rule, max 40 locations\): 12/); + assert.match(txt, /10 of \d+ to create \(cap 10\)|gh unauthenticated/); }); const page = (issues) => ({ ok: true, json: async () => ({ issues, paging: { total: issues.length } }) });