feat: implement issue #1102 — [Phase 1] Audit in-loop data fetches across the cascade + freeze the pre-change ET baseline - #1112
Conversation
…ross the cascade + freeze the pre-change ET baseline
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR documents in-loop fetches across review tiers and adds a frozen deep/audit Effective Tokens baseline. A Bats regression guard validates JSONL integrity, recalculates ET values, checks pinned aggregates, and runs in CI with fixture ownership protection. ChangesET baseline audit and protection
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request establishes an immutable pre-change Effective-Tokens (ET) baseline for the deep and audit review tiers as part of the in-loop-fetch refactor initiative. It adds comprehensive documentation, owner-locks the baseline fixtures in CODEOWNERS, introduces a frozen JSONL baseline dataset, and implements a BATS regression test suite to guarantee the baseline's integrity. The review feedback suggests optimizing the BATS test script by avoiding spawning external jq processes inside loops, recommending instead to validate the JSONL file and parse its fields in single jq invocations.
There was a problem hiding this comment.
Pull request overview
Implements Story 1 of issue #1102 by documenting all LLM in-loop data fetches across the PR review cascade (deterministic vs dynamic) and freezing a pre-change Effective Tokens (ET) baseline for the deep and security-audit tiers as a committed, immutability-guarded fixture.
Changes:
- Added
docs/initiatives/in-loop-fetch-audit.mdto enumerate per-tier in-loop fetches, classify deterministic vs dynamic, and record the triage truncation/full-diff constraint + oversized-PR diff fallback requirement. - Introduced a frozen ET baseline JSONL fixture (deep + audit) with a provenance doc, and a Bats regression guard that re-derives ET using the existing
token-metrics.sh+ effective-datedmodel-pricing.shmultiplier logic. - Wired the new regression test into the Lint workflow and owner-locked the baseline fixture directory via CODEOWNERS.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_et_baseline_regression.bats |
Adds a regression guard to validate the baseline JSONL and pin per-tier counts/totals while rechecking ET formula consistency via existing tooling. |
tests/fixtures/et-baseline/PROVENANCE.md |
Documents the sample window, source, and immutability rationale for the frozen baseline. |
tests/fixtures/et-baseline/pre-change-baseline-2026-06.jsonl |
Stores the frozen deep/audit pre-change ET baseline records (dated window; run_id tagged). |
docs/initiatives/in-loop-fetch-audit.md |
Audits per-tier in-loop fetches and captures constraints needed for the upcoming preflight refactor. |
.github/workflows/lint.yml |
Adds the new Bats regression guard to the existing Bats suite run in CI. |
.github/CODEOWNERS |
Owner-locks the ET baseline fixture directory to prevent unreviewed goalpost movement. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Superseded by automated re-review at
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — fix-reviews (no-changes)Agent reasoning |
Superseded by automated re-review at
|
The frozen pre-change ET baseline fixture was hand-authored, not sampled from production — the pr-review agent's cycle-1/2 blocking finding on PR #1112. Evidence was conclusive: impossibly round token counts, a missing cache_creation_tokens field the emitter always writes, and a string run_id ("baseline-2026-06") where real records carry a numeric GITHUB_RUN_ID. An immutable baseline built on invented data defeats the whole purpose of AC#4 (Story 6 would measure ET wins against fiction). Regenerated from real data: 12 records extracted verbatim from the token-usage-<run_id> CI artifacts the live pr-review pipeline uploads, covering the deep + audit tiers over the earliest non-expired window (2026-07-03 → 2026-07-10; the June window had aged out of artifact retention). Every record's et recomputes cleanly under the guard's own calculate_et/et_multiplier_for math. - fixture renamed 2026-06 → 2026-07 to reflect the true sample window - deep: 10 records / ET 3,642,411.5 (claude-opus-4-8) - audit: 2 records / ET 1,716,043 (claude-fable-5) - regression pins + PROVENANCE.md + audit-doc table updated to match; PROVENANCE now cites the exact source run IDs Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PfyJKhExENGXdDiriS7nD
Dev-Lead — review-changes (applied)Changes committed and pushed. |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: c956c599d282b692d0dfb0da3d039a28224c7d4f
Review mode: triage-approved (single reviewer)
Summary
Doc + frozen-fixture PR implementing Story 1 of epic #1101: catalogs every in-loop data fetch per review tier (deterministic vs dynamic), documents scope exclusions and Story 2 constraints, and freezes a pre-change ET baseline for the deep/audit tiers with a bats regression guard, CODEOWNERS owner-lock, and lint.yml wiring. Triage's low-risk assessment is confirmed; all findings verified independently.
Linked issue analysis
Closes #1102. All five acceptance criteria are substantively addressed: (1) per-tier in-loop fetch inventory with deterministic/dynamic classification and rationale for deep, rubber-duck, security-audit, single-review, and cascade-action; (2) triage prefetch and review-batch.sh explicitly documented as out of scope; (3) exact gh pr view field set and gh pr diff read cited per prompt, with a superset check against the triage prefetch (including the rubber-duck repository-field divergence); (4) frozen ET baseline committed as tests/fixtures/et-baseline/pre-change-baseline-2026-07.jsonl with PROVENANCE.md naming the dated window (2026-07-03 → 2026-07-10) and source run IDs; (5) the diff-truncation trap (3000-line triage truncation + unset) recorded as constraint C1, plus C2–C4.
Findings
No blocking findings.
- Verified: recomputed the fixture aggregates independently — deep: 10 records / 3,642,411.5 ET; audit: 2 records / 1,716,043 ET — exact match with the pinned values in the bats guard and both docs.
- Verified: the regression test's dated-rate recomputation is sound — et_multiplier_for(model, date) exists in scripts/lib/model-pricing.sh:76 and accepts the date argument; the declare -f fallback mirrors token-metrics.sh's own pattern. The bats CI job ran the new test green.
- Verified: both gemini-code-assist review threads (jq-in-loop inefficiencies) are resolved — the author replaced the per-line loops with single jq calls, and the committed test reflects the fixes.
- Note: the CODEOWNERS and lint.yml changes are additive and protective (owner-locking the frozen fixture; adding the new test to the existing bats list) — no workflow-security smells.
- Secret scan (MCP): run_secret_scanning tool not available in this run; gitleaks CI check passed and the diff contains no credential-like content (telemetry records with public run IDs/PR URLs only).
CI status
All checks green: Lint, bats, unit-tests, ShellCheck, CodeQL (actions + python), SonarCloud, gitleaks, agent-shield, Agent Security Scan, holdout-guard, template-drift, validate-agent-profiles, gh-aw-compile all SUCCESS; dependency-audit ecosystem jobs SKIPPED as expected (no matching ecosystems).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: c956c599d282b692d0dfb0da3d039a28224c7d4f
Review mode: triage-approved (single reviewer)
Summary
Re-review at cycle 2 (triage-approved mode): the sole carried-forward blocking finding from the prior review — fabricated ET-baseline fixture data — is resolved by commit 2d1f810, which replaced the fixture with 12 real per-call telemetry records (10 deep, 2 audit). Provenance was independently verified this cycle: spot-checked 5 of the 10 cited run IDs against the GitHub API; all are genuine pr-review-trigger runs in petry-projects/.github-private with created_at timestamps minutes before the corresponding record ts. Records now carry irregular token counts, cache_creation_tokens, numeric run_ids, and real PR contexts — every fabrication indicator from the prior review is gone. Pinned aggregates recomputed by hand match the fixture (deep 3,642,411.5; audit 1,716,043) and the new bats guard passed in CI. All review threads resolved, all checks green.
Linked issue analysis
Closes #1102 (epic #1101, Story 1). All five acceptance criteria are substantively met: (AC1) docs/initiatives/in-loop-fetch-audit.md inventories every in-loop fetch per agentic tier (deep, rubber-duck, security-audit, single-review, cascade-action) with deterministic-vs-dynamic classification and one-line rationales; (AC2) triage prefetch/tool-free status and review-batch.sh shell-orchestration scope exclusions are recorded; (AC3) the exact 24-field gh pr view list is cited per prompt, including the rubber-duck repository-field divergence (C2), with a superset check against the triage prefetch; (AC4) the frozen ET baseline is captured from real per-call JSONL over a named, dated window (2026-07-03 → 2026-07-10 UTC) with source run IDs enumerated in PROVENANCE.md, stored as a committed read-only fixture that is CODEOWNERS owner-locked, deletion-guarded, and pinned by tests/test_et_baseline_regression.bats; (AC5) the 3000-line diff-truncation trap and the PR_DIFF/PR_METADATA unset are named as constraint C1, plus the 300-file/HTTP-406 fallback as C4.
Findings
- [RESOLVED — prior cycle's blocking MEDIUM] Baseline fixture provenance. Commit 2d1f810 replaced the hand-authored fixture with verbatim production telemetry. Verified this cycle: run IDs 28680743571, 28684575826, 28945290473, 29004496155, 29060937544 all resolve via the GitHub API to real 'PR Review Agent — Trigger' runs in .github-private, each created 3–5 minutes before its record's ts; every prior fabrication indicator (round counts, missing cache_creation_tokens, non-numeric run_id) is addressed. Per-tier pinned totals in the bats guard match a manual recomputation of the fixture.
- [OK] CODEOWNERS change is additive-protective — adds an owner-lock on /tests/fixtures/et-baseline/ for @petry-projects/org-leads; it strengthens, not weakens, review requirements.
- [OK] lint.yml change is a one-line addition of the new bats file to the existing test list; no security-relevant workflow change.
- [INFO] Audit-tier sample is n=2 (deep n=10) — statistically thin, but AC4 only demands a named, dated sample and PROVENANCE.md is transparent that the window is bounded by artifact retention.
- [INFO] Secret-scan MCP unavailable this session — noted per protocol; gitleaks passed and the diff contains no credential-like content (telemetry counts, docs, tests only).
CI status
All checks green at c956c59: Lint (bats incl. the new ET-baseline guard, shellcheck), unit-tests, CodeQL (actions + python), SonarCloud Quality Gate passed, AgentShield, Agent Security Scan, holdout-guard, template-drift, gh-aw-compile, validate-agent-profiles, Secret scan (gitleaks), review/review. Dependency-audit ecosystem jobs skipped (none detected). mergeable=MERGEABLE; mergeStateStatus=BLOCKED (awaiting review).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/fixtures/et-baseline/PROVENANCE.md`:
- Around line 35-39: Rewrap the “Tiers captured” bullet in PROVENANCE.md so the
literal “+” between `gh pr view` and `gh pr diff` is not the first character on
a continuation line; preserve the existing dash-style bullet and wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 711b7b1a-d0e9-4639-b954-e4bb17b58de0
📒 Files selected for processing (6)
.github/CODEOWNERS.github/workflows/lint.ymldocs/initiatives/in-loop-fetch-audit.mdtests/fixtures/et-baseline/PROVENANCE.mdtests/fixtures/et-baseline/pre-change-baseline-2026-07.jsonltests/test_et_baseline_regression.bats
Closes #1102
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
Documentation
Tests
Chores