Skip to content

fix(pm): head --report's hold bucket by the filter it actually applies - #12870

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-12823-hold-bucket-heading
Aug 28, 2026
Merged

fix(pm): head --report's hold bucket by the filter it actually applies#12870
os-elon merged 1 commit into
mainfrom
claude/issue-12823-hold-bucket-heading

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#12823

mode_report builds all three distributions from one loop with one shared filter:

awk -v f="$field=" '{ ... if (v ~ /^[0-9]+$/ && v > 0) print v; ... }'

That filter fits the waited heading exactly — a recorded waited of 0 is "did not wait
at all". It does not fit an acquisition. held is written by every terminal outcome, so
held=0 names two disjoint things at once: a run that never acquired, and a run that acquired
and let go inside one clock tick. The heading claimed the bucket held the second kind while
the filter drops it.

The direction is the awkward one: the dropped rows are the fastest holds, so p50 and
p90 read high — and read higher the better the fleet gets at holding the lock briefly.

Before / after

Both rendered over the same six-row fixture ledger. Every number is identical.

lock hold, over runs that acquired (seconds):
n=3 p50=63 p90=931 max=931
lock hold, over runs whose RECORDED hold is 1s or more (seconds):
n=3 p50=63 p90=931 max=931
⇒ this n is BELOW the record count at the top, and the gap is not rounding:
every row recording held=0 is absent. That is the runs that never acquired
AND the runs that acquired and let go inside one clock tick, so the fastest
holds are the missing ones and p50/p90 read high by exactly that omission.
⇒ 1 is not a claim that a full second was held: the field is a difference of
whole-second clock reads, so a sub-second hold records 0 or 1 depending only
on where it fell against the tick.
⇒ an `unlocked` run is IN this bucket and held nothing -- on a host with no
usable flock the command runs unserialized and its RUNTIME lands in this
field. Read the outcomes block above before reading these as contention.

That fixture is worth reading twice: records: 6 against n=3, and the p50=63 is the
unlocked row — a run on which the lock was never taken. Two rows that did acquire
(one after waiting 78s) are absent. The old heading described none of that.

⛔ The filter is NOT changed, and that is the ruling, not a preference

Admitting held = 0 would move n, p50 and p90 for every ledger, past and future: a
figure quoted from an older report would stop matching a re-run over the very same rows, with
nothing in either report saying why. That is a change of definition, not a repair, and it
is carved out to its own card (filed; linked from the report comment). This branch changes no
number the report prints.

waited's heading is untouched — and pinned untouched

over runs that waited at all is already true of its filter. The likeliest way to get this
repair wrong is to make the three headings "consistent" and break the one that was already
right, so a self-test case asserts that heading is still present, not merely that the new
one is.

Two measurements that changed the wording the card floated

The card suggested "over runs that held it for at least a second". Reasonable, and
measurably false — so it is not what shipped:

  1. held is a difference of two whole-second clock reads.held=$(($(now_s) - acquired_at)), and now_s is EPOCHSECONDS or date +%s. A 200ms hold straddling a tick
    records 1; a 900ms hold inside one records 0. Promoting the recorded value to a
    claim about true duration would rebuild this card's own defect one line further down. The
    heading says RECORDED, and a line says what that is worth.
  2. run_unlocked writes ledger_append unlocked 0 "$ran" ... — on a host with no usable
    flock the command's runtime lands in the held field although the lock was never
    taken. So the bucket is wrong in the other direction too: it admits rows that did not
    acquire at all. Annotated here, because removing them is a filter change and belongs with
    the carve-out above.

Tests

bash scripts/pm/os-verify-lock.sh --self-test186 cases, 0 failures, run under the
shared verify lock on final head 193340a3 (baseline before this branch: 180).

Ablation: the six new cases were shown able to fail, and the direction was predicted
before the run. Mutation = the new heading reverted to the old string and the three
blocks deleted, leaving the self-test cases in place.

predictedobserved
the four presence pins go red✗ ✗ ✗ ✗
the "old heading gone" pin goes red
the waited pin stays green — it was already true pre-fix✓ (stayed green)

✗ os-verify-lock self-test: 5 case(s) failed. Exactly five, exactly the predicted five.

There is no build leg to state: this is a shell script executed from source, with no dist/
between the edit and the instrument. The mutation was proved to have reached the executed
surface by reading the mutated --report output itself (it printed the old heading), and the
restore was proved by blob hash equal to HEAD plus an empty git diff HEAD — not by an exit
code. An earlier ablation attempt died mid-mutation on a quoting error in the harness; its
trap ... EXIT restored the tree, verified the same two ways before continuing.

