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..d6ece19 --- /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"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 + + +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()