fix(strip-history): strip inputs/ too, and print what actually remains - #27
Merged
Merged
Conversation
The previous strip set left 71.58 MiB behind while every check reported
success. Measured in a scratch clone: 269.43 MiB -> 81.13 MiB, with
"VESPA/Multiplex blobs remaining (expect 0): 0" printed truthfully, and
71.58 MiB of that pool's content still present under inputs/fastq/.
The cause is structural, not a typo in the path list. `git rev-list
--objects` emits each object EXACTLY ONCE, paired with one of the paths it
is reachable under. Those two blobs lived at both data/Multiplex_pool/ and
inputs/fastq/; the census that chose the strip set saw only the first name,
so inputs/ never appeared in it at all. Removing the path did not remove the
content, and the post-strip grep for the stripped path returned 0 because it
could not have returned anything else. A check that asks "is the path I just
deleted absent" is a tautology, not evidence.
So this does two things:
- adds `inputs` and `logs_*.zip` (a committed CI log artefact no path class
covered) to the strip set. Re-measured: 269.43 MiB -> 9.17 MiB, and the
heaviest remaining path is the 7.71 MiB of LIVE .fastq.gz fixtures, which
is the correct floor.
- prints the heaviest REMAINING path aggregates after the rewrite. A path
strip can never prove a blob is gone, because the blob may have a second
name; the only honest verification is to look at what survived. The
predicted figure was ~21 MiB and the run produced 81 MiB -- that gap was
the entire signal, and nothing in the named checks would have raised it.
The HEAD-tree-identity assertion is kept and still passes. It answers a
different question -- did the strip set catch a LIVE file -- and it was never
capable of detecting dead content that was missed.
Still never runs on its own: refuses without --i-have-read-the-warnings, and
the force-push commands remain printed instructions inside a heredoc.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🔇 Additional comments (1)
📝 SummarySummary by CodeRabbit
WalkthroughThe history-rewriting script now removes ChangesHistory rewrite
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the bug, root cause, fix, measured results, and safety behaviour. It does not follow the repository template because it omits the required
✨ Finishing Touches📝 Generate docstrings
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. A rabbit checks the history trail Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
hyperpolymath
added a commit
that referenced
this pull request
Sep 21, 2026
#27) ## The bug The strip set on `main` leaves **71.58 MiB** behind while reporting success. Measured in a scratch clone: `269.43 MiB -> 81.13 MiB`, with every check green — including `VESPA/Multiplex blobs remaining (expect 0): 0`, printed **truthfully** — and 71.58 MiB of that pool's content still present under `inputs/fastq/`. ## Why, structurally `git rev-list --objects` emits each object **exactly once**, paired with **one** of the paths it is reachable under. Those 2 blobs lived at both `data/Multiplex_pool/…` and `inputs/fastq/…`. The census that chose the strip set saw only the first name, so `inputs/` **never appeared in it at all**. Deleting the path did not delete the content, and the post-strip grep for the deleted path returned 0 because it could not have returned anything else — that check is a tautology, not evidence. ## The fix | | before | after | |---|---|---| | pack | 269.43 MiB | **9.17 MiB** | | heaviest remaining | `inputs/fastq` 71.58 MiB | `data/MiSeq_SOP` 7.71 MiB — the **live fixtures**, the correct floor | - adds `inputs` and `logs_*.zip` (a committed CI log artefact no path class covered) - prints the heaviest **remaining** path aggregates after the rewrite, because a path strip can never prove a blob is gone — the blob may have a second name. The predicted figure was ~21 MiB and the run produced 81 MiB; that gap was the entire signal, and nothing in the named checks would have raised it. The HEAD-tree-identity assertion is kept and still passes — it answers a different question (did the strip set catch a *live* file) and was never able to detect dead content that was missed. Still never runs on its own: refuses without `--i-have-read-the-warnings`, and the force-push commands remain printed instructions inside a heredoc. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
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
1 participant
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.
The bug
The strip set on
mainleaves 71.58 MiB behind while reporting success.Measured in a scratch clone:
269.43 MiB -> 81.13 MiB, with every check green —including
VESPA/Multiplex blobs remaining (expect 0): 0, printed truthfully —and 71.58 MiB of that pool's content still present under
inputs/fastq/.Why, structurally
git rev-list --objectsemits each object exactly once, paired with one of thepaths it is reachable under. Those 2 blobs lived at both
data/Multiplex_pool/…andinputs/fastq/…. The census that chose the strip set saw only the first name, soinputs/never appeared in it at all. Deleting the path did not delete the content,and the post-strip grep for the deleted path returned 0 because it could not have
returned anything else — that check is a tautology, not evidence.
The fix
inputs/fastq71.58 MiBdata/MiSeq_SOP7.71 MiB — the live fixtures, the correct floorinputsandlogs_*.zip(a committed CI log artefact no path class covered)strip can never prove a blob is gone — the blob may have a second name. The predicted
figure was ~21 MiB and the run produced 81 MiB; that gap was the entire signal, and
nothing in the named checks would have raised it.
The HEAD-tree-identity assertion is kept and still passes — it answers a different
question (did the strip set catch a live file) and was never able to detect dead
content that was missed.
Still never runs on its own: refuses without
--i-have-read-the-warnings, and theforce-push commands remain printed instructions inside a heredoc.
🤖 Generated with Claude Code
https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm