From 572c0096ef14f62ca5ea8d70356b3a7d5c4c6686 Mon Sep 17 00:00:00 2001 From: Svilen Stefanov Date: Wed, 19 Aug 2026 23:29:37 +0200 Subject: [PATCH] fix(review): keep reviews 14 days and base graphs 30 The review artifact is the dominant storage cost, being the one kept for weeks, so it drops from 30 days to 14. Base graphs drop from 60 to 30, which still leaves a 16-day window in which a base is reused rather than republished. Modelled at the largest repository we have, that is roughly 3,700 analyses a month within half a Team allowance, against 1,850 before. The renewal threshold moves with the review retention, because it has to: a review references a base by id for its whole life, so a base with less than that remaining is renewed. Ten days against a fourteen-day review would leave a reference dangling for four. A test now fails if the two drift apart, since nothing else would notice. What a pull request open longer than fourteen days loses is the rendered analysis, until someone asks for it again. That costs one incremental over the pull request: the base graph is kept longer and needs no re-analysis. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 +- action.yml | 33 +++++++++++++++++---------------- docs/COMMIT_STRATEGY.md | 12 ++++++++---- tests/test_action_state.py | 9 +++++++++ 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 604e41b..42b6447 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ Every bundle the action publishes carries a `metadata.json` with a `kind` of `review`, `base` or `warmstart`. Check it rather than inferring from the payload: a base bundle is otherwise indistinguishable from a head one. -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. +The action requests 14-day retention for it; a repository or organisation policy can shorten that, so treat an artifact's own `expired` flag as the truth rather than any fixed window. When a pull request outlives its review artifact, `/codeboarding` regenerates it at the cost of one incremental over the pull request — the base graph is kept longer and does not need re-analyzing. ### Reused analysis diff --git a/action.yml b/action.yml index 8c245e2..218599d 100644 --- a/action.yml +++ b/action.yml @@ -222,7 +222,9 @@ runs: REPOSITORY: ${{ github.repository }} ARTIFACT_NAME: ${{ steps.state.outputs.base_name }} DEST: ${{ runner.temp }}/cb-state/${{ github.action }}/base - RENEW_WITHIN_DAYS: '30' + # Must match the review retention below: a review references a base by + # id for its whole life, so a base with less than that left is renewed. + RENEW_WITHIN_DAYS: '14' run: "$GITHUB_ACTION_PATH/scripts/action/fetch-state.sh" - name: Fetch this pull request's last analysis @@ -283,14 +285,13 @@ runs: with: name: codeboarding-base-${{ steps.state.outputs.cfg_hash }}-${{ steps.sync_commit.outputs.baseline_sha }} path: ${{ runner.temp }}/cb-state/${{ github.action }}/out/base - # Longer than a review's 30 days on purpose, and the margin is the whole + # Longer than a review's 14 days on purpose, and the margin is the whole # point: correctness comes from the renewal check, which republishes any # base with less than a review's lifetime left, while this surplus # decides how often that fires. Equal lifetimes renew on every run and # publish a base per run rather than per merge base — more storage, not - # less. 60 leaves a 30-day reuse window, so a pull request open a month - # never triggers one. - retention-days: 60 + # less. 30 leaves a 16-day reuse window. + retention-days: 30 if-no-files-found: ignore - name: Publish baseline analysis for the analyzed commit @@ -300,14 +301,13 @@ runs: with: name: codeboarding-base-${{ steps.state.outputs.cfg_hash }}-${{ steps.sync_commit.outputs.analyzed_sha }} path: ${{ runner.temp }}/cb-state/${{ github.action }}/out/base - # Longer than a review's 30 days on purpose, and the margin is the whole + # Longer than a review's 14 days on purpose, and the margin is the whole # point: correctness comes from the renewal check, which republishes any # base with less than a review's lifetime left, while this surplus # decides how often that fires. Equal lifetimes renew on every run and # publish a base per run rather than per merge base — more storage, not - # less. 60 leaves a 30-day reuse window, so a pull request open a month - # never triggers one. - retention-days: 60 + # less. 30 leaves a 16-day reuse window. + retention-days: 30 if-no-files-found: ignore - name: Write sync summary @@ -371,14 +371,13 @@ runs: with: name: ${{ steps.state.outputs.base_name }} path: ${{ runner.temp }}/cb-state/${{ github.action }}/out/base - # Longer than a review's 30 days on purpose, and the margin is the whole + # Longer than a review's 14 days on purpose, and the margin is the whole # point: correctness comes from the renewal check, which republishes any # base with less than a review's lifetime left, while this surplus # decides how often that fires. Equal lifetimes renew on every run and # publish a base per run rather than per merge base — more storage, not - # less. 60 leaves a 30-day reuse window, so a pull request open a month - # never triggers one. - retention-days: 60 + # less. 30 leaves a 16-day reuse window. + retention-days: 30 if-no-files-found: ignore - name: Render review diagram @@ -419,9 +418,11 @@ runs: name: codeboarding-review-${{ github.run_id }}-${{ github.run_attempt }} path: ${{ steps.review_artifact.outputs.artifact_dir }} if-no-files-found: error - # Each run carries its own copy of the base graph, and the webview only - # ever reads a pull request's latest artifact, so value drops off fast. - retention-days: 30 + # The dominant storage cost, since it is the one kept for weeks. A pull + # request open longer than this loses its rendered analysis until someone + # asks for it again, which costs one incremental over the pull request: + # the base graph is still published and needs no re-analysis. + retention-days: 14 - name: Build review comment id: review_body diff --git a/docs/COMMIT_STRATEGY.md b/docs/COMMIT_STRATEGY.md index f8fe853..a7913f0 100644 --- a/docs/COMMIT_STRATEGY.md +++ b/docs/COMMIT_STRATEGY.md @@ -27,7 +27,7 @@ cache was free. Retention is the lever — see below. | Artifact | Contents | Retention | Read by | |---|---|---|---| -| `codeboarding-review--` | `analysis.json`, `health_report.json`, `metadata.json` | 30 days | the webview, humans | +| `codeboarding-review--` | `analysis.json`, `health_report.json`, `metadata.json` | 14 days | the webview, humans | | `codeboarding-base--` | the merge base's own analysis | 30 days, renewed while still referenced | any later review forking from that commit | | `codeboarding-warmstart--pr` | the working directory: graph, pickle, fingerprint, gate | **1 day**, configurable | only the next run of that pull request | @@ -71,9 +71,13 @@ that commit has a different merge base. Artifacts are charged by size × time, so the three windows are set by what reads them: -- **30 days** for the review artifact — a reader may come back to a pull request. -- **Repository default** for a base graph — it must outlive every review artifact - that names it. +- **14 days** for the review artifact — the dominant cost, since it is the one + kept for weeks. A pull request open longer loses its rendered analysis until + someone asks for it again, which costs one incremental over the pull request: + the base graph is still published, so nothing re-analyzes the base. +- **30 days** for a base graph, which must outlive every review that names it. + The renewal threshold is the review's own retention, so the surplus — 16 days + here — is how long a base is reused before being republished. - **1 day** for the warm-start bundle, since only the next run reads it. This is `warmstart_retention_days` if a repository wants longer. It behaves like the old cache eviction: a pull request left alone longer than the window diff --git a/tests/test_action_state.py b/tests/test_action_state.py index 95e2ed4..622a445 100644 --- a/tests/test_action_state.py +++ b/tests/test_action_state.py @@ -497,6 +497,15 @@ def test_a_base_outlives_the_reviews_that_reference_it(self) -> None: f"{step['name']} does not outlive the reviews that reference it", ) + def test_the_renewal_threshold_matches_the_review_retention(self) -> None: + # A review references a base by id for its whole life, so the threshold + # that triggers renewal has to be that same life. If the two drift apart, + # a review can outlive the base it names and nothing catches it. + text = (ROOT / "action.yml").read_text(encoding="utf-8") + renew = next(l for l in text.splitlines() if "RENEW_WITHIN_DAYS:" in l) + review = next(s for s in self._uploads() if "review_artifact.outputs.artifact_dir" in s.get("path", "")) + self.assertIn(f"'{review['retention-days']}'", renew) + def test_the_reusable_analysis_honours_the_configured_retention(self) -> None: warmstart = [s for s in self._uploads() if "warmstart" in s.get("path", "")] self.assertTrue(warmstart, "no warm-start publication step found")