mem-check: best of 3 on a baseline overrun; baseline 176 MB (#96) #111

Merged
claude-bot merged 2 commits from fix/mem-check-noise into main 2026-10-29 10:01:55 +00:00
Collaborator

Closes #96.

What

tools/mem-check/run.sh no longer fails on one noisy sample. An attempt over a baseline (PSS or start-up) is measured again in a fresh app and sandbox, up to ATTEMPTS (3) times, and the profile passes if any attempt is within both baselines. A real regression still fails all three. Other failures (the app exited, too few chapters, SKIP) aren't retried. The PSS baseline moves from 165000 to 176000 kB, 10% over CI's median since #82. CI's growth is real feature cost (#71 +~10 MB, #82 +~5 MB), not drift.

Blast radius

Tier: trunk
Trunk paths touched: tools/mem-check/ (run.sh, budget-kb)
Testing tier: maximum (CI gate tooling) (also: ship on sight, docs/01 and docs/09)
Gate / rollback: ATTEMPTS=1 gives the old single sample, and BUDGET_KB=165000 the old baseline, with no code change. Full undo: revert this merge.

Proof

  • Why it grew (CI log history, runs 63–192, 120 app jobs; runner v11.3.1 and image ut-app-ci:24.04-2.x-1 throughout). Normal peak was 136–145 MB up to #37. It was 147–157 MB from #71's first CI (run 112, dff31c6) on, and 155–166 MB since #82 (run 150). Since #82: normal median 160 MB (n=20, 154.6–166.4), slow median 163 MB (n=17, 156.3–164.7).
  • Not the task runner's start at launch (A/B in ut-app-ci, interleaved, 5 runs each, normal profile). With Installer.start(): median 168.7 MB, range 168.4–173.3. Without it: median 169.2 MB, range 166.5–172.2. No difference beyond noise, so there's no cheap start-up saving to take instead of raising the baseline.
  • Device number still fits (Plinius, this branch's click fix/mem-check-noise@b1ae64d, from the journal's build line). Start-up PSS is 60,153 kB (54–55 MB on 10-20, so +~6 MB, consistent with the desktop's +15 MB at ~2.5×). Peak over 40 flings Matt 4→14 is 60,889 kB, against the 80 MB device budget. The app is unchanged by this PR; it's left installed.
  • Retry path in CI's image (cargo make app-docker on b1ae64d, local). The normal profile passed attempt 1 at 163,909 kB. The slow profile ran 177,041, then 167,264, then 173,308 kB, each over the then-165000 baseline, and printed mem-check: FAIL: over a baseline in all 3 attempts (exit 1). A desktop under load runs higher than CI, as #96 says, so this shows the mechanics, not the CI numbers.
  • Base fails on one sample: CI runs 150, 154, 179, 183 and 188 failed app on single samples 92–1436 kB over 165000, on trees that passed elsewhere (#96 comments). With this change each of those would have been measured again.
  • Arguments and exit codes (df978f0, local): ATTEMPTS=abc and ATTEMPTS=0 give rc 2 with a message. A child failing early (missing paths) gives rc 1 after one attempt, with no retry.
  • CI on df978f0 (run 195): changes, rust, audit, app, e2e and click all passed. The app job (1907) passed on attempt 1 in both profiles: normal 158,671 kB, slow 164,577 kB, against 176000.
  • Not shown: an attempt that fails and then passes on a retry. It depends on noise, and I didn't fake it. The code path is the same retry branch that the 3-failure run took.

Review

Reviewer: fresh-context code-review at high (review 302 on b1ae64d): 5 findings, all low; 4 fixed in df978f0, 1 declined (no parent TERM trap: CI cancels by container teardown, and a group kill reaches the child; comment 2283). No re-review (lows only).
Review-Confidence: 85/100. The review was sound and no defect was missed. −15 because df978f0's fixes to low findings were never re-reviewed.

Confidence by part

  • Retry wrapper (run.sh top): high. It is small, exit codes are checked, and it was exercised in the real image.
  • Baseline 176000: medium. It is a judgement call on 20 CI samples. With best-of-3, a +10 MB regression (the size of #71) would land at ~170 MB and pass. A human should decide if 10% over the median is still the right margin now that the gate is best-of-3. 170000 (~6%) would be the tighter option.
  • Root-cause analysis: medium. The steps line up with #71 and #82 across many runs, but no commit-level bisect was done (#71 was a 3-commit PR).

🤖 Generated with Claude Code

Closes #96. ## What `tools/mem-check/run.sh` no longer fails on one noisy sample. An attempt over a baseline (PSS or start-up) is measured again in a fresh app and sandbox, up to `ATTEMPTS` (3) times, and the profile passes if any attempt is within both baselines. A real regression still fails all three. Other failures (the app exited, too few chapters, SKIP) aren't retried. The PSS baseline moves from 165000 to 176000 kB, 10% over CI's median since #82. CI's growth is real feature cost (#71 +~10 MB, #82 +~5 MB), not drift. ## Blast radius **Tier:** trunk **Trunk paths touched:** `tools/mem-check/` (run.sh, budget-kb) **Testing tier:** maximum (CI gate tooling) (also: ship on sight, docs/01 and docs/09) **Gate / rollback:** `ATTEMPTS=1` gives the old single sample, and `BUDGET_KB=165000` the old baseline, with no code change. Full undo: revert this merge. ## Proof - **Why it grew** (CI log history, runs 63–192, 120 app jobs; runner v11.3.1 and image `ut-app-ci:24.04-2.x-1` throughout). Normal peak was 136–145 MB up to #37. It was 147–157 MB from #71's first CI (run 112, dff31c6) on, and 155–166 MB since #82 (run 150). Since #82: normal median 160 MB (n=20, 154.6–166.4), slow median 163 MB (n=17, 156.3–164.7). - **Not the task runner's start at launch** (A/B in `ut-app-ci`, interleaved, 5 runs each, normal profile). With `Installer.start()`: median 168.7 MB, range 168.4–173.3. Without it: median 169.2 MB, range 166.5–172.2. No difference beyond noise, so there's no cheap start-up saving to take instead of raising the baseline. - **Device number still fits** (Plinius, this branch's click `fix/mem-check-noise@b1ae64d`, from the journal's build line). Start-up PSS is 60,153 kB (54–55 MB on 10-20, so +~6 MB, consistent with the desktop's +15 MB at ~2.5×). Peak over 40 flings Matt 4→14 is 60,889 kB, against the 80 MB device budget. The app is unchanged by this PR; it's left installed. - **Retry path in CI's image** (`cargo make app-docker` on b1ae64d, local). The normal profile passed attempt 1 at 163,909 kB. The slow profile ran 177,041, then 167,264, then 173,308 kB, each over the then-165000 baseline, and printed `mem-check: FAIL: over a baseline in all 3 attempts` (exit 1). A desktop under load runs higher than CI, as #96 says, so this shows the mechanics, not the CI numbers. - **Base fails on one sample**: CI runs 150, 154, 179, 183 and 188 failed `app` on single samples 92–1436 kB over 165000, on trees that passed elsewhere (#96 comments). With this change each of those would have been measured again. - **Arguments and exit codes** (df978f0, local): `ATTEMPTS=abc` and `ATTEMPTS=0` give rc 2 with a message. A child failing early (missing paths) gives rc 1 after one attempt, with no retry. - **CI on df978f0** (run 195): changes, rust, audit, app, e2e and click all passed. The `app` job (1907) passed on attempt 1 in both profiles: normal 158,671 kB, slow 164,577 kB, against 176000. - Not shown: an attempt that fails and then passes on a retry. It depends on noise, and I didn't fake it. The code path is the same retry branch that the 3-failure run took. ## Review **Reviewer:** fresh-context `code-review` at high (review 302 on b1ae64d): 5 findings, all low; 4 fixed in df978f0, 1 declined (no parent TERM trap: CI cancels by container teardown, and a group kill reaches the child; comment 2283). No re-review (lows only). **Review-Confidence:** 85/100. The review was sound and no defect was missed. −15 because df978f0's fixes to low findings were never re-reviewed. ## Confidence by part - Retry wrapper (run.sh top): high. It is small, exit codes are checked, and it was exercised in the real image. - Baseline 176000: medium. It is a judgement call on 20 CI samples. With best-of-3, a +10 MB regression (the size of #71) would land at ~170 MB and pass. A human should decide if 10% over the median is still the right margin now that the gate is best-of-3. 170000 (~6%) would be the tighter option. - Root-cause analysis: medium. The steps line up with #71 and #82 across many runs, but no commit-level bisect was done (#71 was a 3-commit PR). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
mem-check: measure again on a baseline overrun, best of 3; baseline 176 MB (#96)
Some checks failed
CI / changes (pull_request) Successful in 10s
CI / rust (pull_request) Has been cancelled
CI / audit (pull_request) Has been cancelled
CI / app (pull_request) Has been cancelled
CI / e2e (pull_request) Has been cancelled
CI / click (pull_request) Has been cancelled
b1ae64d8a9
A shared runner only adds to PSS and start-up time, so one sample gated on
noise: two of the last three app runs failed by under 1 MB on trees that
had passed. An attempt over a baseline is now measured again in a fresh
app and sandbox, up to ATTEMPTS (3) times; failures that aren't a
baseline (exit 1, e2e_fail) aren't retried.

CI's peak grew with the first run (#71, +~10 MB) and the module manager
(#82, +~5 MB), not with the runner or its image (CI log history since
run 63). Starting the task runner only when needed saves nothing (A/B,
5 runs each, medians 168.7 vs 169.2 MB). Since #82 CI measures a median
of 160 MB (normal), so the baseline is 176000 kB, 10% over it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-review left a comment

Level: high (trunk: tools/mem-check/). Commit b1ae64d8a9. Range: origin/main...HEAD.

Files reviewed: tools/mem-check/run.sh, tools/mem-check/budget-kb, docs/09-quality-gates.md; read for context: tools/e2e/lib.sh (e2e_setup, e2e_cleanup, e2e_fail, exit 77 SKIP path), tools/ci/app.sh, Makefile.toml.

No high or medium findings. The retry loop is sound: exit 77 and 1 pass through, only 3 is retried, each attempt gets a fresh mktemp sandbox and EXIT-trap cleanup, and ATTEMPTS=1 keeps the old single-sample behaviour.

Low, outside the diff lines:

  • low · docs/01-pathway.md:49 · still says "budget 165 MB" while budget-kb is now 176000, so the doc disagrees with the gate.

Inline comments cover the other low items.

Level: high (trunk: tools/mem-check/). Commit b1ae64d8a9a887a5b09e757a25f2b3163586c0a8. Range: origin/main...HEAD. Files reviewed: tools/mem-check/run.sh, tools/mem-check/budget-kb, docs/09-quality-gates.md; read for context: tools/e2e/lib.sh (e2e_setup, e2e_cleanup, e2e_fail, exit 77 SKIP path), tools/ci/app.sh, Makefile.toml. No high or medium findings. The retry loop is sound: exit 77 and 1 pass through, only 3 is retried, each attempt gets a fresh mktemp sandbox and EXIT-trap cleanup, and ATTEMPTS=1 keeps the old single-sample behaviour. Low, outside the diff lines: - low · docs/01-pathway.md:49 · still says "budget 165 MB" while budget-kb is now 176000, so the doc disagrees with the gate. Inline comments cover the other low items.
@ -34,0 +43,4 @@
n=1
while :; do
rc=0
MEM_CHECK_ATTEMPT=$n sh "$0" "$@" || rc=$?
Collaborator

low: the parent shell has no TERM/INT trap, whereas the child (lib.sh) has trap 'exit 143' TERM INT. If CI cancels by signalling only the parent sh, the child keeps running (Xvfb, app, busy-loop) until it finishes, and nothing waits for it. Process-group kills are fine; a lone kill of leaks. Add trap 'kill $child' TERM INT with the child run in the background and waited on, or document.

low: the parent shell has no TERM/INT trap, whereas the child (lib.sh) has `trap 'exit 143' TERM INT`. If CI cancels by signalling only the parent sh, the child keeps running (Xvfb, app, busy-loop) until it finishes, and nothing waits for it. Process-group kills are fine; a lone kill of $$ leaks. Add `trap 'kill $child' TERM INT` with the child run in the background and waited on, or document.
@ -34,0 +45,4 @@
rc=0
MEM_CHECK_ATTEMPT=$n sh "$0" "$@" || rc=$?
[ "$rc" = 0 ] && exit 0
if [ "$rc" != 3 ]; then exit "$rc"; fi
Collaborator

low: exit 3 is the in-band 'retry' signal, but the child runs under set -e and sources lib.sh, so any helper or tool that happens to exit with status 3 (xdotool, timeout wrapper, taskset, a failing command in e2e_*) is read as 'over a baseline' and retried instead of failing at once. Unlikely today, but a distinct marker (a file in a dir the parent owns, or a rarely used code like 79) would be safer.

low: exit 3 is the in-band 'retry' signal, but the child runs under `set -e` and sources lib.sh, so any helper or tool that happens to exit with status 3 (xdotool, timeout wrapper, taskset, a failing command in e2e_*) is read as 'over a baseline' and retried instead of failing at once. Unlikely today, but a distinct marker (a file in a dir the parent owns, or a rarely used code like 79) would be safer.
@ -34,0 +46,4 @@
MEM_CHECK_ATTEMPT=$n sh "$0" "$@" || rc=$?
[ "$rc" = 0 ] && exit 0
if [ "$rc" != 3 ]; then exit "$rc"; fi
if [ "$n" -ge "$attempts" ]; then
Collaborator

low: ATTEMPTS=abc (or empty-but-set to a non-number) makes [ "$n" -ge "$attempts" ] return 2 inside if, which is false, so a persistently over-baseline attempt (rc 3) loops forever instead of failing. Validate ATTEMPTS as a positive integer, or test with [ "$n" -lt "$attempts" ] || fail-style logic that errors out.

low: ATTEMPTS=abc (or empty-but-set to a non-number) makes `[ "$n" -ge "$attempts" ]` return 2 inside `if`, which is false, so a persistently over-baseline attempt (rc 3) loops forever instead of failing. Validate ATTEMPTS as a positive integer, or test with `[ "$n" -lt "$attempts" ] || fail`-style logic that errors out.
@ -34,0 +47,4 @@
[ "$rc" = 0 ] && exit 0
if [ "$rc" != 3 ]; then exit "$rc"; fi
if [ "$n" -ge "$attempts" ]; then
echo "mem-check: FAIL: over a baseline in all $attempts attempts" >&2
Collaborator

low: the final FAIL line no longer says which baseline (PSS or start-up) or the numbers; they are only in the earlier per-attempt stderr lines, which are not prefixed FAIL and are not repeated, so a log tail or grep for FAIL shows no cause. Consider remembering the last attempt's reasons and repeating them here.

low: the final FAIL line no longer says which baseline (PSS or start-up) or the numbers; they are only in the earlier per-attempt stderr lines, which are not prefixed FAIL and are not repeated, so a log tail or grep for FAIL shows no cause. Consider remembering the last attempt's reasons and repeating them here.
mem-check: retry on exit 75 (EX_TEMPFAIL), not 3; check ATTEMPTS (review 302)
All checks were successful
CI / changes (pull_request) Successful in 5s
CI / rust (pull_request) Successful in 4m58s
CI / audit (pull_request) Successful in 11s
CI / app (pull_request) Successful in 2m57s
CI / e2e (pull_request) Successful in 7m49s
CI / click (pull_request) Successful in 1m58s
df978f034e
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Collaborator

Answers to review 302 (all low), fixed in df978f0 unless noted:

  • run.sh:49, ATTEMPTS not validated — fixed: anything but a whole number ≥ 1 exits 2 with a message (checked: ATTEMPTS=abc and ATTEMPTS=0 → rc 2).
  • run.sh:48, exit 3 as in-band signal — fixed: the retry code is now 75 (EX_TEMPFAIL, "try again"), which no tool in the path returns.
  • run.sh:46, no TERM/INT trap in the parent — not changed: CI cancels by tearing down the job container, and a terminal ^C / process-group kill reaches the child (which has lib.sh's exit 143 trap and cleans up). Only a signal sent to the parent's pid alone would leave the child running, and nothing does that.
  • run.sh:50, final FAIL line lacks the numbers — fixed: it says each attempt's numbers are in the lines above it (they're printed right above, per attempt).
  • docs/01-pathway.md:49, stale 165 MB — fixed: that line records the phase-0 result, so it keeps 165 and adds "176 MB since #96".

No re-review: only low findings (owner rule, features-via-workflow).

Answers to review 302 (all low), fixed in df978f0 unless noted: - **run.sh:49, ATTEMPTS not validated** — fixed: anything but a whole number ≥ 1 exits 2 with a message (checked: `ATTEMPTS=abc` and `ATTEMPTS=0` → rc 2). - **run.sh:48, exit 3 as in-band signal** — fixed: the retry code is now 75 (`EX_TEMPFAIL`, "try again"), which no tool in the path returns. - **run.sh:46, no TERM/INT trap in the parent** — not changed: CI cancels by tearing down the job container, and a terminal ^C / process-group kill reaches the child (which has lib.sh's `exit 143` trap and cleans up). Only a signal sent to the parent's pid alone would leave the child running, and nothing does that. - **run.sh:50, final FAIL line lacks the numbers** — fixed: it says each attempt's numbers are in the lines above it (they're printed right above, per attempt). - **docs/01-pathway.md:49, stale 165 MB** — fixed: that line records the phase-0 result, so it keeps 165 and adds "176 MB since #96". No re-review: only low findings (owner rule, features-via-workflow).
claude-bot changed title from WIP: mem-check: best of 3 on a baseline overrun; baseline 176 MB (#96) to mem-check: best of 3 on a baseline overrun; baseline 176 MB (#96) 2026-10-29 10:00:35 +00:00
claude-review left a comment

Gate approval at df978f0 (medium).

  1. Review 302 had 5 lows and no high or medium; nothing outstanding at that level.
  2. df978f0 (the only commit after b1ae64d) reads clean: ATTEMPTS validated (rc 2), retry on exit 75 only, e2e_fail still exits 1 so real failures are not retried, 75 is used nowhere else in tools/mem-check or tools/e2e/lib.sh, docs note matches the 176 MB baseline.
  3. CI is green on head: changes, rust, audit, app, e2e, click all success (run 195).
  4. Proof has no unrun gate for the maximum tier: CI's real image (app-docker, 3-failure path shown, exit 1), local fail and bad-argument paths, CI history of base failures, and CI on head passing attempt 1. The retry-then-pass case is honestly stated as not shown; it shares the retry branch that was exercised. Open judgement for the owner, not blocking: whether 176000 is the right margin now the gate is best-of-3 (170000 is the tighter option).
Gate approval at df978f0 (medium). 1. Review 302 had 5 lows and no high or medium; nothing outstanding at that level. 2. df978f0 (the only commit after b1ae64d) reads clean: ATTEMPTS validated (rc 2), retry on exit 75 only, e2e_fail still exits 1 so real failures are not retried, 75 is used nowhere else in tools/mem-check or tools/e2e/lib.sh, docs note matches the 176 MB baseline. 3. CI is green on head: changes, rust, audit, app, e2e, click all success (run 195). 4. Proof has no unrun gate for the maximum tier: CI's real image (app-docker, 3-failure path shown, exit 1), local fail and bad-argument paths, CI history of base failures, and CI on head passing attempt 1. The retry-then-pass case is honestly stated as not shown; it shares the retry branch that was exercised. Open judgement for the owner, not blocking: whether 176000 is the right margin now the gate is best-of-3 (170000 is the tighter option).
Author
Collaborator

Owner's call on the open question (2026-10-29): keep the baseline at 176000 under best-of-3; no change to 170000.

Owner's call on the open question (2026-10-29): keep the baseline at 176000 under best-of-3; no change to 170000.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
schaefera/utStudyBIble!111
No description provided.