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")