Gates — the family was re-derived from the real change set with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack after the final commit, and it added nothing. All green,
each read from its own printed verdict line rather than a bare $?:

check:agent-test-spelling · check:bash32-floor · check:cli-command-ids
check:cross-package-test-inputs · check:entry-guard · check:parse-guard
check:pnpm-filter-targets · check:watch-hint-literal · check:nul-bytes
node scripts/check-ci-filter-parity.mjs · node scripts/check-cross-package-test-inputs.mjs

check:bash32-floor: 22 tracked shell file(s) ... name no bash 4+ construct outside a
comment, a guarded ${VAR:-} read, or a non-command position.

pnpm lint was not narrowed — its population and this diff are disjoint, measured.
isPathIgnored('scripts/pm/os-verify-lock.sh') returns true and calculateConfigForFile
returns nothing for it, while the positive control scripts/check-nul-bytes.mjs returns a
real config — so the instrument can return both answers. eslint.config.mjs declares file
globs only for {ts,tsx,mts,cts,js,jsx,mjs,cjs} and mentions .sh zero times, and it enables
no type-aware linting anywhere (its own comment at line 328: no parserOptions.project, no
typed rules, for ANY file
), so no untouched file's verdict can move as a function of this
diff.

No changeset

Root scripts/ tooling publishes nothing. Consistent with the last twelve commits touching
scripts/pm/, which carry zero changesets between them — including PR #12827, the
directly comparable repair to this same function, which carried skip-changeset. An
empty-frontmatter changeset is not the alternative; check-empty-changeset.mjs reds on it.

Scope

The heading half only, per the ruling on the card. Related work, not addressed here and
still open: #12828 (the /tmp ephemerality source comment) and #12795 (routing gate runs
through the lock). #12782 and #12783 are the two sibling repairs to this same function whose
pattern this one follows.


Generated by Claude Code

#12823)
`mode_report` builds all three distributions with one shared filter,
`v > 0`. That filter fits the `waited` heading exactly -- a recorded
`waited` of 0 IS "did not wait at all" -- and it does not fit an
acquisition. `held` is written by every terminal outcome, so `held=0`
names two disjoint things: a run that never acquired at all, and a run
that acquired and released inside one clock tick. The heading claimed
the bucket held the second kind while the filter drops it -- measured,
3 of 69 rows on the ledger this was filed from, one of which had waited
78s for the lock before holding it briefly.
The dropped rows are the FASTEST holds, so p50 and p90 read high, and
they read higher the better the fleet gets at holding the lock briefly.
Presentation only. The filter is NOT changed: admitting held=0 would
move n, p50 and p90 for every ledger past and future, so a figure quoted
from an older report would stop matching a re-run over the same rows.
That is a change of definition and is carved out to its own card. Every
number this report prints is unchanged by this commit.
Two things measured while writing it, which shaped the wording:
- `held` is a difference of two whole-second clock reads (`now_s`), so
the obvious spelling -- "over runs that held it for at least a
second" -- is itself false: a 200ms hold straddling a tick records 1
and a 900ms hold inside one records 0. The heading says RECORDED.
- `run_unlocked` writes `ledger_append unlocked 0 "$ran" ...`, so on a
host with no usable flock the command's RUNTIME lands in the `held`
field although the lock was never taken. Those rows are IN this
bucket. Annotated, not filtered out.
The `n` gap the report never explained now has a line of its own, and
`waited`'s heading -- already true of its filter -- is untouched, with a
self-test case pinning it present so the likeliest wrong repair (making
the three headings "consistent") reds.
Six --self-test cases: the new heading present, the old one GONE rather
than annotated, the three explanatory lines, and the `waited` heading
still there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CPrUz21stTFhJRUirdc4yw
@os-elonos-elon added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed size/s labels Aug 28, 2026 — with Claude
@os-elon
os-elon marked this pull request as ready for review August 28, 2026 06:34
@os-elon
os-elon enabled auto-merge August 28, 2026 06:34
@os-elon
os-elon added this pull request to the merge queueAug 28, 2026
Merged via the queue into main with commit 4a7e1b0Aug 28, 2026
37 checks passed
@os-elon
os-elon deleted the claude/issue-12823-hold-bucket-heading branch August 28, 2026 06:50
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] os-verify-lock --report's hold bucket says "over runs that acquired" but its filter drops every sub-second hold — measured, 3 of 69 rows

2 participants

@os-elon@claude