ci(nix): put record back after sources, and record why the experiment failed - #447
Merged
Conversation
… failed The ordering experiment did not answer its question and could not have. By the time it ran, record had started succeeding from its old position anyway -- twice in a row, first attempt, 12-17ms -- so there was no contrast left to measure. From position 4 it succeeded, which proves nothing it was not already doing from position 9. Sources came in at 2/5, which looks like the other half of the prediction and is not readable as such: the three failures fell at invocations 7, 8 and 9 with successes either side, which is a window rather than a gradient. So the block goes back where it was. The comment says the experiment was tried and why it is not worth a third attempt, because a bare revert would leave the next person to have the same idea. What the six runs did establish is that the premise was wrong, and that is worth more than the experiment would have been. Enumeration is bimodal: 12-31ms when it answers, no return at all when it does not, nothing in between across every measurement taken. The failures cluster by run, and by window within a run, not by command. The record-versus-sources gap that started this -- 1 in 10 against 13 in 20 -- was that clustering seen through a denominator. I read it as a property of the record path and it was not one. Kept: the run_cli counter and the per-attempt labels, which cost nothing and are what makes the question answerable the next time a run actually fails. That is the condition for reopening it. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe smoke-test workflow now enumerates sources before running the record/export acceptance test. The acceptance test retains bounded retries, export gating, MP4 validation, stage-specific errors, and the final export verdict. ChangesSmoke-test workflow
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR restores the prior CI workflow ordering while retaining diagnostic counters and labels; no actionable merge-blocking risk remains after normal checks and review. 🚥 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 |
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.
What this does
Reverts the block move from #438. Keeps the
run_clicounter and the per-attempt labels from the same PR.Why the experiment did not answer
It could not have. By the time it ran,
recordhad started succeeding from its old position anyway — twice in a row, first attempt:sourcesrecordFrom position 4
recordsucceeded, which proves nothing it was not already doing from position 9. The contrast the A/B needed had disappeared from the baseline before the test ran.sourcescame in at 2/5, which looks like the other half of the prediction and is not readable as such — the labels are what show why:Three failures bracketed by successes is a window, not a gradient. Position would predict monotonic decay with rank.
What the six runs did establish
More useful than the experiment would have been, and it is that the premise was wrong.
Enumeration is bimodal: 12–31ms when it answers, no return at all when it does not, with nothing in between across every measurement taken. The failures cluster by run, and by window within a run, not by command. The record-versus-sources gap that started all this — 1 in 10 against 13 in 20 — was that clustering seen through a denominator. I read it as a property of the record path; it is not one.
What stays
The counter and the labels. They cost nothing, and they are what makes the question answerable the next time a run actually fails — which is the condition for reopening it. The comment in the file now says the experiment was tried and why it is not worth a third attempt, so a bare revert does not leave the next person to have the same idea.
Verified
Workflow YAML parses; all four embedded
runblocks passbash -n. Invocation order is back to probes →--help→ sources → record/export, confirmed against the extracted script. Stale ordering comments updated in both directions. No code changes.🤖 Generated with Claude Code
Summary by CodeRabbit