Conversation
`peak_rss_bytes` was false on Windows. A witness settles it rather than a manual page: a child that commits 256 MiB and never touches a page is reported as 262.5 MiB by the job object's PeakProcessMemoryUsed, while on Linux nothing becomes resident. The job object counts COMMITTED memory — job and process memory limits are defined on committed virtual memory — and `ru_maxrss` counts RESIDENT pages. One field name claimed both. The same witness refuted the other half of the suspicion. `wait4` on the immediate child does NOT stop at the shell: shell -> heavy child and shell -> shell -> heavy child both return the descendant's 260 MiB against 13 MiB for a light control, so the POSIX path already measures the tree it should. So the quantity now travels with the number. Each sample and each cell carries `memory_metric`, drawn from a closed set — `max_process_peak_resident` on the wait4 and `/usr/bin/time -v` paths, `max_process_peak_commit` on the Windows job object — and a value produced under any other kind raises rather than being recorded. No downstream reader infers semantics from `sys.platform`, and no alias lets Windows commit go on being read as RSS. Renamed with it, because the old nouns lied in the same way: `peak_rss` -> `peak_memory`, `rss_mechanism` -> `memory_mechanism`, `rss_unavailable_reason` -> `memory_unavailable_reason`, `raw_peak_rss_bytes` -> `raw_peak_memory_bytes`, `RssProbe` -> `MemoryProbe`. Deliberately NOT done: Windows resident-set sampling. `PeakWorkingSetSize` is the resident analogue, but obtaining it for a tree of processes that come and go needs handle tracking or polling, and this instrument does not sample. Two operating systems answering different, honest questions beats a profiler added so they can pronounce the same noun. This moves `measurement_harness_digest` 562a7f7232da -> 104c384d01bf. The bindings that name the old digest are re-bound in the next commit; nothing here tries to preserve it. perf instrument controls 16/16, round 7 apparatus controls 10/10. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018xhcg5opoFbSTdYHpkSXCh
The memory-semantics repair moved `measurement_harness_digest`
562a7f7232dad2f4c79c6adfe0e1e7e25680b4b6f3444d54824bf0405e3c14b3 before
104c384d01bf6060bdec1e7c916053ddb04b97fcbd0b39f8a4fc57b8f139672f after
and three controls said so, each by name: `freeze-harness-untouched`,
`training-prereg-bindings` and `envcapture-frozen-untouched`. That is the
provenance system delivering its inconvenience on purpose, so nothing here tries
to preserve the old digest.
Re-bound: the digest in the policy freeze, in the ratified design constants, and
in the training preregistration's bindings — and with it
`design_constants_blob_sha1`, which moved because the design-constants artifact
itself was re-bound. A chain of bindings re-bound in the order the chain runs.
Not touched: the committed evidence of runs that actually happened. The sizing,
calibration and round-7 datasets were produced by the old instrument and record
what it produced; rewriting them would be forging a record, not re-accepting a
binding. They carry `peak_rss` because that is the field the instrument had when
they were taken.
This is a re-acceptance of the step-4/5/6 bindings, not a code change; it is a
separate commit so it can be reviewed as one. Controls after re-binding:
calibration constants 4/4, calibration freeze 7/7, calibration policy 10/10,
perf instrument 16/16, round 7 apparatus 10/10, step 7 environment capture
11/11, training preregistration 9/9.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018xhcg5opoFbSTdYHpkSXCh
The section was called "Peak RSS" and described two mechanisms as though they produced one number. They do not: POSIX `ru_maxrss` is a peak resident set over the waited-for descendant chain, and the Windows job object's `PeakProcessMemoryUsed` is peak committed memory over the processes of the job. The witness that settles it is recorded here rather than left in a session log — 256 MiB committed and never touched reads as 262.5 MiB on Windows and as nothing resident on Linux — as is the half of the suspicion the same witness refuted: `wait4` does not stop at a wrapping shell. Recorded with it: the strata answer different honest questions and may not be pooled across platforms; Windows resident sampling was considered and refused because this instrument does not sample; and the digest moved 562a7f7232da -> 104c384d01bf, with steps 4/5/6 re-bound in their own commit and the committed evidence of past runs deliberately left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018xhcg5opoFbSTdYHpkSXCh
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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
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.
S8 from the hostile freeze review of #354. Instrument semantics repair — no policy, no threshold, no authorization.
What a witness established
peak_rss_byteswas false on Windows, and the witness settles it rather than a manual page.The job object counts committed memory — job and process memory limits are defined on committed virtual memory — while
ru_maxrsscounts resident pages. One field name claimed both.The same witness refuted the other half of the suspicion:
wait4does not stop at a wrapping shell. The POSIX path already measures the tree it should.The repair
The quantity now travels with the number. Every sample and every cell carries
memory_metricfrom a closed set:max_process_peak_resident— POSIXwait4, and the/usr/bin/time -vfallbackmax_process_peak_commit— the Windows job objectA value produced under any other kind raises
InstrumentErrorrather than being recorded. No downstream reader infers semantics fromsys.platform, and no alias lets Windows commit go on being read as RSS.Renamed because the old nouns lied the same way:
peak_rss→peak_memory,rss_mechanism→memory_mechanism,rss_unavailable_reason→memory_unavailable_reason,raw_peak_rss_bytes→raw_peak_memory_bytes,RssProbe→MemoryProbe.Deliberately not done: Windows resident-set sampling.
PeakWorkingSetSizeis the resident analogue, but obtaining it for a tree of processes that come and go needs handle tracking or polling. This instrument does not sample, and will not perturb the elapsed-time interval so two operating systems can pronounce the same noun.The digest moved, on purpose
Three controls said so by name —
freeze-harness-untouched,training-prereg-bindings,envcapture-frozen-untouched— which is the provenance system delivering its inconvenience on purpose. Steps 4/5/6 are re-bound in their own commit (includingdesign_constants_blob_sha1, which moved because the design-constants artifact was itself re-bound).Committed evidence of runs that actually happened is not rewritten. The sizing, calibration and round-7 datasets record what the old instrument produced and still carry
peak_rss; rewriting them would forge a record rather than re-accept a binding.Controls
calibration constants 4/4 · calibration freeze 7/7 · calibration policy 10/10 · perf instrument 16/16 · round 7 apparatus 10/10 · step 7 environment capture 11/11 · training preregistration 9/9 · evidence coverage 12/12.
The repository runner also reports 17
FAIL[...]lines fromvariant-corpus,variant-ledgerandartifact-golden. Those are pre-existing on this Windows checkout — the same 7 + 10 appear at the base commit6194d7e— and are byte-exact fixtures against a CRLF working tree, unrelated to this change.🤖 Generated with Claude Code
https://claude.ai/code/session_018xhcg5opoFbSTdYHpkSXCh