From 37caf810ac9adec9e5bd7173fe9fc58715d1d427 Mon Sep 17 00:00:00 2001 From: Svilen Stefanov Date: Wed, 19 Aug 2026 23:09:17 +0200 Subject: [PATCH 1/2] fix(review): let a base outlive the reviews that reference it The renewal rule added for dangling references republished a base whenever the one it found expires within a review's lifetime. Base retention was also 30 days, the same as a review's, so a fetched base always had less than that left and the rule fired on every run: every review republished the base graph, which is the duplication that publishing it separately removed. The optimisation was inert. Base graphs are now kept 90 days. Correctness still comes from the renewal check, so a review never names a base that expires under it; the margin only decides how often renewal fires, which is now at most once per sixty days per merge base instead of once per run. A test asserts base retention exceeds review retention, since equal lifetimes are what made the rule degenerate. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit 576695ac9e8370f1dfedd3d17be447a323ed7856) --- action.yml | 18 +++++++++++++++--- tests/test_action_state.py | 21 ++++++++++++++------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/action.yml b/action.yml index f14d13a..e06ba64 100644 --- a/action.yml +++ b/action.yml @@ -283,7 +283,11 @@ 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 - retention-days: 30 + # Longer than a review's 30 days on purpose. Correctness comes from the + # renewal check, which republishes any base with less than a review's + # lifetime left; this margin is what decides how often that fires. Equal + # lifetimes would renew on every run and undo the deduplication. + retention-days: 90 if-no-files-found: ignore - name: Publish baseline analysis for the analyzed commit @@ -293,7 +297,11 @@ 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 - retention-days: 30 + # Longer than a review's 30 days on purpose. Correctness comes from the + # renewal check, which republishes any base with less than a review's + # lifetime left; this margin is what decides how often that fires. Equal + # lifetimes would renew on every run and undo the deduplication. + retention-days: 90 if-no-files-found: ignore - name: Write sync summary @@ -357,7 +365,11 @@ runs: with: name: ${{ steps.state.outputs.base_name }} path: ${{ runner.temp }}/cb-state/${{ github.action }}/out/base - retention-days: 30 + # Longer than a review's 30 days on purpose. Correctness comes from the + # renewal check, which republishes any base with less than a review's + # lifetime left; this margin is what decides how often that fires. Equal + # lifetimes would renew on every run and undo the deduplication. + retention-days: 90 if-no-files-found: ignore - name: Render review diagram diff --git a/tests/test_action_state.py b/tests/test_action_state.py index 27aca15..95e2ed4 100644 --- a/tests/test_action_state.py +++ b/tests/test_action_state.py @@ -482,13 +482,20 @@ def test_the_base_is_inlined_whenever_no_artifact_holds_it(self) -> None: self.assertIn("fetch_base.outputs.artifact_id == ''", inline[0]) def test_a_base_outlives_the_reviews_that_reference_it(self) -> None: - # A review artifact points at a base by id for 30 days, so a base kept - # for less leaves that review unusable for the rest of its life. - for step in self._uploads(): - if "out/base" in step.get("path", ""): - self.assertEqual( - step.get("retention-days"), "30", f"{step['name']} may expire before the reviews naming it" - ) + # A review points at a base by id for its whole life, so a base kept for + # the same period is only ever good at the instant it is written: the + # renewal check would then fire on every run and republish it every time, + # which is exactly the duplication that splitting it out removed. + review = next(s for s in self._uploads() if "review_artifact.outputs.artifact_dir" in s.get("path", "")) + review_days = int(review["retention-days"]) + bases = [s for s in self._uploads() if "out/base" in s.get("path", "")] + self.assertTrue(bases, "no base publication step found") + for step in bases: + self.assertGreater( + int(step.get("retention-days", 0)), + review_days, + f"{step['name']} does not outlive the reviews that reference it", + ) def test_the_reusable_analysis_honours_the_configured_retention(self) -> None: warmstart = [s for s in self._uploads() if "warmstart" in s.get("path", "")] From 86dd638e941d26423fdd2e76998a473ba8994812 Mon Sep 17 00:00:00 2001 From: Svilen Stefanov Date: Wed, 19 Aug 2026 23:14:07 +0200 Subject: [PATCH 2/2] fix(review): keep base graphs 60 days, not 90 90 was more headroom than the behaviour needs. The reuse window only has to outlast a pull request, so 60 days leaves 30 after the renewal threshold: a pull request open a month never triggers a republish, and it costs a third less than 90 to store. Modelled at the largest repository we have, 50 pull requests a month with five runs each: 30-day bases cost 173 MB of monthly average because they republish per run, 60-day bases cost 69 MB because they republish per merge base, and 90 cost 104 MB for no additional reuse. Co-Authored-By: Claude Opus 5 (1M context) --- action.yml | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/action.yml b/action.yml index e06ba64..8c245e2 100644 --- a/action.yml +++ b/action.yml @@ -283,11 +283,14 @@ 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. Correctness comes from the - # renewal check, which republishes any base with less than a review's - # lifetime left; this margin is what decides how often that fires. Equal - # lifetimes would renew on every run and undo the deduplication. - retention-days: 90 + # Longer than a review's 30 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 if-no-files-found: ignore - name: Publish baseline analysis for the analyzed commit @@ -297,11 +300,14 @@ 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. Correctness comes from the - # renewal check, which republishes any base with less than a review's - # lifetime left; this margin is what decides how often that fires. Equal - # lifetimes would renew on every run and undo the deduplication. - retention-days: 90 + # Longer than a review's 30 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 if-no-files-found: ignore - name: Write sync summary @@ -365,11 +371,14 @@ 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. Correctness comes from the - # renewal check, which republishes any base with less than a review's - # lifetime left; this margin is what decides how often that fires. Equal - # lifetimes would renew on every run and undo the deduplication. - retention-days: 90 + # Longer than a review's 30 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 if-no-files-found: ignore - name: Render review diagram