From de0ae6d0a27301f34784c69cf970ed131a42a04a Mon Sep 17 00:00:00 2001 From: Kris Wong Date: Thu, 10 Sep 2026 10:40:38 -0500 Subject: [PATCH] fix(code-review): make override-cache tests independent of wall-clock date Three tests pinned an override's asserted_at to a fixed 2026-05-29 timestamp and asserted the override was honored. Once that timestamp aged past the 90-day overrides TTL they began failing with no code change. Derive the timestamp relative to now instead. Renames the existing _stale_cached_at() helper to _iso_days_ago() to match its already-generic docstring, and routes the two override TTL boundary tests through it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011QjUMmrqfGU4QNXDDRLPrN --- CHANGELOG.md | 8 +++++++ .../code-review/.claude-plugin/plugin.json | 2 +- plugins/code-review/README.md | 6 ++++- .../tools/python/test_code_review_helpers.py | 24 +++++++++---------- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17325ab..ec24011 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to the claude-plugins project will be documented in this fil The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Entries are listed newest-first; each plugin section is treated as released when merged to `main`. +### code-review v3.7.1 + +#### Fixed +- Override-cache tests no longer depend on the wall-clock date. Three tests pinned an override's `asserted_at` to a fixed `2026-05-29` timestamp and asserted the override was honored — by `verify-prepare` short-circuiting into `override_hits`, and by the prepare-then-consolidate paths that route an override to `RE_ASSERTED`. Once that fixed timestamp aged past the 90-day `overrides` cache TTL, the overrides were correctly treated as expired and the three tests began failing with no accompanying code change. They now derive `asserted_at` relative to the current time, so they exercise the honored-override path regardless of when the suite runs. + +#### Changed +- Test helper `_stale_cached_at()` renamed to `_iso_days_ago()` so the name matches its documented behavior (an ISO-8601 timestamp N days in the past, used for both within-TTL and past-TTL fixtures); all call sites updated, and the two override TTL boundary tests now delegate to it instead of inlining the same `datetime.now(timezone.utc) - timedelta(...)` expression. + ### code v1.14.10 #### Added diff --git a/plugins/code-review/.claude-plugin/plugin.json b/plugins/code-review/.claude-plugin/plugin.json index 684332f..de55d5a 100644 --- a/plugins/code-review/.claude-plugin/plugin.json +++ b/plugins/code-review/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "code-review", "description": "Code review plugin", - "version": "3.7.0", + "version": "3.7.1", "author": { "name": "ClosedLoop", "email": "support@closedloop.ai" diff --git a/plugins/code-review/README.md b/plugins/code-review/README.md index 7d41425..3c12e78 100644 --- a/plugins/code-review/README.md +++ b/plugins/code-review/README.md @@ -48,10 +48,14 @@ plugins/code-review/ python/code_review_schema.py Canonical Finding + ResultEnvelope schema + validators (PLN-719) python/test_code_review_schema.py Schema tests + round-trips python/code_review_helpers.py Deterministic helper CLI (parse-diff, hygiene, partition, route, validate, cache, finalize-result, arbitrate-budget, prepare-run, etc.) + python/config/cli.json Declarative argparse spec the helper CLI builds its subparsers from + python/config/stages.json Declarative stage table backing `prepare-run`'s `run_plan.json` and `run-prefix` + python/signal_taxonomy.json Signal taxonomy loaded by signal extraction; its bytes are hashed into the extraction cache key + python/conftest.py Shared pytest fixtures and finding factories for the co-located tests python/test_code_review_helpers.py Unit tests for the helper CLI python/golden_fixture_harness.py Golden fixture harness: replays canonical inputs through helper subcommands and diffs against expected envelopes (PLN-719 Phase 8) python/test_golden_fixtures.py Pytest driver that runs every fixture under tools/python/fixtures/ - python/fixtures// Per-fixture directory (config.yaml + inputs/ + expected/); 3 full scenarios + 6 README-stubs for future coverage + python/fixtures// Per-fixture directory (config.yaml + inputs/ + expected/); 4 full scenarios + 3 README-stubs for future coverage python/prefix_golden_harness.py Prefix golden harness + subprocess A/B parity oracle: walks the deterministic prefix against real git fixtures — in-process for golden snapshots, and per-stage-subprocess vs `run-prefix` for byte-equal parity (PLN-1229 Phase 0/1) python/test_prefix_golden.py Pytest driver for the prefix harness: determinism oracle + golden diff across the prefix_fixtures/ matrix python/prefix_fixtures// Per-fixture directory (expected/ golden snapshots); 7 branch scenarios (standard, fast-path, hygiene-only, empty-diff, cache-hit, since-last-review, coverage-critic) diff --git a/plugins/code-review/tools/python/test_code_review_helpers.py b/plugins/code-review/tools/python/test_code_review_helpers.py index df6bbc8..23fb276 100644 --- a/plugins/code-review/tools/python/test_code_review_helpers.py +++ b/plugins/code-review/tools/python/test_code_review_helpers.py @@ -108,11 +108,11 @@ # A `cached_at` timestamp always within the BHA cache TTL (30 days). # PLN-719 Phase 7 added sweep-on-read TTL eviction; fixtures that want a hit # must use a fresh timestamp. Tests that want eviction behavior should use -# an explicitly-stale timestamp via ``_stale_cached_at()``. +# an explicitly-stale timestamp via ``_iso_days_ago()``. _FRESH_CACHED_AT = datetime.now(timezone.utc).isoformat() -def _stale_cached_at(days_ago: int = 365) -> str: +def _iso_days_ago(days_ago: int = 365) -> str: """Return an ISO timestamp ``days_ago`` days in the past (default: 1 year).""" return (datetime.now(timezone.utc) - timedelta(days=days_ago)).isoformat() @@ -3385,14 +3385,14 @@ def test_stale_entry_within_ttl_hits(self, tmp_path: Path) -> None: from code_review_helpers import _is_entry_fresh, CACHE_NAMESPACE_BHA # 29 days old: under the 30-day BHA TTL. - entry = {"cached_at": _stale_cached_at(days_ago=29)} + entry = {"cached_at": _iso_days_ago(days_ago=29)} assert _is_entry_fresh(entry, CACHE_NAMESPACE_BHA) is True def test_stale_entry_past_ttl_misses(self, tmp_path: Path) -> None: from code_review_helpers import _is_entry_fresh, CACHE_NAMESPACE_BHA # 31 days old: past the 30-day BHA TTL. - entry = {"cached_at": _stale_cached_at(days_ago=31)} + entry = {"cached_at": _iso_days_ago(days_ago=31)} assert _is_entry_fresh(entry, CACHE_NAMESPACE_BHA) is False def test_missing_cached_at_treated_as_fresh(self, tmp_path: Path) -> None: @@ -3406,7 +3406,7 @@ def test_missing_cached_at_treated_as_fresh(self, tmp_path: Path) -> None: def test_unknown_namespace_skips_ttl_check(self, tmp_path: Path) -> None: from code_review_helpers import _is_entry_fresh - entry = {"cached_at": _stale_cached_at(days_ago=365 * 10)} + entry = {"cached_at": _iso_days_ago(days_ago=365 * 10)} assert _is_entry_fresh(entry, "future-namespace") is True def test_v1_cache_check_evicts_stale_entry(self, tmp_path: Path) -> None: @@ -3425,7 +3425,7 @@ def test_v1_cache_check_evicts_stale_entry(self, tmp_path: Path) -> None: "prompt_hash": "abc123", "patch_hash": patch_hash, "findings": [{"file": "a.ts", "line": 1, "issue": "stale"}], - "cached_at": _stale_cached_at(days_ago=45), + "cached_at": _iso_days_ago(days_ago=45), } } _write_manifest(cache_dir, manifest) @@ -3443,7 +3443,7 @@ def test_v2_cache_check_evicts_stale_entry(self, tmp_path: Path) -> None: patch_hash = _compute_patch_hash("a.ts", diff_data["patch_lines"]["a.ts"]) composite = _compute_composite_key("opus", "abc123", patch_hash, "ctx") - stale = _stale_cached_at(days_ago=45) + stale = _iso_days_ago(days_ago=45) v2_manifest = { "a.ts": { composite: { @@ -10133,7 +10133,7 @@ def test_verify_prepare_short_circuits_on_valid_override( "finding_id": "bha_p0_f0", "file_content_hash": _file_content_hash(cr, "src/x.py", 3), "override": "RE_ASSERT", - "asserted_at": "2026-05-29T22:00:00+00:00", + "asserted_at": _iso_days_ago(days_ago=1), }) # PR #114 review fix — delegate to the shared helper with an # explicit cr_dir override so the per-test stdout/Namespace dance @@ -11555,7 +11555,7 @@ def test_prepare_then_consolidate_routes_override_to_verified( "finding_id": "bha_p0_f0", "file_content_hash": _file_content_hash(cr, "src/x.py", 3), "override": "RE_ASSERT", - "asserted_at": "2026-05-29T22:00:00+00:00", + "asserted_at": _iso_days_ago(days_ago=1), }) # Phase 1 — prepare. Should record the fid in override_hits and @@ -11604,7 +11604,7 @@ def test_prepare_then_consolidate_writes_re_asserted_to_stats( "finding_id": "bha_p0_f0", "file_content_hash": _file_content_hash(cr, "src/x.py", 3), "override": "RE_ASSERT", - "asserted_at": "2026-05-29T22:00:00+00:00", + "asserted_at": _iso_days_ago(days_ago=1), }) _, manifest = _run_verify_prepare( @@ -11736,7 +11736,7 @@ def test_override_invalidated_when_ttl_expired( self._write_target_file(tmp_path, "src/x.py", "a\nb\nc\nd\ne\n") cache = tmp_path / "cache" cache.mkdir() - old_ts = (datetime.now(timezone.utc) - timedelta(days=120)).isoformat() + old_ts = _iso_days_ago(days_ago=120) _write_override(cache, { "finding_id": "bha_p0_f0", "file_content_hash": _file_content_hash(cr, "src/x.py", 3), @@ -11762,7 +11762,7 @@ def test_override_honored_when_ttl_within_bounds( self._write_target_file(tmp_path, "src/x.py", "a\nb\nc\nd\ne\n") cache = tmp_path / "cache" cache.mkdir() - recent_ts = (datetime.now(timezone.utc) - timedelta(days=30)).isoformat() + recent_ts = _iso_days_ago(days_ago=30) _write_override(cache, { "finding_id": "bha_p0_f0", "file_content_hash": _file_content_hash(cr, "src/x.py", 3),