diff --git a/README.md b/README.md index 3746fa6..e8310a6 100644 --- a/README.md +++ b/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`: diff --git a/docs/COMMIT_STRATEGY.md b/docs/COMMIT_STRATEGY.md index f0b37f3..48acb8a 100644 --- a/docs/COMMIT_STRATEGY.md +++ b/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--/ 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 | diff --git a/scripts/action/build-review-artifact.sh b/scripts/action/build-review-artifact.sh index 014dc5a..5c85c98 100755 --- a/scripts/action/build-review-artifact.sh +++ b/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: diff --git a/scripts/action/deliver-sync.sh b/scripts/action/deliver-sync.sh index f647855..bbc454c 100755 --- a/scripts/action/deliver-sync.sh +++ b/scripts/action/deliver-sync.sh @@ -86,6 +86,9 @@ if [ "$remote_sha" != "$BASE_SHA" ]; then exit 0 fi +# The -I filters keep a run that changed nothing from committing: analysis.json +# carries generated_at and health_report.json carries timestamp, both rewritten +# every run. Dropping either filter turns every sync into a commit. if git diff --cached --quiet || git diff --cached --quiet -I '"generated_at"' -I '"timestamp"'; then git reset -q close_stale_pr diff --git a/scripts/action/install-sync.sh b/scripts/action/install-sync.sh index b5b7725..47c81c3 100755 --- a/scripts/action/install-sync.sh +++ b/scripts/action/install-sync.sh @@ -40,7 +40,20 @@ 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 @@ -48,10 +61,4 @@ for legacy in "$output"/*.md "$CHECKOUT_DIR/docs/development/architecture.md"; d 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" diff --git a/tests/test_action_cache.py b/tests/test_action_cache.py index f7a7fa3..e1d2f71 100644 --- a/tests/test_action_cache.py +++ b/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.""" diff --git a/tests/test_action_sync.py b/tests/test_action_sync.py index 073f494..b88e39f 100644 --- a/tests/test_action_sync.py +++ b/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)