chore(review): manual-only Greptile as the CodeRabbit fallback, capped at 16 reviews/month - #98
Merged
Merged
Conversation
…d at 16 reviews/month
Adds Greptile as the step-3 review-gate fallback for when CodeRabbit is rate-
or quota-limited (CLAUDE.md §5.1), under two constraints: it never fires on its
own, and it is capped at 16 reviews per month from 2026-08-03.
Inputs: .coderabbit.yaml (review scope to mirror); Greptile config schema.
Outputs: .greptile/config.json, .greptile/README.md,
scripts/greptile_budget.py, tests/unit/test_greptile_budget.py.
Never automatic
.greptile/config.json pins skipReview:"AUTOMATIC" (the exact literal; any other
value is invalid and silently restores auto-review), plus triggerOnUpdates,
statusCheck, statusCommentsEnabled and autoApprove all false. ignorePatterns
mirrors .coderabbit.yaml's path_filters so the fallback reviews the same scope
as the primary, and self-excludes .greptile/. Repo config outranks dashboard
settings; only org-enforced rules could override it, and this repo is
user-owned, so none exist.
The 16/month cap
Greptile exposes no native per-repo review quota — its only usage control is an
org-wide dollar cap on flex spend — so the cap is enforced repo-side by
scripts/greptile_budget.py. The count is RE-DERIVED from the GitHub API on every
call, never from a ledger this repo writes about itself (§10.3), and the script
fails closed: exit 0 available / 2 exhausted / 3 could-not-measure, with no
budget number printed on the unmeasured path.
Counting unit — the trigger, not Greptile's output
Sampling real public PRs shows Greptile's output surface is not a stable unit:
it edits one sticky <!-- greptile-status --> comment in place across re-reviews
(observed: 2 reviews, 1 comment), often posts a summary with zero
PullRequestReview objects, and emits inline comments that scale with findings.
With auto-review disabled every review is preceded by exactly one human
@greptileai comment, so one trigger == one review. Greptile activity on a thread
with no in-period trigger is charged as budget AND raised as auto_fire_suspected,
so the cap doubles as the detector for the config not being in force.
Validation
32 unit tests pass; ruff 0.15.15 + black 25.11.0 (CI pins) clean; live run
reports 0/16 used. Six sabotages each bite their named test and no other:
boundary inversion (used>=limit -> used>limit), counting bot output instead of
triggers, dropping the auto-fire charge, pinning one bot login, HTTP errors no
longer failing closed, and swapping the repo-wide comment feed for a PR
enumeration. Bot matching is by `greptile` login prefix + type=="Bot", not one
hardcoded login, because Greptile owns two bot identities (greptile-apps[bot],
the active reviewer, and greptile[bot]) while `greptile-apps`/`greptile`/
`greptileai` also exist as non-Bot decoy accounts.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 23 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Comment |
…he fallback usable Addresses the CodeRabbit review on PR #98, plus two facts the config-landing auto-review exposed. CodeRabbit findings (both verified by execution before fixing) * major - parse_anchor/_parse_ts called datetime.strptime directly, so a malformed --anchor, a malformed --now, or an unexpected created_at escaped as a bare ValueError and exited 1 - outside the documented 0/2/3 contract, and indistinguishable from a crash to a caller keying on those codes. Confirmed: `--anchor not-a-date` exited 1; now exits 3. * minor - test_non_array_payload_refuses asserted only _next_link and never reached the isinstance(batch, list) guard it was named for, so that guard was untested. Split into test_next_link_follows_only_rel_next plus a real non-array test, and added unparseable-JSON coverage. fileChangeLimit (load-bearing for the fallback to work at all) The auto-review of this PR refused with "Too many files changed for review. (`4 files found`, `1 file limit`)" - the dashboard has fileChangeLimit=1. 11 of the last 12 PRs here touch >1 file (median ~9, max 32), so at the dashboard value Greptile could not review essentially anything and would be useless as a CodeRabbit replacement. Repo config outranks the dashboard; pinned to 50, which covers the observed distribution with headroom. Second trigger handle That same refusal advertises "Bypass the limit by tagging `@greptile-apps` to review." - a second, undocumented invocation handle. The counter matched only @greptileai, so a review triggered that way would have gone unbilled, permissive in exactly the direction that overruns the cap. _TRIGGER_RE now matches both. Validation 53 unit tests pass (was 32); ruff 0.15.15 + black 25.11.0 clean; malformed --anchor and --now now both exit 3; live run reports 1/16 used, correctly charging and naming this PR's unrequested auto-review as auto_fire_suspected. Dropping the @greptile-apps alternative from _TRIGGER_RE fails exactly the two @greptile-apps cases and nothing else.
…budget contract Second CodeRabbit round on PR #98. Both findings verified by execution first. * major - a read timeout mid-`resp.read()` surfaces as a bare TimeoutError, which urllib does NOT wrap in URLError, so it escaped _get entirely and exited 1 instead of the documented 3. Confirmed by injecting TimeoutError into urlopen: "RESULT: ESCAPED as TimeoutError". Added a trailing `except OSError` arm - trailing because HTTPError < URLError < OSError, so placing it first would flatten every HTTP status to the status-0 transport case. A test asserts a 404 still reports 404, and putting the OSError arm first fails exactly that test. * minor - argparse exits 2 on a usage error, and 2 is this script's "budget exhausted". Confirmed: `--bogus-flag` and `--limit notanint` both exited 2, so a mistyped flag read as "the month's budget is spent". parse_args is now wrapped and usage errors return USAGE_ERROR_EXIT = 4, outside the 0/2/3 contract; --help still exits 0. Validation 58 unit tests pass (was 53); ruff + black clean; measured exit codes are now --bogus-flag 4, --limit notanint 4, --help 0, --anchor not-a-date 3, live run 0. Exit-code contract updated in the module docstring, .greptile/README.md and CLAUDE.md §5.1a: only 0 means invoke.
…ent budget Third CodeRabbit round on PR #98. `--limit 0` made `used >= limit` true on an empty period, so the script reported exit 2 - "this month's budget is spent" - when nothing had been spent and the argument was simply wrong. Same conflation the argparse-exit-2 remap fixed. --limit now takes a _positive_int argparse type, so 0/-1/notanint are usage errors (exit 4) while --limit 16 still exits 0. 61 unit tests pass (was 58); ruff + black clean.
…ment still charges Fourth CodeRabbit round on PR #98. Closes the last undercount. Greptile edits ONE sticky <!-- greptile-status --> comment in place across re-reviews rather than posting a new one (the same behaviour this counter was built around: 2 reviews, 1 comment). So a review that ran in THIS period on a PR first touched in a PREVIOUS one carries a created_at outside the window, and the created_at-only filter dropped it — an auto-fired re-review on an older PR was invisible. Permissive, in the direction that overruns the cap. Bot comments are now placed in-period if EITHER created_at or updated_at falls in the window; `since=` already filters on updated_at, so such a comment was always in the fetched page. Triggers stay created_at-only: editing an old comment does not re-trigger a review and must not spend budget. Validation 64 unit tests pass (was 61); ruff + black clean; live run still 1/16. Sabotaged BOTH directions, each biting one distinct test and no other: reverting to created_at-only fails the edited-sticky test, and charging every bot comment regardless of date fails the untouched-sticky positive control.
Owner
Author
…ly config caveat Two facts observed while validating on PR #98 itself: * Auto-fire matching is per-thread, not per-event: an auto-fire followed later by a manual trigger on the SAME PR in the same period counts as one, not two. Charging both would double-count the ordinary trigger-then-response case, which is far more common; the residual undercount is bounded by the number of auto-fires, and an auto-fire on an untriggered thread is still caught and charged. Recorded in the module docstring rather than left implicit. * Greptile reads .greptile/config.json from the DEFAULT BRANCH, not the PR head: on #98 it kept applying the dashboard's fileChangeLimit=1 and refused the 4-file PR even with the config present on the branch. So this config only takes effect once merged. Recorded in .greptile/README.md.
Owner
Author
…n on this repo On PR #98 both @greptileai and @greptile-apps were posted and Greptile produced no review — the sticky comment was never updated — while the dashboard's fileChangeLimit=1 was still the governing config (this file is not read from a PR head). Two budget units were spent for zero reviews, which the counter recorded correctly (2/16). Rather than claim a working fallback, .greptile/README.md and CLAUDE.md §5.1a now say the path is configured but UNPROVEN, and that Greptile staying silent means the review gate is UNMET — fall through to /code-review, never merge on an absent review.
|
| Filename | Overview |
|---|---|
| .greptile/config.json | Adds Greptile config; all keys verified against the official schema — skipReview, triggerOnUpdates, statusCheck, statusCommentsEnabled, fileChangeLimit, autoApprove, and ignorePatterns are all valid and correctly typed. |
| .greptile/README.md | Documentation-only file explaining the config rationale; accurately describes Greptile's precedence hierarchy and both known edge cases (this PR's first-run auto-review and malformed-config regression). |
| scripts/greptile_budget.py | New budget counter script; period arithmetic, trigger/bot classification, fail-closed HTTP layer, argparse exit-code remap, and auto-fire detection all look correct with no logical issues found. |
| tests/unit/test_greptile_budget.py | 32 unit tests covering period boundaries, both trigger handles, sticky-comment undercount regression, auto-fire alarm, boundary inversion, fail-closed guards, and endpoint choice; every failing branch has a passing positive control. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User wants to invoke Greptile] --> B[Run greptile_budget.py]
B --> C{Parse args\nanchor / now / limit}
C -->|BudgetError| X3[exit 3\nCould not measure]
C --> D[compute current_period\nUTC monthly window]
D --> E[fetch_issue_comments\nGitHub API since=period_start]
E -->|HTTP error\nor pagination > 100 pages| X3
E --> F[summarise comments]
F --> G{For each comment}
G -->|Greptile Bot\ntype==Bot && login starts greptile| H{created OR updated\nin period?}
H -->|No| G
H -->|Yes| I[record in bot_threads set]
G -->|Non-Bot user\ncreated in period\nbody matches @greptileai or @greptile-apps| J[append to triggers list]
G -->|anything else| G
I --> K[compute unmatched\nbot threads with no trigger]
J --> K
K --> L[used = len triggers + len unmatched]
L --> M{unmatched > 0?}
M -->|Yes| N[emit auto_fire_suspected warning]
M --> O{used >= limit?}
N --> O
O -->|Yes| X2[exit 2\nBudget exhausted]
O -->|No| X0[exit 0\nBudget available]
Reviews (2): Last reviewed commit: "fix(review): place bot comments by updat..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+199
to
+202
| for part in (headers.get("Link") or headers.get("link") or "").split(","): | ||
| seg = part.split(";") | ||
| if len(seg) >= 2 and 'rel="next"' in seg[1] and seg[0].strip().startswith("<"): | ||
| return seg[0].strip()[1:-1] |
There was a problem hiding this comment.
,, but RFC 5988 / GitHub's own format can technically include semicolons inside the URL segment (e.g. query strings using ; as a delimiter). More importantly, the check only inspects seg[1] — if GitHub ever emits a Link header with additional attributes (; type="application/json"; rel="next"), the rel would land in seg[2] and the page-walk would silently stop at page 1, returning a truncated comment list read as "few reviews used". The in-operator over the full remaining-segments string avoids both risks.
Suggested change
| for part in (headers.get("Link") or headers.get("link") or "").split(","): | |
| seg = part.split(";") | |
| if len(seg) >= 2 and 'rel="next"' in seg[1] and seg[0].strip().startswith("<"): | |
| return seg[0].strip()[1:-1] | |
| for part in (headers.get("Link") or headers.get("link") or "").split(","): | |
| seg = part.split(";") | |
| if len(seg) >= 2 and any('rel="next"' in s for s in seg[1:]) and seg[0].strip().startswith("<"): | |
| return seg[0].strip()[1:-1] |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/greptile_budget.py
Line: 199-202
Comment:
The Link header is split on a bare `,`, but RFC 5988 / GitHub's own format can technically include semicolons inside the URL segment (e.g. query strings using `;` as a delimiter). More importantly, the check only inspects `seg[1]` — if GitHub ever emits a Link header with additional attributes (`; type="application/json"; rel="next"`), the `rel` would land in `seg[2]` and the page-walk would silently stop at page 1, returning a truncated comment list read as "few reviews used". The in-operator over the full remaining-segments string avoids both risks.
```suggestion
for part in (headers.get("Link") or headers.get("link") or "").split(","):
seg = part.split(";")
if len(seg) >= 2 and any('rel="next"' in s for s in seg[1:]) and seg[0].strip().startswith("<"):
return seg[0].strip()[1:-1]
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| # is permissive in exactly the direction that overruns the cap. | ||
| # Matched case-insensitively and required to be followed by a non-word character (or end of | ||
| # string) so `@greptileaifoo` does not count. | ||
| _TRIGGER_RE = re.compile(r"@greptile(?:ai|-apps)(?!\w)", re.IGNORECASE) |
There was a problem hiding this comment.
_TRIGGER_RE has no negative lookbehind before @, so it matches wherever @greptileai or @greptile-apps appears as a substring — including inside email addresses or quoted code blocks (e.g. "email@greptileai.example" or a markdown code span containing the handle). Adding a word-boundary-or-start assertion ((?<!\w)) prevents any such accidental trigger from consuming budget.
Suggested change
| _TRIGGER_RE = re.compile(r"@greptile(?:ai|-apps)(?!\w)", re.IGNORECASE) | |
| _TRIGGER_RE = re.compile(r"(?<!\w)@greptile(?:ai|-apps)(?!\w)", re.IGNORECASE) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/greptile_budget.py
Line: 101
Comment:
`_TRIGGER_RE` has no negative lookbehind before `@`, so it matches wherever `@greptileai` or `@greptile-apps` appears as a substring — including inside email addresses or quoted code blocks (e.g. `"email@greptileai.example"` or a markdown code span containing the handle). Adding a word-boundary-or-start assertion (`(?<!\w)`) prevents any such accidental trigger from consuming budget.
```suggestion
_TRIGGER_RE = re.compile(r"(?<!\w)@greptile(?:ai|-apps)(?!\w)", re.IGNORECASE)
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+47
to
+56
| def _comment(*, thread, login, kind, body="", at="2026-08-10T12:00:00Z", cid=1): | ||
| """One /issues/comments item. `kind` is 'User' or 'Bot' (the GitHub user.type).""" | ||
| return { | ||
| "id": cid, | ||
| "issue_url": thread, | ||
| "created_at": at, | ||
| "html_url": f"{thread}#issuecomment-{cid}", | ||
| "user": {"login": login, "type": kind}, | ||
| "body": body, | ||
| } |
There was a problem hiding this comment.
_comment fixture omits updated_at
Real GitHub issue-comment objects always include updated_at. Because the fixture omits it, every bot-comment test that does not manually add updated_at exercises the c.get("updated_at") or created_raw fallback path — not the path that fires on a real API response. The gap is harmless today (the fallback is correct), but a future refactor that changes how updated_at is read would pass the full suite while silently breaking on live data. Adding updated_at to the fixture (defaulting to the same value as at) would make the normal-case tests exercise the primary path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/test_greptile_budget.py
Line: 47-56
Comment:
**`_comment` fixture omits `updated_at`**
Real GitHub issue-comment objects always include `updated_at`. Because the fixture omits it, every bot-comment test that does not manually add `updated_at` exercises the `c.get("updated_at") or created_raw` fallback path — not the path that fires on a real API response. The gap is harmless today (the fallback is correct), but a future refactor that changes how `updated_at` is read would pass the full suite while silently breaking on live data. Adding `updated_at` to the fixture (defaulting to the same value as `at`) would make the normal-case tests exercise the primary path.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
bioedca
pushed a commit
that referenced
this pull request
Aug 3, 2026
…ptim guard, +5 Review gate met via the CodeRabbit CLI. The GitHub app returned only its fair-usage notice and Copilot returned "quota limit reached"; Greptile was invoked and stayed SILENT for a third consecutive time (PR #98 x2, PR #99 x1), spending a credit for nothing. 11 findings, each verified by execution before being accepted or refused. THE ONE THAT REACHES THE SHIPPED CHECKPOINTS: the LR scheduler's domain was micro-batches while scheduler.step() fires per OPTIMISER step. With accumulation 2, job 1064's cosine covered exactly 50% — 1,415 of 2,830 — ending at 0.5501x base instead of ~0. All six arms trained under a schedule their config does not describe. User decision: fix the code, KEEP the checkpoints, record the defect. Nothing downstream is invalidated (P3-08 compares arms that all share it), but they are not optimally-annealed models and any absolute number derived from them carries that. A CLAIM I MADE WAS WRONG: I reported growth_ratio 1.01-1.04 as a flat PER-STEP footprint. reset_peak_memory_stats() ran once before the loop, so the series was a RUNNING MAXIMUM, monotonic by construction. The no-leak conclusion survives — a leak would still raise a running max — but the instrument was mislabelled and I oversold it. Fixed; the sbatch header repeating the claim is corrected too. Also fixed, each sabotage-bitten: unknown optim.* keys raise as loss.* ones do; accumulation counts per-epoch and flushes the trailing group (283 odd micro-batches made the cumulative phase shift every epoch); the sbatch clears the checkpoint dir only AFTER the health check; WORLD_SIZE=0 raises; the backfill reads optim.lr from its real group and refuses null seed/env_lock/input; sizing COMPUTES headroom instead of asserting it. TWO OF MY OWN NEW TESTS WERE VACUOUS. The LR-domain test asserted arithmetic and never touched train_stage2's wiring — reverting the trainer left it green. Now asserts the shipped construction. (The sbatch test did bite; my sabotage for it was wrong.) Refused with reasons: device_record()'s index 0 is correct — every rank calls set_device(local_rank) first. Validation: trainer 102, sizing 18, overrides 22, backfill 3; ruff + black clean; bash -n clean; 23 sabotages across the step. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1
bioedca
pushed a commit
that referenced
this pull request
Aug 3, 2026
…aim I put in CLAUDE.md Greptile DID review PR #99 — ~18 minutes after the trigger, as greptile-apps[bot], with 3 P2 findings. A 6-minute poll had concluded "silent", reported the credit wasted, and WROTE INTO CLAUDE.md §5.1a that the trigger path was dead. That was a false conclusion from an impatient poll, promoted to documentation where a future session would have obeyed it. §5.1a corrected: it works, budget ~25 min, and look in pulls/<n>/reviews for greptile-apps[bot] — the reply is a REVIEW, not an issue comment, so a poll reading only issue comments misses it. The prior PR #98 "silences" are now suspect for the same reason. Findings, all real: (P2) losses_finite was blind to a NaN gradient on any non-primary rank. n_nonfinite counts the RANK-0 FORWARD LOSS; under DDP a NaN gradient from another rank all-reduces into every rank's gradients and poisons the weights while rank 0's loss stays finite. Fixed by reading clip_grad_norm_'s pre-clip total norm — computed after the all-reduce, non-finite iff some gradient is — as n_nonfinite_grad_steps, now a second term in the clause. This is the THIRD gate in this step that measured the wrong side of what it named, after the checkpointing flag-count and the micro-batch-only step check. (P2) _bool_or_none blamed the wrong column: written for nested_train, also called for calib and is_tbox, so a "yes" in calib would raise an error naming nested_train. Now takes the field name; every call site passes its own. (P2, advisory) Ternary precedence in three clauses — semantics confirmed correct, readability only. Left as-is and recorded rather than silently dropped. Also corrected in §5.1: CodeRabbit is INCREMENTAL and AUTOMATIC. @coderabbitai review on an already-reviewed commit is a no-op. Both rounds I attributed to manual triggers were the automatic reviews of the pushes; coverage reads from original_commit_id, not from whether a command was sent. And a green CodeRabbit CHECK does not mean a review ran — it read pass while the body was only the Fair-Usage notice. Validation: trainer 113, sizing 20, overrides 22, backfill 4; ruff + black clean; bash -n clean. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1
bioedca
added a commit
that referenced
this pull request
Aug 3, 2026
… (production + §11 sweep) (#99) * feat(stage2): SLURM LoRA fine-tune entrypoint + Hydra run config + smoke gate (P3-06) The Stage-2 RiNALMo-giga LoRA fine-tune the P1-15 harness config was waiting for. Nothing is trained here: the §9.3 submit-ack is open, so no checkpoint, no dvc add. Inputs: data/processed/stage2_dataset.parquet (P3-01); stage2/{model,losses,heads, tokenizer}.py (P3-03/04/05); train/lora_harness.py (P1-15 §10.3 contract); reports/p1/lora_vram_smoke.json (P1-16 sizing). Outputs: src/tbox_finder/stage2/train.py; src/tbox_finder/train/ddp.py; conf/train/stage2.yaml; conf/optim/stage2.yaml; slurm/p3/stage2_lora_finetune.sbatch; tests/ml/test_stage2_train_smoke.py. The §7 stop imp.md hands this step — the train-eligibility policy — is settled by user sign-off (AskUserQuestion 2026-08-03) and encoded in row_eligibility(): admit iff fold_random == train AND NOT calib AND (nested_train is True OR fold_basis == decoy_pool_random). D5 because PRD §12 grades leave-one-order-out on the TWO-STAGE system; the scheme-A intersection so P3-07 fits its temperature on the disjoint calib split and P3-10 grades GATE-2 on THIS checkpoint rather than an ADR-0004 A6 eval twin. Measured: 9,059 train rows (5,199 corpus + 3,860 parentless decoy; 4,795 pos / 4,264 neg), 2,070 val, row-id overlap 0, partition exact. The sweep is a SLURM --array, not a literal Hydra --multirun (the P2-06 reasoning: one process per point, so an OOM in point k cannot poison point k+1). Six points = loss.aux_weight {0.0,0.5,1.0} x optim.lr {1e-4,3e-4}; aux_weight=0 is P3-08's no-aux arm. LoRA rank is NOT swept — §10.3 pins r=16 as a drift-guarded constant. Two hand-offs resolved: boundary_use_crf is False and True is REFUSED at construction; pooling stays masked-mean. DDP uses find_unused_parameters=True (a pure-decoy batch leaves the record-level heads without a gradient) with use_reentrant=False checkpointing, and the module count carrying the flag is measured, not assumed. train/ddp.py promotes the rank helpers, the PYTHONHASHSEED verifier and ShardedSampler out of train_stage1.py, which now imports them: the two trainers run in different conda envs (ADR-0002 A4), so a fork would ship the bug in one while fixing the other. Validation: 80 pass torch-free (tbox-finder-data, hydra + real-corpus tiers armed), 87 pass under tbox-ml-rna (torch tier, real LoRA wrap + backward); test_sbatch_overrides composes all ten override tokens (22 pass); test_sbatch_rm_targets clean; ruff 0.15.15 + black 25.11.0 clean; bash -n clean. 10 source sabotages, each RED against its NAMED test, each restored byte-identically. A CPU end-to-end run with max_records=32 failed its own gate on exactly one clause, full_population — the completeness clause. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(p3-06): the sbatch HF-warm probe was a SyntaxError; add a compile gate for every inline python -c block Caught by the §9.3 pre-ack check that extracts the command's OWN bytes from the shipped sbatch and executes them on the cluster, rather than running a retyped equivalent. The block wrote an escaped quote inside an f-string expression: f"attn={getattr(model.config, \"_attn_implementation\", None)}" The body is a single-quoted shell string, so the backslash survives into the Python source and the block does not parse. `bash -n` passes it — the shell string is well-formed — so the defect was invisible to every local check. The job would have died seconds in, after the queue wait; that is job 789's failure shape. Fix: bind `attn` to a name before the f-string, plus a header note saying why no backslash-escaped quote may appear in that block. Gate: tests/ml/test_stage2_train_smoke.py:: test_every_inline_python_block_in_every_sbatch_actually_compiles compiles the body of every `PYTHONPATH=... python -c '...'` block in every shipped sbatch (7 found, all parse) and carries an emptiness guard, so a regex matching nothing cannot pass it vacuously. Validation: bash -n clean; ruff 0.15.15 + black 25.11.0 clean; 81 pass / 0 fail torch-free with the hydra + real-corpus tiers armed; test_sbatch_overrides 22 pass. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(p3-06): the HF-warm probe printed the warm-load's attention backend, not the run's Running the shipped bytes on the cluster surfaced `attn=eager`, which is correct for what the probe loads and WRONG as a description of the run. `load_rinalmo_backbone()` called bare passes attn_implementation=None to from_pretrained, so transformers picks its default; the training run goes through build_peft_model, which SELECTS the backend from the measured sm_86 evidence in reports/p1/kernel_smoke.json. Verified on the cluster: flash_attn_importable=True, is_sm86=True, model_supports_flash_attn=True -> the run selects flash_attention_2. Left as it was, every job log would have advertised eager attention for a run using FA-2 — the "a report that names a backend the model never used" defect P1-15 wrote its own read-back guard against. The probe now prints both, labelled: the warm-load's value marked NOT the run backend, and the selection the run will actually make, with its reason. Validation: bash -n clean; the inline-python compile gate still passes (81 pass / 0 fail torch-free with the hydra + real-corpus tiers armed). Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * docs(p3-06): dev-log — the probe backend-label fix + the feature-branch CI counts Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(stage2): decoys carry no label_string — supervise them as all-background, and test real rows Job 1036 died on all six points. This fixes the one cause that is mine, and closes the test gap that let it reach the cluster. All 7,007 decoy rows carry label_string = None (all four pools; 0 of 23,535 corpus rows), because a decoy has no T-box element annotation. Stage2SequenceDataset.__getitem__ applied its alignment guard unconditionally and raised on the first decoy. Decoys are now supervised as ALL-BACKGROUND, not ignored — the Stage-1 convention for the same shared SegmentationHead (data/negatives.py: "every real nucleotide labelled background, with no IGNORE_INDEX ... That is not a trick"), so head (b) gets real anti-mimicry signal from 7,007 rows in the dense-background regime PRD §11's focal loss targets. User decision 2026-08-03. The index is DERIVED via labels.CLASS_CODE -> CODE_TO_CLASS -> the spec's own vocabulary, not a literal 0. The branch is fail-closed: source == decoy AND is_tbox is False, so an unlabelled corpus row still raises. THE TEST GAP, which is the real defect: _decoy() inherited a 16-char label_string from _row() that no real decoy has, and the real-corpus tier only COUNTED eligibility — nothing ever built a dataset ITEM from a real row. Closed with: one real row of every pool through __getitem__; a walk of the entire 9,059-row admitted population; and a fail-closed check that an unlabelled POSITIVE still raises. Both new branches sabotage-bitten. Also, from cause (B): the sbatch now fails fast on an unhealthy node. Node two returned "Failed to initialize NVML: Driver/library version mismatch (595.84)" and killed three points inside init_process_group. nvidia-smi + a real torch.cuda allocation now run BEFORE the 2.5 GB download, turning a 2-min opaque NCCL death into a 5-second STAGE2_NODE_UNHEALTHY exit. It asks the node whether it works rather than assuming which node is good, so §13's never-pin rule is untouched. Array serialised (%1) while one of two nodes is unusable. Validation: 92 pass / 0 fail with every tier armed; test_sbatch_overrides 22 pass; ruff + black clean; bash -n clean; 12 sabotages each RED against its NAMED test. The CPU end-to-end run now completes 16/16 steps over decoy-containing batches, all six terms contributing, failing its gate on only full_population. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * ops(p3-06): exclude the broken gpu node, dated and self-expiring; record the root cause Node `two` traced from /var/log/dpkg.log and /proc/driver/nvidia/version: it runs kernel 6.8.0-110-generic (booted 2026-07-14) with a RESIDENT nvidia module of 595.71.05, while userspace is libnvidia-ml.so.595.84. The 595 series was installed 2026-07-14 14:52 and upgraded 595.71.05 -> 595.84 on 2026-07-31 06:26; DKMS built 595.84 only for kernels -134/-136, not for the running -110 (whose nvidia.ko.zst no longer exists), and removed the old build while the loaded module stayed resident. The node has been GPU-dead since 2026-07-31 06:27 — three days before job 1036. A reboot fixes it (GRUB_DEFAULT=0 selects -136, which has the matching module); a module reload cannot. This also REFUTES the tidy explanation for cause (C): the upgrade predates the run by three days, so "a driver upgrade rolled mid-run" is not merely unsupported, it is contradicted. Point 0's SIGSEGV stays unattributed. --exclude=two is added, dated 2026-08-03 and scoped to this fault, with a test that refuses the flag unless the sbatch also carries its removal condition — so the carve-out cannot outlive the reboot and silently halve the cluster. This is --exclude, not the --nodelist §13 forbids: §13 bans pinning TO a preferred node because `one` is often down; this excludes a node MEASURED broken and leaves `one` an ordinary scheduler choice. User-approved. Validation: 92 pass / 0 fail all tiers; test_sbatch_overrides 22 pass; test_sbatch_rm_targets clean for this file (the 8 failures there are the pre-existing _KNOWN_DEFECTIVE xfails); ruff + black clean; bash -n clean. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * feat(stage2): sizing smoke that measures THE shipped step, after job 1044 OOM'd Job 1044: all 6 points CUDA-OOM in the backbone forward, 13.97 GiB allocated of 15.60, ~16 min in, 0 checkpoints. The three job-1036 causes are gone — (A) fixed, every point reached training over decoy batches; (B) neutralised, all six placed on `one`; (C) did NOT recur, no SIGSEGV anywhere including the aux_weight=0.0 arm. ROOT CAUSE OF THE MIS-SIZING, and it is methodological: both submits were sized from reports/p1/lora_vram_smoke.json (P1-16, batch 8 -> 2.014 GiB). That report's own config says loss_is_placeholder: true, data_is_synthetic: true, is_science: false, seq_len_nt: 350, 5 steps — a bare backbone under a placeholder loss, no heads, no objective. It was extrapolated to a seven-head objective on 552-token sequences for 1,420 steps. The wall-clock was labelled an extrapolation; the VRAM headroom was never questioned. imp.md's Compute line had asked for the 100-record smoke that was skipped. The correction, in two parts: 1. train.forward_backward is EXTRACTED from the training loop, so the sizing harness measures the step the trainer runs rather than a copy of it. A harness that reimplements the step can drift from it exactly as P1-16 did; `real_objective` is a gate clause reading step_function == the trainer's own, beside data_is_synthetic/loss_is_placeholder false, and a test asserts by INTERCEPTION that measure_batch really calls it. 2. stage2/sizing.py + slurm/p3/stage2_sizing_smoke.sbatch: 1 GPU, no DDP, real rows from the real admitted population. Descending batch sweep (8,4,2,1) with OOM caught per point; WORST-CASE and typical batches (59-552 tokens, collator pads to batch max — sizing on the median is how this failed); SIX optimiser steps because AdamW allocates its state on the first .step() and a growing footprint only shows as a series (growth_ratio); gradient-checkpointing effectiveness as an on/off comparison. The gate grades the MEASUREMENT, not the answer: "batch 8 does not fit" is a successful run and exits 0. A first draft of `swept` demanded a point that ran steps, which failed an all-OOM sweep — a gate pressuring the harness toward a convenient number, the very failure being corrected. It now requires a DEFINITE outcome per point (ran, or OOM'd). Validation: test_stage2_sizing 17 pass / 0 fail (torch + real-corpus tiers armed); the trainer suite still 92 pass after the extraction; ruff + black clean; bash -n clean; 4 further sabotages each RED against its named test (16 across the step). Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(stage2): re-size from the measurement (batch 4) and stop claiming a checkpointing no-op Job 1051 GREEN. Measured on one A4000 over 200 real admitted rows through the trainer's own forward_backward: batch 8 worst_case (551 tok) OOM batch 8 typical (417 tok) 11.6641 GiB 857 ms batch 4 worst_case (551 tok) 8.1827 GiB 596 ms <- chosen batch 2 worst_case 4.8775 GiB 335 ms batch 1 worst_case 3.1640 GiB 202 ms Job 1044 is fully explained: batch 8 TYPICAL fits, batch 8 WORST CASE does not — a run that trains ~16 min then dies on the first long-sequence batch. growth_ratio 1.01-1.04 across six steps, i.e. FLAT: a per-step footprint, not an accumulation, so batch size is the whole fix. THE FINDING THE GATES COULD NOT SEE: gradient checkpointing is a NO-OP on this backbone. 3.1640 GiB on vs 3.1595 GiB off — saving ratio 0.9986 — across 36 modules carrying the flag. The source confirms it: modeling_rinalmo.py advertises supports_gradient_checkpointing = True (:82) and stores self.gradient_checkpointing (:118), but the encoder loop (:704) never calls _gradient_checkpointing_func. P1-16's report records bf16_and_gradient_checkpointing as a PASSING gate clause; this step's gradient_checkpointing_verified counts flagged modules and build_model raises when the count is zero. All three are satisfied by an attribute nothing reads — flag-counting where only effect-measuring would do. The sizing harness caught it by measuring a difference instead of reading a setting. Response: gradient_checkpointing ships FALSE with the measurement and source lines recorded, plus a drift-guard test asserting the port still lacks the hook, so a future multimolecule that adds it makes the test fail rather than the project silently foregoing the optimisation. Hand-wiring torch.utils.checkpoint is deferred, not assumed. Re-sized: batch_size=4, gradient_accumulation_steps=2 (effective batch unchanged at 8), eval_batch_size=8. ~283 steps/epoch/rank at ~500 ms => ~25-30 min/point against a 2 h limit. The stale P1-16 rationale is DELETED from conf/train/stage2.yaml, not left beside the new one. Validation: test_stage2_sizing 18 pass, trainer suite 92 pass, test_sbatch_overrides 22 pass; ruff + black clean; bash -n clean. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(stage2): scope the run gate to the rank that holds its evidence Job 1053 point 0 TRAINED SUCCESSFULLY — 10 epochs, best val 0.001677 at epoch 4, rank 0's report overall_pass: true, 51.4 MB adapter, 13,040,268 saved params — and wrote no .OK, because ranks 1-7 raised on `checkpoint_written, provenance_recorded`. Only the primary rank saves the adapter and takes the git snapshot, but every rank built the report AND judged it. On ranks 1-7 those two clauses were correctly False, and raising on them turned a healthy run into a non-zero exit: torchrun propagated it, the sbatch's rc check skipped the cp out of node-local scratch, and the EXIT trap deleted the checkpoint. ~45 GPU-minutes destroyed at the finish line. Third variant of one theme in this step, worth naming: a clause is only as good as the SCOPE it is evaluated in. P2-10d'-c read state the run mutated; the checkpointing clause counted a flag instead of measuring an effect; this grades artifacts on processes never meant to produce them. Each passed its unit tests, because each was tested where its evidence existed. Fix: non-primary ranks build the report and RETURN WITHOUT JUDGING. Every rank still raises on its own OOM/NaN/collate failure — those propagate out of the loop and never reach the gate, and that path caught jobs 1036 and 1044. No collective, so no barrier to get wrong. Guarded by a clause-level test (a non-primary report derives exactly those two False while every OTHER clause still holds) and an ast/source check that the early return precedes validate_report. Sabotage-bitten by deleting the return. The checkpoint-copy behaviour is left as is, deliberately: a run that fails its gate must not leave a checkpoint. The bug was the gate, not the copy. Also: node two is FIXED (verified — kernel 6.8.0-134-generic, resident module 595.84 == userspace 595.84, nvidia-smi lists 8 A4000s). The cluster is now heterogeneous (one on 590.48.01), so --exclude=two is RETAINED for a different, weaker reason — holding the driver constant across six points meant to be compared. Its rationale is REWRITTEN, not left: the fault it cited no longer exists. The guard test now requires a dated rationale plus a removal condition rather than one specific sentence. Validation: trainer 94 pass, sizing 18 pass, overrides 22 pass; ruff + black clean; bash -n clean; 17 sabotages across the step. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * feat(stage2): drop the node carve-out, record the driver each run actually used User decision 2026-08-03: --exclude=two and --array=0-5%1 both removed. The node fault they guarded is fixed, and the %1 comment had already committed to its own removal ("Restore 0-5 once both nodes are healthy"), so this executes a recorded decision. Trigger: cryosparc holds 2 of 8 GPUs on EACH node, so a whole-node request cannot start on either until those clear (the P2-06 starvation pattern, ~4 h), and excluding a node halved the chance of one freeing. sbatch --test-only reporting a 2027 start is SLURM's infinite-TIMELIMIT placeholder. ACCEPTED CAVEAT: the nodes now run different drivers — one on 590.48.01, two on 595.84 — so points may land on either. Chosen over an open-ended wait. Expected immaterial (same CUDA 12.x, same torch build, same pinned wheels), but expected is not controlled, and six arms that exist to be compared should not be uniform on an axis only by assumption. So the axis is RECORDED: train.device_record() writes GPU name, capability, visible-device count, torch version, CUDA runtime AND the kernel driver version into every report. The driver comes from nvidia-smi, not torch — torch.version.cuda is the CUDA RUNTIME, which is not what diverged; reporting it would have looked like provenance while recording the wrong number. It is provenance, NOT a verdict: a run on either driver is legitimate, so the block is not a gate clause, and a test asserts that stripping it leaves the derived clauses unchanged — a device block leaking into the gate would fail a good run for landing on the other node. A second test asserts device_record() survives a missing nvidia-smi and records None. Validation: trainer 96 pass, sizing 18 pass, overrides 22 pass; ruff + black clean; bash -n clean. The sbatch guard now requires any future carve-out to carry a date and a removal condition, and requires the accepted caveat to stay written down. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(stage2): provenance outputs must be FILES — the adapter is a directory Job 1064's first two points trained, passed their gate, and copied 48 MB checkpoints to the repo path — then crashed writing provenance.json with IsADirectoryError. The sbatch declared `{ckpt_dir}/lora_adapter` as a provenance output; that is PEFT's save_pretrained layout, a DIRECTORY, and provenance.sha256_file opens each declared output as a file. sha256_file is NOT weakened: raising on a non-file is correct fail-loud behaviour for a shared helper used across the repo. The defect was the declaration. Fix: train.checkpoint_output_files(ckpt_dir) enumerates every FILE under the checkpoint, recursively and sorted, and refuses both silent-nothing cases (not a directory; a directory with no files). Shared by the sbatch and the pending backfill so the two cannot disagree about what a checkpoint's outputs are. Enumerated rather than hard-coded so a PEFT version writing a different file set is hashed in full instead of silently under-recorded. Notably this failure was CHEAP where the previous one was not: the copy out of node-local scratch precedes the sidecar write, so the checkpoints survived — the same bug class as job 1053's, at ~0 GPU-h instead of ~45 GPU-min, because that ordering was fixed deliberately. Tests: the enumerator yields only files, sorted, never the adapter dir; every declared output is hashable by the real sha256_file; the directory itself still raises IsADirectoryError so the contract is asserted rather than assumed; both refusal paths covered. Sabotage-bitten by dropping the is_file() filter. The inline-python compile gate now also covers heredoc blocks (`python - <<'PY'`), which is where the provenance writer lives — an unparsed heredoc is as dead as an unparsed -c block, though note compiling would NOT have caught this one. Validation: trainer 97 pass, overrides 22 pass; ruff + black clean; bash -n clean. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * feat(p3-06): backfill script for the provenance sidecars job 1064 could not write Every point of job 1064 trains, passes its gate and copies its checkpoint, then dies writing provenance.json. The checkpoints are intact; only the sidecars are missing. This rebuilds them from each run's OWN report — the artifact the producing node wrote, carrying git_sha, env_lock_sha256, seed, config, population census and the device record. Nothing is invented: every field is either copied from that report or computed by hashing the artifact. It is WEAKER than a native sidecar and says so in its own output: `reconstructed: true`, the reason, the report it came from, and an explicit limitation field. A reader must be able to tell a backfilled record from a native one without reading the script. Two guards found by testing the write path rather than assuming it: 1. build_provenance derives the TOP-LEVEL git_sha from the working tree it runs in and takes no override — so a backfill from a moved checkout would silently attribute the run to the wrong commit, in the very field CLAUDE.md §11 names. Now VERIFIED: if the tree is not on the run's own commit the sidecar is deleted and the point refused, making "back-fill before re-syncing" an enforced precondition rather than something to remember. Confirmed by watching it refuse and unlink. 2. A report whose gate did not pass is refused outright — a sidecar must never describe a failed run. Confirmed by flipping overall_pass in a real report. Also: the tally counts actual writes, so a write-time refusal cannot be reported as a write (it said "wrote 1" while writing none). Exercised end-to-end against a REAL cluster report: 2 inputs and 3 outputs hashed, all 64-hex, git_sha 5a215d7 carried from the run, driver 595.84, best_val 0.00037977. The write path surfaced two missing-input failures in my mock before it worked — provenance hashes inputs too, which is why the backfill must run where the dataset and vocab live. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * result(p3-06): job 1064 landed — six Stage-2 LoRA checkpoints, six passing gates Artifact-based verification (sacct off): array gone from squeue, six reports, six 48 MB checkpoints (4 files each, stage2_heads.pt present), zero real failures. The only .err error was 6x IsADirectoryError, uniformly the known sidecar defect. point aux_w lr best_val epoch driver aux0.0_lr1e-4 0.0 1e-4 0.000380 6 595.84 aux0.0_lr3e-4 0.0 3e-4 0.000599 8 590.48.01 aux0.5_lr1e-4 0.5 1e-4 0.023589 8 590.48.01 These totals are NOT comparable across arms: at aux_weight=0 the objective IS the binary term alone, at 0.5 it is that plus five weighted aux terms. Arm comparison is P3-08's job, on the calibrated binary head with matched metrics; the reports carry the per-term breakdown it needs. Driver split 3/3 across the two nodes, recorded per point in each report's device block — the accepted caveat made auditable rather than assumed. Provenance: all six sidecars were lost to the IsADirectoryError and are RECONSTRUCTED from each run's own report, each carrying reconstructed: true, the reason, the source report and an explicit limitation. Two guards earned their place: the git_sha check (deletes the sidecar and refuses on a moved checkout), and the constraint it exposed — the script had imported a helper added AFTER the run's commit, while the guard requires running AT it. A repair script cannot depend on code newer than the tree it repairs, so the enumeration is now defined in the script with tests/unit/test_backfill_provenance.py pinning it to the shared helper, both refusal paths included, so the deliberate fork cannot drift. Retrieval verified: all 24 files match their recorded sha256. dvc add -> stage2_rinalmo.dvc (md5 b3d1c09c…dir, 313,437,285 B, 30 files) into the SHARED main-repo cache via .dvc/config.local, since a worktree's own .dvc/ is empty and the object would land where phase-exit dvc push never looks. Still owed: the six .OK markers were never written (the sidecar crash preceded them), so the §9.3 verification stands on reports + checkpoints + .err scan — a weaker signal, stated as such. The corrected sbatch (cfb8971) writes both natively. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(p3-06): committing the reports turned both sbatch files into tracked-path deleters CI went red on the RESULT commit. tests/unit/test_sbatch_rm_targets.py compares every `rm` target against `git ls-files`, and both p3 sbatch files did `rm -f "$REPORT"`. That was safe while reports/p3/**.json were UNTRACKED — which is why it passed locally and in CI up to 0f968fd. The RESULT commit committed all six sweep reports plus stage2_sizing.json, which retroactively made those `rm`s delete tracked paths. The test's verdict changed because the repo state changed, not because the test is flaky, and the rule it enforces is real: deleting a tracked path dirties the tree outside _DATA_STAGING_PREFIXES, which in the Stage-1 design makes _provenance_complete derive FALSE and a run fail its own gate AFTER training. Fix is the P2-09 pattern the failure message names: do not rm a tracked report, fingerprint it. Both files now `rm -f` only their untracked marker ($OK / $DONE) and take an md5 before the run, then assert after it that the report CHANGED. Both failure directions are explicit — an uncomputable fingerprint must not pass by being unequal to the old one, and an unchanged one must not pass at all — so "a report exists" can no longer be mistaken for "this run wrote it". Validation: test_sbatch_rm_targets clean for both p3 files (the 8 remaining failures there are the pre-existing _KNOWN_DEFECTIVE xfails); trainer 97, sizing 18, overrides 22, backfill 3; ruff + black clean; bash -n clean on both. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(p3-06): CodeRabbit r1 — LR schedule domain, accumulation flush, optim guard, +5 Review gate met via the CodeRabbit CLI. The GitHub app returned only its fair-usage notice and Copilot returned "quota limit reached"; Greptile was invoked and stayed SILENT for a third consecutive time (PR #98 x2, PR #99 x1), spending a credit for nothing. 11 findings, each verified by execution before being accepted or refused. THE ONE THAT REACHES THE SHIPPED CHECKPOINTS: the LR scheduler's domain was micro-batches while scheduler.step() fires per OPTIMISER step. With accumulation 2, job 1064's cosine covered exactly 50% — 1,415 of 2,830 — ending at 0.5501x base instead of ~0. All six arms trained under a schedule their config does not describe. User decision: fix the code, KEEP the checkpoints, record the defect. Nothing downstream is invalidated (P3-08 compares arms that all share it), but they are not optimally-annealed models and any absolute number derived from them carries that. A CLAIM I MADE WAS WRONG: I reported growth_ratio 1.01-1.04 as a flat PER-STEP footprint. reset_peak_memory_stats() ran once before the loop, so the series was a RUNNING MAXIMUM, monotonic by construction. The no-leak conclusion survives — a leak would still raise a running max — but the instrument was mislabelled and I oversold it. Fixed; the sbatch header repeating the claim is corrected too. Also fixed, each sabotage-bitten: unknown optim.* keys raise as loss.* ones do; accumulation counts per-epoch and flushes the trailing group (283 odd micro-batches made the cumulative phase shift every epoch); the sbatch clears the checkpoint dir only AFTER the health check; WORLD_SIZE=0 raises; the backfill reads optim.lr from its real group and refuses null seed/env_lock/input; sizing COMPUTES headroom instead of asserting it. TWO OF MY OWN NEW TESTS WERE VACUOUS. The LR-domain test asserted arithmetic and never touched train_stage2's wiring — reverting the trainer left it green. Now asserts the shipped construction. (The sbatch test did bite; my sabotage for it was wrong.) Refused with reasons: device_record()'s index 0 is correct — every rank calls set_device(local_rank) first. Validation: trainer 102, sizing 18, overrides 22, backfill 3; ruff + black clean; bash -n clean; 23 sabotages across the step. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(p3-06): CodeRabbit r2 — the checkpoint advertised a val its weights never achieved 12 findings, each verified by execution. r2 was sharper than r1 because it reviewed r1's fixes. THE ONE THAT CORRECTS A REPORTED NUMBER: the checkpoint block carried best_val_total while saved_from_epoch is 9 in every arm — there is no best-checkpoint restore. So best_val_* described epochs whose weights were discarded: point saved best reported SAVED weights score aux0.0_lr1e-4 9 6 0.000380 0.008841 (23x) aux0.0_lr3e-4 9 8 0.000599 0.002500 (4x) aux1.0_lr1e-4 9 9 0.039954 0.039954 (best == final) The RESULT table I wrote quoted the best-epoch column. Fixed: `saved_val_total` is now in the checkpoint block with a `note` saying the weights are the final epoch's and arms compare on it; best_val_* is renamed *_observed_during_training. AND r1 HAD POINTED AT THIS — it flagged the config comment calling the val rung a "best-checkpoint signal", a minor I filed as wording and folded into the schedule fix instead of following to the artifact. A comment claiming behaviour the code lacks is a defect report about the CODE; editing the comment is the wrong fix when the comment describes what a reader would want to be true. Other majors, each sabotage-bitten: steps_ran now grades the optimiser domain too (it could not have seen its own regression return); gradient_checkpointing_verified renamed gradient_checkpointing_flag_consistent (counting an attribute on 36 modules while the hook is never called is not verification); the sizing recommendation is re-derived by the validator; the sbatch checkpoint clear moved again, to immediately before the copy — past the health check still let an OOM end a point with nothing and the previous arm deleted; the backfill excludes its own sidecar so --write is idempotent; num_workers deleted (no DataLoader exists, so reports claimed a setting never applied). THREE MORE OF MY TESTS WERE WEAKER THAN THEY LOOKED, all found by sabotage: steps_ran passed with either comparison removed (they covered for each other), the sbatch test asserted only "after the health check" rather than the property r2 established, and one anchor was stale. Five such tests across this step now. Validation: trainer 110, sizing 20, overrides 22, backfill 4; ruff + black clean; bash -n clean; 28 sabotages across the step. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(p3-06): Greptile r3 — NaN-gradient blindness; and undo a false claim I put in CLAUDE.md Greptile DID review PR #99 — ~18 minutes after the trigger, as greptile-apps[bot], with 3 P2 findings. A 6-minute poll had concluded "silent", reported the credit wasted, and WROTE INTO CLAUDE.md §5.1a that the trigger path was dead. That was a false conclusion from an impatient poll, promoted to documentation where a future session would have obeyed it. §5.1a corrected: it works, budget ~25 min, and look in pulls/<n>/reviews for greptile-apps[bot] — the reply is a REVIEW, not an issue comment, so a poll reading only issue comments misses it. The prior PR #98 "silences" are now suspect for the same reason. Findings, all real: (P2) losses_finite was blind to a NaN gradient on any non-primary rank. n_nonfinite counts the RANK-0 FORWARD LOSS; under DDP a NaN gradient from another rank all-reduces into every rank's gradients and poisons the weights while rank 0's loss stays finite. Fixed by reading clip_grad_norm_'s pre-clip total norm — computed after the all-reduce, non-finite iff some gradient is — as n_nonfinite_grad_steps, now a second term in the clause. This is the THIRD gate in this step that measured the wrong side of what it named, after the checkpointing flag-count and the micro-batch-only step check. (P2) _bool_or_none blamed the wrong column: written for nested_train, also called for calib and is_tbox, so a "yes" in calib would raise an error naming nested_train. Now takes the field name; every call site passes its own. (P2, advisory) Ternary precedence in three clauses — semantics confirmed correct, readability only. Left as-is and recorded rather than silently dropped. Also corrected in §5.1: CodeRabbit is INCREMENTAL and AUTOMATIC. @coderabbitai review on an already-reviewed commit is a no-op. Both rounds I attributed to manual triggers were the automatic reviews of the pushes; coverage reads from original_commit_id, not from whether a command was sent. And a green CodeRabbit CHECK does not mean a review ran — it read pass while the body was only the Fair-Usage notice. Validation: trainer 113, sizing 20, overrides 22, backfill 4; ruff + black clean; bash -n clean. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(p3-06): schema 1->2, and PIN the job-1064 gate failure instead of excusing it All seven committed reports failed their own validators after r2/r3 strengthened the clause sets — found by running the validators against the committed artifacts rather than assuming they still applied. SCHEMA_VERSION bumped 1 -> 2 on both modules: a clause set is part of a report's shape, so adding one makes earlier reports a different schema, not a corrupt one. The two failures are categorically different and must not be conflated: steps_ran — a REAL defect. Job 1064 sized the cosine in micro-batches; the optimiser advanced 1,415 of 2,830 scheduled steps. Schema 2 refuses those runs correctly. The habit on a version bump is to excuse a clause the old schema lacks — right only when the old report re-derives TRUE. This re-derives FALSE for a real reason, so excusing it would launder a known defect through a compatibility shim. Asserted as failing instead. losses_finite — NOT a defect, an absence. Schema 1 predates n_nonfinite_grad_steps (added for Greptile's NaN-gradient finding), so the post-all-reduce check was never measured. Unmeasured is neither passed nor failed; excusing it into TRUE would assert those runs had finite gradients when nobody looked. Pinned by test: six reports present (emptiness guard first), all schema 1, failing EXACTLY ["losses_finite", "steps_ran"] — a new entry means a second defect, a missing one means a clause stopped biting or the reports were regenerated. The distinction is checked, not described: the scheduler clause has its evidence and fails on it, the gradient clause has no evidence at all, and what schema 1 did measure was clean. The reports are kept as written — they are job 1064's own record, and the decision was to keep the checkpoints and RECORD the defect. Regenerating them to pass is the opposite. Validation: trainer 116, sizing 20, overrides 22, backfill 4; ruff + black clean. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(p3-06): CodeRabbit r4 — annotate the artifacts, stop grad_clip=0 corrupting gradients Three findings from the manual review (auto-review is disabled on this repo, so the manual comment is the live path). All real. (High) The producer schema was fixed in r2 but the ARTIFACTS stayed misleading: a reader saw best_val_total: 0.00037977 beside saved_from_epoch: 9 and had to infer they describe different weights. Four of six arms have a best epoch before 9; two have best == final, so an annotation claiming a gap everywhere would itself be wrong. Each report now carries a `legacy` block with saved_val_total derived from that file's OWN val.history, an explicit statement that best_val_* describe discarded weights, and the clauses it fails under schema 2. ANNOTATED, NOT REWRITTEN — 84 insertions, 0 deletions, no measured value touched. (Medium) grad_clip == 0 was silently corrupting gradients. I wrote clip_grad_norm_(params, inf) believing it a no-op; PyTorch scales by max_norm/(total_norm+1e-6), which is nan when both are infinite, and multiplies it through. Verified on the pinned torch 2.7.1: [inf,1,2] came back [nan,nan,nan] while the returned norm still read inf — detection signal fine, optimiser input not. gradient_total_norm now uses get_total_norm when clipping is off. (Medium) TWO REGRESSION TESTS WRITTEN FOR THE PREVIOUS ROUND WERE STILL VACUOUS. The step-count test reimplemented the loop instead of running it — deleting scheduler.step() from the flush left it green. The NaN test edited a report fixture — deleting the counter increment left it green. Both are now PRODUCER tests calling train_stage2 on CPU with odd micro-batches and accumulation 2, spying on the real optimizer/scheduler steps and injecting a real inf into a live gradient. Both of CodeRabbit's counterexamples now fail, verified by source mutation. Seven vacuous tests across this step; the pattern is that I keep testing the predicate and calling it a test of the producer. Three assertions I wrote this round were also wrong, each caught by running them: that the forward loss stays finite under a poisoned gradient (stepping on inf poisons the weights); that n_nonfinite_grad_steps >= n_nonfinite_steps (different denominators — micro-batches vs optimiser steps); and a sabotage that removed one of the counter's two increment sites. Confirmed by the reviewer, not assumed: backward() returns only after every DDP bucket all-reduce completes, including with find_unused_parameters=True, so the norm is a valid post-reduction signal. Validation: trainer 121, sizing 20, overrides 22, backfill 4; ruff + black clean. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 * fix(p3-06): CodeRabbit r5 — derive the annotation flag instead of asserting it r5 via the CLI, scoped with --base-commit c6fc074 to the two unreviewed commits, and via the CLI rather than a PR comment so it consumed no GitHub review quota. ONE finding, minor. Series across the step: 11 -> 12 -> 3 -> 3 -> 1. The finding is a self-inflicted inconsistency: the legacy annotation hardcoded best_val_total_is_NOT_the_saved_weights: True on all six arms, while aux1.0_lr1e-4 and aux1.0_lr3e-4 have best epoch == 9 == saved. The test written in the SAME commit asserts that four of six differ — so the annotation contradicted its own test file. Now derived from the epoch comparison, with two note wordings: one for arms where best_val_total describes discarded weights, one for arms where the two agree and neither is misleading (and why that does not make the distinction moot). Still annotation-only: 84 insertions, 0 deletions against the pre-annotation commit. The test asserts the flag equals the epoch comparison and that the note matches its case. Verified non-tautological by sabotage: restoring the hardcoded True turns it red. Four consecutive rounds found real defects in the previous round's fixes, so none was a safe stopping point. This one found a documentation-consistency nit in an annotation block — no behaviour, no gate, no artifact. Validation: trainer 121, sizing 20, overrides 22, backfill 4; ruff + black clean. Claude-Session: https://claude.ai/code/session_01N8NJ1ksgJeJzV5Q1JHGkt1 --------- Co-authored-by: bioedca <bioedca@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds Greptile as the step-3 review-gate fallback (CLAUDE.md §5.1) for when CodeRabbit is rate- or quota-limited, under two constraints: it never fires on its own, and it is capped at 16 reviews/month from 2026-08-03.
Never automatic
.greptile/config.jsonpinsskipReview:"AUTOMATIC"— the exact literal (any other value is invalid and silently restores auto-review) — plustriggerOnUpdates,statusCheck,statusCommentsEnabledandautoApproveallfalse.ignorePatternsmirrors.coderabbit.yaml'spath_filtersso the fallback reviews the same scope as the primary, and self-excludes.greptile/.Repo config outranks dashboard settings; only org-enforced rules could override it, and this repo is user-owned, so none exist.
The 16/month cap
Greptile has no native per-repo review quota (its only usage control is an org-wide dollar cap on flex spend), so the cap is enforced repo-side by
scripts/greptile_budget.py. The count is re-derived from the GitHub API on every call, never from a ledger this repo writes about itself (§10.3), and it fails closed: exit0available /2exhausted /3could-not-measure, printing no budget number on the unmeasured path.Counting unit — the trigger, not Greptile's output
Sampling real public PRs shows Greptile's output surface is not a stable unit: it edits one sticky
<!-- greptile-status -->comment in place across re-reviews (observed: 2 reviews, 1 comment), often posts a summary with zeroPullRequestReviewobjects, and emits inline comments that scale with findings. With auto-review disabled, every review is preceded by exactly one human@greptileaicomment — so one trigger == one review.Greptile activity on a thread with no in-period trigger is charged as budget and raised as
auto_fire_suspected, so the cap doubles as the detector for the config not being in force.Validation
ruff==0.15.15+black==25.11.0(CI pins) clean; live run reports0/16used.used>=limit→used>limit), counting bot output instead of triggers, dropping the auto-fire charge, pinning one bot login, HTTP errors no longer failing closed, and swapping the repo-wide comment feed for a PR enumeration.greptilelogin prefix +type=="Bot"rather than one hardcoded login: Greptile owns two bot identities (greptile-apps[bot], the active reviewer, andgreptile[bot]), whilegreptile-apps/greptile/greptileaialso exist as non-Bot decoy accounts.Known: this PR may itself get auto-reviewed
Greptile's PR-level rule is conjunctive — it skips "only if all applicable configs specify AUTOMATIC" — and
maindoes not carry this file until merge. Verified precedent: NVIDIA/Megatron-LM#5166, the PR that restored their config, was auto-reviewed with zero@greptileaimentions. If it happens here the budget counter will flag it asauto_fire_suspectedand charge it, which is the intended behaviour.