Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Ignore the generated health report in the sample workflow

Once sync commits this newly listed file, the workflow example below still omits .codeboarding/health/health_report.json from paths-ignore. In the documented sync_strategy: pull_request setup, merging the rolling PR can therefore trigger another full sync job because the merge commit is not necessarily authored with the bot email handled by guard.sh; that redundant run consumes analysis time and LLM quota even though it should find no baseline change. Add the generated health path to the sample ignore list, as the repository's own sync workflow already does.

Useful? React with 👍 / 👎.


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`:

Expand Down
10 changes: 7 additions & 3 deletions docs/COMMIT_STRATEGY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 |
Expand Down
5 changes: 5 additions & 0 deletions scripts/action/build-review-artifact.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear seeded health reports before packaging the review

When an incremental review is seeded from the merge-base state or a previous PR-chain run and Core produces no health report, the seeded health/health_report.json remains beside ANALYSIS_PATH, so this line uploads old findings as though they describe the current head. This is fresh evidence beyond the previously reported sync issue: analyze_review copies cached state into head_state before running incremental analysis, and this newly added artifact path now exposes that stale file. Remove the seeded report before analysis or track whether the current run emitted it.

Useful? React with 👍 / 👎.


# 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:
Expand Down
3 changes: 3 additions & 0 deletions scripts/action/deliver-sync.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 14 additions & 7 deletions scripts/action/install-sync.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Comment on lines +48 to +50

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear the seeded report before checking for fresh output

When a previous baseline already contains a health report but the current engine run produces none, analyze_sync copies that old report into ANALYSIS_DIR via seed_state before analysis starts (scripts/action/analyze.sh:88). This condition therefore sees the seeded file as fresh and copies it back, making the promised stale-removal branch unreachable after the first report exists. Remove the report from the seeded state before invoking Core, or otherwise track whether this run actually emitted it.

Useful? React with 👍 / 👎.

elif [ -e "$health_target" ]; then
rm -f "$health_target"
fi
printf '%s\n' "$health_target"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include the health report in files_written

Whenever this path is installed and staged, deliver-sync.sh:73-75 still computes files_written using find -maxdepth 1 and a whitelist of only the root-level artifacts. The public action output and sync summary therefore undercount persisted artifacts by one whenever a health report exists, despite the updated documentation classifying it as persisted analysis state. Extend that counter to include health/health_report.json.

Useful? React with 👍 / 👎.


# 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"
26 changes: 25 additions & 1 deletion tests/test_action_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")],
Expand All @@ -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"])
Expand All @@ -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."""
Expand Down
44 changes: 44 additions & 0 deletions tests/test_action_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading