fix: stop discarding the health report

The engine runs health checks on every analysis, full or incremental, and
writes health_report.json beside the graph it produced. Nothing kept it:
sync deleted the file instead of installing the fresh one, and review never
uploaded it. The only surviving copy was inside the Actions cache, which
has no download API, so neither the extension nor the webview could read
health information at all. It has been that way since the v2 rewrite in
474d40a, which removed the file as a v1 leftover without noticing that the
engine still produced it.

Sync now installs the report it just generated, and removes a stale one
when a run produced none, matching how the other optional artifacts are
handled. Review ships it in the artifact next to the analyses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: Svilen-Stefanov

README.md

@@ -60,9 +60,10 @@ The uploaded artifact holds both sides of the comparison, so a reader can reprod
 |---|---|
 | `analysis.json` | the head analysis, at `head_sha` |
 | `base_analysis.json` | the analysis it was compared against, at `merge_base_sha` |
-| `metadata.json` | both SHAs, `merge_base_resolved`, `seed_source`, `chain_depth`, PR number |
+| `health_report.json` | the head's health findings, when the engine produced any |
+| `metadata.json` | which commits those graphs describe — see [the field list](docs/COMMIT_STRATEGY.md#names) |
 
-Artifacts are kept 30 days. Reading the default branch's committed baseline instead of `base_analysis.json` would drift from the merge base in exactly the way described above.
+The action requests 30-day retention; a repository or organisation policy can shorten it, so treat an artifact's own `expired` flag as the truth rather than any fixed window. Reading the default branch's committed baseline instead of `base_analysis.json` would drift from the merge base in exactly the way described above.
 
 ### Reused analysis
 
@@ -127,8 +128,9 @@ Sync mode commits only Core's persisted incremental-analysis state under `.codeb
 - `static_analysis.pkl`
 - `static_analysis.sha`
 - `codeboarding_version.json` when emitted by Core
+- `health/health_report.json`
 
-It does **not** render or commit architecture Markdown. Existing v1-generated `.codeboarding/*.md` and `docs/development/architecture.md` files carrying CodeBoarding's generated badge, plus `.codeboarding/health/health_report.json`, are removed on the first v2 sync. Hand-written Markdown and user-authored CodeBoarding configuration are preserved.
+It does **not** render or commit architecture Markdown. Existing v1-generated `.codeboarding/*.md` and `docs/development/architecture.md` files carrying CodeBoarding's generated badge are removed on the first v2 sync. Hand-written Markdown and user-authored CodeBoarding configuration, including `health/health_config.json` and `health/.healthignore`, are preserved.
 
 Create `.github/workflows/codeboarding-sync.yml`:
 

docs/COMMIT_STRATEGY.md

@@ -27,10 +27,13 @@ branch, so generated files cannot conflict during a merge.
 | `static_analysis.pkl` | LSP/CFG cache and the cluster baseline incremental needs |
 | `static_analysis.sha` | the warm-start gate for the pickle |
 | `codeboarding_version.json` | when Core emits it |
+| `health/health_report.json` | the warnings, read without regenerating them |
 
-Everything else generated is removed on sync: v1 architecture Markdown and
-`health/health_report.json`. Hand-written Markdown and user configuration
-(`.codeboardingignore`, health config) are preserved.
+The engine writes a health report on every run, full or incremental, so sync
+installs the fresh one and drops a stale one when a run produced none. v1
+architecture Markdown is removed. Hand-written Markdown and user configuration
+(`.codeboardingignore`, `health/health_config.json`, `health/.healthignore`) are
+preserved.
 
 **Delivery (`sync_strategy`).** The committed set is identical either way; only
 how it reaches the branch differs. `push` fast-forwards it directly.
@@ -100,6 +103,7 @@ lists a pull request's artifacts and takes the newest.
     codeboarding-review-<run_id>-<attempt>/
       analysis.json        the head analysis, at metadata.head_sha
       base_analysis.json   what it was compared against, at metadata.merge_base_sha
+      health_report.json   the head's health findings, when the engine wrote one
       metadata.json        which commits those graphs describe
 
 | `metadata.json` field | Meaning |

scripts/action/build-review-artifact.sh

@@ -8,6 +8,11 @@ cp "$ANALYSIS_PATH" "${RUNNER_TEMP}/cb-review-artifact/analysis.json"
 # branch's committed baseline instead would drift from the merge base exactly as
 # the review itself used to.
 cp "$BASE_ANALYSIS_PATH" "${RUNNER_TEMP}/cb-review-artifact/base_analysis.json"
+# The engine writes this next to the analysis it came from. Ship it when present
+# so a pull request's warnings are readable without rerunning the analysis.
+HEALTH_REPORT="$(dirname "$ANALYSIS_PATH")/health/health_report.json"
+[ ! -f "$HEALTH_REPORT" ] || cp "$HEALTH_REPORT" "${RUNNER_TEMP}/cb-review-artifact/health_report.json"
+
 # base_sha stays the event's base branch tip for consumers that key on it;
 # merge_base_sha records the commit the diagram actually compared against.
 # pr_base_sha carries the same value under the name the webview already reads:

scripts/action/install-sync.sh

@@ -40,18 +40,25 @@ for name in "${artifacts[@]}"; do
   printf '%s\n' "$target"
 done
 
-# Remove identifiable v1 Markdown and its generated health report.
+# The engine writes a health report on every run, full or incremental. Install it
+# beside the analysis so the extension and webview can read warnings without
+# regenerating, and remove a stale one when a run produced none.
+health_source="$ANALYSIS_DIR/health/health_report.json"
+health_target="$output/health/health_report.json"
+if [ -f "$health_source" ]; then
+  mkdir -p "$output/health"
+  cp "$health_source" "$health_target"
+elif [ -e "$health_target" ]; then
+  rm -f "$health_target"
+fi
+printf '%s\n' "$health_target"
+
+# Remove identifiable v1 Markdown.
 marker='https://img.shields.io/badge/Generated%20by-CodeBoarding'
 for legacy in "$output"/*.md "$CHECKOUT_DIR/docs/development/architecture.md"; do
   [ -f "$legacy" ] || continue
   grep -Fq "$marker" "$legacy" || continue
   rm -f "$legacy"
   printf '%s\n' "$legacy"
 done
-legacy="$output/health/health_report.json"
-if [ -e "$legacy" ]; then
-  rm -f "$legacy"
-  printf '%s\n' "$legacy"
-fi
-
 echo "installed=$installed" >> "$GITHUB_OUTPUT"

tests/test_action_cache.py

@@ -337,7 +337,7 @@ def setUp(self) -> None:
     def tearDown(self) -> None:
         self.temp_dir.cleanup()
 
-    def test_it_ships_both_graphs_and_the_commit_they_describe(self) -> None:
+    def _build(self) -> subprocess.CompletedProcess:
         output = self.root / "github-output"
         result = subprocess.run(
             [str(ROOT / "scripts" / "action" / "build-review-artifact.sh")],
@@ -361,6 +361,10 @@ def test_it_ships_both_graphs_and_the_commit_they_describe(self) -> None:
             check=False,
         )
         self.assertEqual(result.returncode, 0, result.stderr or result.stdout)
+        return result
+
+    def test_it_ships_both_graphs_and_the_commit_they_describe(self) -> None:
+        self._build()
 
         artifact = self.root / "cb-review-artifact"
         self.assertEqual(json.loads((artifact / "analysis.json").read_text())["components"], ["head"])
@@ -379,6 +383,26 @@ def test_it_ships_both_graphs_and_the_commit_they_describe(self) -> None:
         self.assertEqual(metadata["seed_source"], "pr-chain")
 
 
+class ReviewHealthArtifactTests(ReviewArtifactTests):
+    """The engine writes a health report beside every analysis it produces."""
+
+    def test_it_ships_the_health_report_when_the_engine_wrote_one(self) -> None:
+        (self.root / "health").mkdir()
+        (self.root / "health" / "health_report.json").write_text('{"overall_score": 0.9}', encoding="utf-8")
+
+        self._build()
+
+        report = self.root / "cb-review-artifact" / "health_report.json"
+        self.assertEqual(report.read_text(encoding="utf-8"), '{"overall_score": 0.9}')
+
+    def test_it_still_builds_when_no_health_report_was_written(self) -> None:
+        self._build()
+
+        artifact = self.root / "cb-review-artifact"
+        self.assertTrue((artifact / "analysis.json").is_file())
+        self.assertFalse((artifact / "health_report.json").exists())
+
+
 class CachePathParityTests(unittest.TestCase):
     """actions/cache derives its lookup version from the path strings, so a save
     under a different path than the restore can never be found again."""

tests/test_action_sync.py

@@ -244,6 +244,50 @@ def test_installs_core_manifest_and_preserves_user_configuration(self) -> None:
             self.assertEqual(result.returncode, 0, result.stderr or result.stdout)
             self.assertEqual(architecture.read_text(encoding="utf-8"), "hand-written architecture\n")
 
+    def test_installs_the_health_report_the_engine_produced(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            checkout = root / "checkout"
+            analysis = root / "analysis"
+            fake_core = root / "core"
+            static_analyzer = fake_core / "static_analyzer"
+            for directory in (checkout / ".codeboarding", analysis / "health", static_analyzer):
+                directory.mkdir(parents=True)
+            (fake_core / "utils.py").write_text(
+                "ANALYSIS_FILENAME = 'analysis.json'\nFINGERPRINT_FILENAME = 'fingerprint.json'\n",
+                encoding="utf-8",
+            )
+            (static_analyzer / "__init__.py").touch()
+            (static_analyzer / "analysis_cache.py").write_text(
+                "STATIC_ANALYSIS_PKL = 'static_analysis.pkl'\nSTATIC_ANALYSIS_SHA = 'static_analysis.sha'\n",
+                encoding="utf-8",
+            )
+            (analysis / "analysis.json").write_text("{}\n", encoding="utf-8")
+            (analysis / "health" / "health_report.json").write_text('{"overall_score": 1.0}', encoding="utf-8")
+
+            result = subprocess.run(
+                [str(INSTALL_SYNC)],
+                cwd=checkout,
+                env={
+                    "PATH": os.environ["PATH"],
+                    "PYTHONPATH": str(fake_core),
+                    "ACTION_PATH": str(ROOT),
+                    "ANALYSIS_DIR": str(analysis),
+                    "CHECKOUT_DIR": str(checkout),
+                    "GITHUB_OUTPUT": str(root / "github-output"),
+                },
+                capture_output=True,
+                text=True,
+                check=False,
+            )
+
+            self.assertEqual(result.returncode, 0, result.stderr or result.stdout)
+            installed = checkout / ".codeboarding" / "health" / "health_report.json"
+            self.assertEqual(installed.read_text(encoding="utf-8"), '{"overall_score": 1.0}')
+            # Printed paths are what the delivery step stages, so an installed
+            # file that is never printed is silently left out of the commit.
+            self.assertIn(str(installed), result.stdout.splitlines())
+
     def test_empty_review_is_successful(self) -> None:
         with tempfile.TemporaryDirectory() as tmp:
             root = Path(tmp)