From 280255ac89b36ab5f9b71f33f2623714756dfeb4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 19:42:50 +0000 Subject: [PATCH 1/2] feat: surface the engine's run diagnostics instead of publishing past them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A run that exits zero is not a run that finished cleanly. Core records every degradation it survived — a language server that never started, a language nothing indexed under, naming that stopped answering — in the analysis it writes. Until now this action ignored that field, so a baseline missing a whole language was committed green and a review comment showed a diagram with nothing to say it was short. Read metadata.run_diagnostics back and put it where the reader already is: an annotation per entry on the run page, the list in the sync job summary, and the same list at the top of the review comment, above the diagram rather than under it — a caveat printed below a picture is read after the picture is believed. Entries carry their own remedy. Where nothing on the reader's side would have changed the outcome the remedy is empty, and the block links Discord instead of inventing an instruction nobody can follow. Diagnostics never fail the run: a degraded analysis is still worth having, and the point is that its reader learns it is degraded. An analysis written by an engine that predates the field, or one that never got written at all, reads as silence rather than an error. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BSnQMqJKj7zbdfjy7KHF9x --- README.md | 18 ++++ action.yml | 25 +++++ scripts/action/build-review-comment.sh | 6 ++ scripts/action/sync-summary.sh | 8 ++ scripts/analysis_diagnostics.py | 121 ++++++++++++++++++++++ tests/test_analysis_diagnostics.py | 137 +++++++++++++++++++++++++ 6 files changed, 315 insertions(+) create mode 100755 scripts/analysis_diagnostics.py create mode 100644 tests/test_analysis_diagnostics.py diff --git a/README.md b/README.md index 1d01f82..a6fafbd 100644 --- a/README.md +++ b/README.md @@ -191,6 +191,24 @@ Every run reports what it resolved, so the answer never has to be inferred from - on a configuration failure, an error annotation and — in review mode — a pull request comment with the fix, so the person who has to add the secret sees it where they are. +### Analysis diagnostics + +A run can finish, exit zero, and still have produced a diagram that is missing something: +a language server that never started, a language nothing indexed under, naming that +stopped answering mid-run. Core records each of those in the analysis it writes +(`metadata.run_diagnostics`), and this action reads them back rather than publishing the +result as though nothing happened: + +- a `::warning::` annotation per degradation, on the run page; +- the same list in the job summary (sync) and at the top of the review comment, above the + diagram — a caveat printed under a picture is read after the picture is believed; +- each entry carries what to do about it. Where nothing on your side would have changed + the outcome, it says so and links Discord instead of inventing an instruction. + +Diagnostics never fail the run: a degraded analysis is still worth having, and the whole +point is that you learn it is degraded. The webview reads the same field out of the +analysis it loads, so a diagram opened there carries the same warning. + ## Model selection All model inputs are optional and are passed directly to Core without action-side validation: diff --git a/action.yml b/action.yml index 2d815f7..c4ea45e 100644 --- a/action.yml +++ b/action.yml @@ -496,6 +496,19 @@ runs: retention-days: 30 if-no-files-found: ignore + # An analysis that finished is not an analysis that finished cleanly. The engine + # records every degradation it survived; without this the run is green and the + # committed diagram is short of a language with nothing to say so. + - name: Read analysis diagnostics + id: sync_diagnostics + if: always() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync' && steps.sync_analyze.outputs.analysis_path != '' + continue-on-error: true + shell: bash + env: + ANALYSIS_PATH: ${{ steps.sync_analyze.outputs.analysis_path }} + DIAGNOSTICS_OUT: ${{ runner.temp }}/codeboarding-diagnostics.md + run: 'python3 "$GITHUB_ACTION_PATH/scripts/analysis_diagnostics.py" --analysis "$ANALYSIS_PATH" --out "$DIAGNOSTICS_OUT"' + - name: Write sync summary if: always() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync' shell: bash @@ -505,6 +518,7 @@ runs: FILES: ${{ steps.sync_commit.outputs.files_written }} STRATEGY: ${{ inputs.sync_strategy }} PR_URL: ${{ steps.sync_commit.outputs.sync_pr_url }} + DIAGNOSTICS_MD: ${{ steps.sync_diagnostics.outputs.markdown_path }} run: "$GITHUB_ACTION_PATH/scripts/action/sync-summary.sh" - name: Analyze pull request @@ -566,6 +580,16 @@ runs: retention-days: 30 if-no-files-found: ignore + - name: Read analysis diagnostics + id: review_diagnostics + if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_analyze.outputs.analysis_path != '' + continue-on-error: true + shell: bash + env: + ANALYSIS_PATH: ${{ steps.review_analyze.outputs.analysis_path }} + DIAGNOSTICS_OUT: ${{ runner.temp }}/codeboarding-diagnostics.md + run: 'python3 "$GITHUB_ACTION_PATH/scripts/analysis_diagnostics.py" --analysis "$ANALYSIS_PATH" --out "$DIAGNOSTICS_OUT"' + - name: Render review diagram id: review_render if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' @@ -622,6 +646,7 @@ runs: BEHIND_BY: ${{ steps.guard.outputs.behind_by }} BASE_REF: ${{ steps.guard.outputs.base_ref }} MERGE_BASE_RESOLVED: ${{ steps.guard.outputs.merge_base_resolved }} + DIAGNOSTICS_MD: ${{ steps.review_diagnostics.outputs.markdown_path }} run: "$GITHUB_ACTION_PATH/scripts/action/build-review-comment.sh" - name: Post review comment diff --git a/scripts/action/build-review-comment.sh b/scripts/action/build-review-comment.sh index d28d0e2..6a8e876 100755 --- a/scripts/action/build-review-comment.sh +++ b/scripts/action/build-review-comment.sh @@ -12,6 +12,12 @@ WEBVIEW_URL="https://app.codeboarding.org/${GITHUB_REPOSITORY}/pull/${PR_NUMBER} BODY="${RUNNER_TEMP}/review-comment.md" printf '### CodeBoarding review\n\n**Status:** %s changed %s\n' "$N_CHANGED" "$COMPONENT_NOUN" > "$BODY" printf '\nSee the full change in [CodeBoarding](%s).\n' "$WEBVIEW_URL" >> "$BODY" +# Above the diagram, not below it: the whole point is that the picture that follows +# is missing something, and a caveat under it is read after the picture is believed. +if [ -n "${DIAGNOSTICS_MD:-}" ] && [ -s "${DIAGNOSTICS_MD}" ]; then + printf '\n' >> "$BODY" + cat "${DIAGNOSTICS_MD}" >> "$BODY" +fi # The diagram compares against the merge base, so commits landed on the base # branch since this PR forked are excluded. Say so rather than hide it. BEHIND="${BEHIND_BY:-0}" diff --git a/scripts/action/sync-summary.sh b/scripts/action/sync-summary.sh index 287b9af..8c2ca8e 100755 --- a/scripts/action/sync-summary.sh +++ b/scripts/action/sync-summary.sh @@ -10,4 +10,12 @@ set -euo pipefail if [ -n "${PR_URL:-}" ]; then echo "- Sync PR: ${PR_URL}" fi + # A baseline that is short a language is committed and read for weeks. The + # bullets above cannot show that, so the engine's own account goes here. + if [ -n "${DIAGNOSTICS_MD:-}" ] && [ -s "${DIAGNOSTICS_MD}" ]; then + echo + echo "#### Analysis diagnostics" + echo + cat "${DIAGNOSTICS_MD}" + fi } >> "$GITHUB_STEP_SUMMARY" diff --git a/scripts/analysis_diagnostics.py b/scripts/analysis_diagnostics.py new file mode 100755 index 0000000..f3ae9ab --- /dev/null +++ b/scripts/analysis_diagnostics.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python3 +"""Turn an analysis.json's run diagnostics into what the run and the reader see. + +CodeBoarding writes ``metadata.run_diagnostics`` for every degradation it +survived — a language server that never started, a language nothing indexed +under, naming that stopped answering. A run that only checks the exit code +publishes those diagrams as though nothing happened, so this reads them back +and gives the run three things: annotations on the failing step, a Markdown +block for the job summary and the review comment, and a count to branch on. +""" + +from __future__ import annotations + +import argparse +import json +import os +import sys +from pathlib import Path + +# Where a reader goes when the cause is ours rather than theirs. +DISCORD_URL = "https://discord.gg/T5zHTJYFuy" + +MAX_RENDERED_ENTRIES = 10 + + +def load_entries(analysis_path: Path) -> list[dict]: + """Diagnostic entries from an analysis document, newest schema or none at all. + + An analysis written before the field existed, or by a build that never sets + it, has nothing to say — that is not an error, so it reads as an empty list. + """ + try: + with analysis_path.open(encoding="utf-8") as handle: + document = json.load(handle) + except (OSError, json.JSONDecodeError) as exc: + print(f"::warning::Could not read run diagnostics from {analysis_path}: {exc}", file=sys.stderr) + return [] + + report = document.get("metadata", {}).get("run_diagnostics") + if not isinstance(report, dict): + return [] + entries = report.get("entries") + return [entry for entry in entries if isinstance(entry, dict)] if isinstance(entries, list) else [] + + +def annotations(entries: list[dict]) -> list[str]: + """One workflow annotation per entry, so the run page shows them without scrolling.""" + lines = [] + for entry in entries: + level = "warning" if entry.get("severity") == "degraded" else "notice" + remedy = entry.get("remedy") or f"Not something you can fix — please report it: {DISCORD_URL}" + lines.append(f"::{level}::{entry.get('title', 'Analysis diagnostic')} {entry.get('detail', '')} {remedy}") + return lines + + +def markdown(entries: list[dict]) -> str: + """The block that goes in the job summary and the review comment. + + Degraded entries lead with a GitHub alert, because the point is that the + diagram beneath is missing something and would otherwise be read as complete. + """ + degraded = [entry for entry in entries if entry.get("severity") == "degraded"] + if not entries: + return "" + + lines: list[str] = [] + if degraded: + lines.append("> [!WARNING]") + lines.append( + "> This analysis did not complete cleanly, so the diagram is missing structure " + "a clean run would have had." + ) + lines.append("") + + for entry in entries[:MAX_RENDERED_ENTRIES]: + title = entry.get("title", "Analysis diagnostic") + count = entry.get("count", 1) + repeated = f" (×{count})" if isinstance(count, int) and count > 1 else "" + lines.append(f"- **{title}**{repeated} — {entry.get('detail', '')}") + remedy = entry.get("remedy") + if remedy: + lines.append(f" - {remedy}") + else: + lines.append(f" - Nothing on your side causes this. Please report it on [Discord]({DISCORD_URL}).") + + hidden = len(entries) - MAX_RENDERED_ENTRIES + if hidden > 0: + lines.append(f"- …and {hidden} more, in the run log.") + + return "\n".join(lines) + "\n" + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--analysis", required=True, help="Path to the analysis.json the run produced") + parser.add_argument("--out", required=True, help="File to write the Markdown block to") + args = parser.parse_args(argv) + + analysis_path = Path(args.analysis) + entries = load_entries(analysis_path) if analysis_path.is_file() else [] + degraded = sum(1 for entry in entries if entry.get("severity") == "degraded") + + for line in annotations(entries): + print(line) + + body = markdown(entries) + out_path = Path(args.out) + out_path.parent.mkdir(parents=True, exist_ok=True) + out_path.write_text(body, encoding="utf-8") + + github_output = os.environ.get("GITHUB_OUTPUT") + if github_output: + with open(github_output, "a", encoding="utf-8") as handle: + handle.write(f"degraded={degraded}\n") + handle.write(f"entries={len(entries)}\n") + handle.write(f"markdown_path={out_path if body else ''}\n") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_analysis_diagnostics.py b/tests/test_analysis_diagnostics.py new file mode 100644 index 0000000..cdec27b --- /dev/null +++ b/tests/test_analysis_diagnostics.py @@ -0,0 +1,137 @@ +"""What the run publishes when the engine finished with less than it should have.""" + +import json +import os +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "scripts")) + +import analysis_diagnostics as ad + + +def _entry(**overrides: object) -> dict: + entry = { + "code": "static.language_server_unavailable", + "severity": "degraded", + "title": "CSharp could not be analyzed", + "detail": "Its language server failed to start.", + "remedy": "Install the CSharp toolchain, then run the analysis again.", + "subject": "CSharp", + "count": 1, + } + entry.update(overrides) + return entry + + +def _document(entries: list[dict]) -> dict: + degraded = sum(1 for e in entries if e["severity"] == "degraded") + return { + "metadata": { + "run_diagnostics": { + "version": 1, + "degraded": degraded, + "notices": len(entries) - degraded, + "entries": entries, + } + } + } + + +class LoadEntriesTests(unittest.TestCase): + def _write(self, content: str) -> Path: + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + path = Path(tmp.name) / "analysis.json" + path.write_text(content, encoding="utf-8") + return path + + def test_an_analysis_without_the_field_has_nothing_to_say(self): + """Older engines wrote no diagnostics; that is silence, not a failure.""" + self.assertEqual(ad.load_entries(self._write(json.dumps({"metadata": {"depth_cap": 2}}))), []) + + def test_unparseable_json_does_not_take_the_run_down_with_it(self): + self.assertEqual(ad.load_entries(self._write("{not json")), []) + + def test_entries_are_read_back_intact(self): + entries = ad.load_entries(self._write(json.dumps(_document([_entry()])))) + self.assertEqual([e["code"] for e in entries], ["static.language_server_unavailable"]) + + +class AnnotationTests(unittest.TestCase): + def test_a_degraded_entry_annotates_as_a_warning(self): + self.assertTrue(ad.annotations([_entry()])[0].startswith("::warning::")) + + def test_a_notice_stays_a_notice(self): + self.assertTrue(ad.annotations([_entry(severity="notice")])[0].startswith("::notice::")) + + def test_an_entry_nobody_can_act_on_points_at_the_community(self): + self.assertIn(ad.DISCORD_URL, ad.annotations([_entry(remedy="")])[0]) + + +class MarkdownTests(unittest.TestCase): + def test_a_clean_run_renders_nothing(self): + self.assertEqual(ad.markdown([]), "") + + def test_a_degradation_leads_with_an_alert(self): + body = ad.markdown([_entry()]) + self.assertTrue(body.startswith("> [!WARNING]")) + self.assertIn("Install the CSharp toolchain", body) + + def test_notices_alone_do_not_raise_an_alert(self): + body = ad.markdown([_entry(severity="notice")]) + self.assertNotIn("[!WARNING]", body) + self.assertIn("CSharp could not be analyzed", body) + + def test_a_repeated_entry_shows_its_count(self): + self.assertIn("(×3)", ad.markdown([_entry(count=3)])) + + def test_a_long_list_is_capped_and_says_so(self): + entries = [_entry(subject=str(i), title=f"Entry {i}") for i in range(ad.MAX_RENDERED_ENTRIES + 4)] + body = ad.markdown(entries) + self.assertIn("…and 4 more, in the run log.", body) + self.assertNotIn("Entry 12", body) + + +class MainTests(unittest.TestCase): + def _run(self, document: object | None) -> tuple[Path, dict[str, str]]: + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + root = Path(tmp.name) + analysis = root / "analysis.json" + if document is not None: + analysis.write_text(json.dumps(document), encoding="utf-8") + out = root / "diagnostics.md" + github_output = root / "github-output" + github_output.write_text("", encoding="utf-8") + os.environ["GITHUB_OUTPUT"] = str(github_output) + self.addCleanup(os.environ.pop, "GITHUB_OUTPUT", None) + + ad.main(["--analysis", str(analysis), "--out", str(out)]) + + outputs = dict( + line.split("=", 1) for line in github_output.read_text(encoding="utf-8").splitlines() if "=" in line + ) + return out, outputs + + def test_a_degraded_run_reports_its_count_and_a_body(self): + out, outputs = self._run(_document([_entry(), _entry(subject="Go", title="Go could not be analyzed")])) + self.assertEqual(outputs["degraded"], "2") + self.assertEqual(outputs["markdown_path"], str(out)) + self.assertTrue(out.read_text(encoding="utf-8")) + + def test_a_clean_run_publishes_an_empty_path_so_callers_skip_the_block(self): + _, outputs = self._run(_document([])) + self.assertEqual(outputs["degraded"], "0") + self.assertEqual(outputs["markdown_path"], "") + + def test_a_missing_analysis_is_not_an_error(self): + """The step runs on always(); an analysis that never got written has no diagnostics.""" + _, outputs = self._run(None) + self.assertEqual(outputs["degraded"], "0") + + +if __name__ == "__main__": + unittest.main() From 6c66dd86645aa83f3a84c9beb85779b91ee05301 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 19:43:44 +0000 Subject: [PATCH 2/2] style: keep the diagnostics block free of em dashes The same title/detail/remedy strings render in the webview, which enforces the house rule with a test, so the prose the action wraps them in should read the same way rather than switching voice between surfaces. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BSnQMqJKj7zbdfjy7KHF9x --- scripts/analysis_diagnostics.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/analysis_diagnostics.py b/scripts/analysis_diagnostics.py index f3ae9ab..d6ece19 100755 --- a/scripts/analysis_diagnostics.py +++ b/scripts/analysis_diagnostics.py @@ -48,7 +48,7 @@ def annotations(entries: list[dict]) -> list[str]: lines = [] for entry in entries: level = "warning" if entry.get("severity") == "degraded" else "notice" - remedy = entry.get("remedy") or f"Not something you can fix — please report it: {DISCORD_URL}" + remedy = entry.get("remedy") or f"Nothing on your side causes this; please report it: {DISCORD_URL}" lines.append(f"::{level}::{entry.get('title', 'Analysis diagnostic')} {entry.get('detail', '')} {remedy}") return lines @@ -76,7 +76,7 @@ def markdown(entries: list[dict]) -> str: title = entry.get("title", "Analysis diagnostic") count = entry.get("count", 1) repeated = f" (×{count})" if isinstance(count, int) and count > 1 else "" - lines.append(f"- **{title}**{repeated} — {entry.get('detail', '')}") + lines.append(f"- **{title}**{repeated}: {entry.get('detail', '')}") remedy = entry.get("remedy") if remedy: lines.append(f" - {remedy}")