Skip to content

fix(bin): fold OPEN DECISIONS in one awk pass for a full re-fold - #2836

Open
armchairo wants to merge 1 commit into
kunchenguid:mainfrom
armchairo:fm/drain-fold-perf
Open

fix(bin): fold OPEN DECISIONS in one awk pass for a full re-fold#2836
armchairo wants to merge 1 commit into
kunchenguid:mainfrom
armchairo:fm/drain-fold-perf

Conversation

@armchairo

Copy link
Copy Markdown

Fixes #2808.

Problem

status_open_decisions and the cursor-invalidated full-refold branch of status_open_decisions_incremental (bin/fm-classify-lib.sh) rebuild a task's open-decision set by looping _fm_decision_fold_line once per status line in bash: several subshells per line (status_line_verb, _fm_decision_key and its helpers, status_line_note) plus an O(open-count) _fm_decision_drop rescan on every open/close.

Incremental cursors (#1737) bound the steady-state cost to newly appended bytes, but any cursor miss, fold-version mismatch, or shrink still re-folds the whole file from byte 0 through that same bash loop, which is CPU-bound-shell-slow on a large status log.

Fix

_fm_decision_fold_awk re-derives the exact same fold rule as one awk pass: both key positions ([key=x] before the colon or at the head of the note), the drop-then-append ordering, the reserved pending-reply- key-prefix namespace check, and the default one-open-per-task key. status_open_decisions and the full-refold branch of status_open_decisions_incremental (identified by the chunk starting at byte 0) now call it instead of looping _fm_decision_fold_line. The incremental steady-state branch is unchanged and keeps the bash loop, where per-call overhead already dwarfs a handful of newly appended lines. #2801's wake-context-packet work is untouched; this PR is fold cost only.

Equivalence

tests/fm-classify-decision-fold-awk-equivalence.test.sh drives both a bash reference fold (built from the still-live _fm_decision_fold_line) and the new awk engine over the same fixture logs - an empty file, blank/whitespace-only lines, no-colon legacy lines, multiple [key=...] tags on one line, a file with no trailing newline, and a generated ~20KB fixture exercising every decision-line shape - and asserts byte-identical output. The existing tests/fm-classify-decision-key.test.sh and tests/fm-wake-drain-open-decisions(-cursor).test.sh suites now exercise the awk engine through status_open_decisions/status_open_decisions_incremental and pass unchanged.

Benchmark

Generated a ~330KB fixture shaped like the reported scenario (mostly routine working: lines, an occasional needs-decision/resolved cycle, ~5,200 lines, 104 decisions still open at the end) and timed status_open_decisions before and after this change, sourcing each version of bin/fm-classify-lib.sh standalone:

$ wc -c fixture-realistic-330k.status
  330053 fixture-realistic-330k.status
$ wc -l fixture-realistic-330k.status
    5199 fixture-realistic-330k.status

# before (git show <base-sha>:bin/fm-classify-lib.sh, unmodified bash loop)
$ bash -c '
. fm-classify-lib-before.sh
start=$(perl -MTime::HiRes=time -e "print time")
out=$(status_open_decisions fixture-realistic-330k.status)
end=$(perl -MTime::HiRes=time -e "print time")
printf "BEFORE (bash) wall time: %.3fs\n" "$(echo "$end - $start" | bc)"
'
BEFORE (bash) wall time: 24.694s

# after (this branch's awk fold)
$ bash -c '
. fm-classify-lib-after.sh
start=$(perl -MTime::HiRes=time -e "print time")
out=$(status_open_decisions fixture-realistic-330k.status)
end=$(perl -MTime::HiRes=time -e "print time")
printf "AFTER (awk) wall time: %.3fs\n" "$(echo "$end - $start" | bc)"
'
AFTER (awk) wall time: 0.024s   # repeated: 0.032s, 0.029s, 0.024s

# byte-identical output, confirmed with a hash rather than eyeballing a large fold
$ shasum -a 256 before-realistic-output.txt after-realistic-output-clean.txt
a69e8909bbef7fa923ea9b2f78663e48832c2711582488708365b38dab678556  before-realistic-output.txt
a69e8909bbef7fa923ea9b2f78663e48832c2711582488708365b38dab678556  after-realistic-output-clean.txt

~1,000x faster on this fixture (24.7s -> 0.03s), same output.

Verification

$ bash tests/fm-classify-decision-fold-awk-equivalence.test.sh
ok - generated 20623-byte fixture (same generator as the #2808-scale PR benchmark): awk engine matches the bash reference fold byte for byte
ok - empty status file: awk engine matches the bash reference fold byte for byte
ok - blank and whitespace-only lines interleaved: awk engine matches the bash reference fold byte for byte
ok - legacy colon-free free-text lines: awk engine matches the bash reference fold byte for byte
ok - two [key=...] tags before the colon on one line: awk engine matches the bash reference fold byte for byte
ok - final line has no trailing newline: awk engine matches the bash reference fold byte for byte

$ bash tests/fm-classify-decision-key.test.sh
ok - a stated [key=X] opens X whether it precedes or follows the verb colon
ok - a keyless needs-decision still opens and closes the default key
ok - a resolution closes its decision regardless of either line's key position
ok - blocked [key=X] opens X in both key positions
ok - two colon-form decisions stay distinct
ok - a [key=x] mentioned mid-note is prose, never an opened or closed key
ok - a malformed stated key is rejected in both positions, never folded as default
ok - status_line_verb strips every bracket tag before the colon, in any order, and recovers the bare verb
ok - a [corr=...] tag ahead of [key=...] no longer swallows the verb: opens and closes under the stated key
ok - a [corr=...] tag with no stated key opens under 'default', exactly like a bare needs-decision line
ok - a [key=x] tag alone (no corr tag) still opens x - no regression from the tag-stripping fix
ok - blocked/resolved parse their bare verb with any bracket-tag order preceding the colon
ok - the incremental fold matches the full fold across appends in both key positions
ok - status_key_closing_verb separates resolution, durable transfer, and still-open
ok - status_key_closing_verb reports the last real transition, in either key position

$ bash tests/fm-wake-drain-open-decisions.test.sh && bash tests/fm-wake-drain-open-decisions-cursor.test.sh
# all ok, including the truncation/rotation/read-failure full-refold cases

$ bin/fm-lint.sh
fm-lint.sh: ShellCheck 0.11.0 (pinned 0.11.0)   # clean

status_open_decisions and the cursor-invalidated full-refold branch of
status_open_decisions_incremental (bin/fm-classify-lib.sh) rebuilt a
task's open-decision set by looping _fm_decision_fold_line per line in
bash: several subshells per line plus an O(open-count) drop rescan on
every open/close.

Incremental cursors (kunchenguid#1737) bound the steady-state cost to new
appends, but any cursor miss, fold-version mismatch, or shrink still
re-folds the whole file from byte 0 through that same bash loop. On a
~330KB status log this is minutes of CPU-bound shell, stalling every
wake-handling turn (kunchenguid#2808).

_fm_decision_fold_awk re-derives the exact same rule (both key
positions, the drop rules, the reserved-key-prefix namespace check,
and the "default" one-open-per-task key) as one awk pass. Both full
folds now call it; the incremental steady-state branch keeps the bash
loop, where per-call overhead already dwarfs a handful of new lines.

tests/fm-classify-decision-fold-awk-equivalence.test.sh drives both
engines over the same fixture logs, including a generated multi-KB
one, and asserts byte-identical output. The existing
tests/fm-classify-decision-key.test.sh and
tests/fm-wake-drain-open-decisions(-cursor).test.sh suites now
exercise the awk engine through status_open_decisions and
status_open_decisions_incremental and still pass unchanged.

Fixes kunchenguid#2808
@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge with no concrete changed-code failure identified.

The new full-fold implementation matches the existing parsing, key validation, reserved-prefix handling, transition, and ordering behavior for supported configurations, while reachable incremental cursor states preserve correct byte-zero refolding.

Reviews (1): Last reviewed commit: "fix(bin): fold OPEN DECISIONS in one awk..." | Re-trigger Greptile

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Scheduled 3:10pm PT 8/23 pass. VISION.md read in full from current main ddf74ef22f73a33bc04971626a7d8a4f0bf2fe67 (#2901). Reconfirmed. Issue #2808 is ready-for-pr; that is a queue label, not a merge vote. No captain comment authorizing a merge. First stamp of this PR (unstamped prior passes).

VISION (inspected _fm_decision_fold_awk in bin/fm-classify-lib.sh and tests/fm-classify-decision-fold-awk-equivalence.test.sh). Per-rule: scripts own the mechanics aligns (one awk pass re-derives the live bash fold rule; incremental steady-state stays bash); token/time efficiency aligns (reported ~1000x on a ~330KB status log; drain no longer CPU-stalls a wake turn); honest interface aligns (equivalence tests assert byte-identical OPEN DECISIONS vs _fm_decision_fold_line); restart/peace of mind aligns (a large-log re-fold stops being a multi-minute outage). New capability as opt-in is N/A — this is the same fold, faster.

Class: corrective (performance rewrite of existing fold semantics, not a default-behavior change of which decisions surface).

Security: none. Diff reviewed in full before first-time-fork approval: only bin/fm-classify-lib.sh and the equivalence test. Awk parser of a local status file; no workflow files, no secrets, no network, no untrusted-input execution. Safe.

Overlap / HOLD: same-file overlap with open #2877 / #2867 / #2801 on bin/fm-classify-lib.sh. Not a standing spawn/teardown/herdr hold. Not a covering PR for #2808.

CI / NM: HEAD ce48753603ee24430dc2817620d0918f5f6ee85c. MERGEABLE / UNSTABLE, ahead 1 / behind 12 vs current main. Body has no no-mistakes-pipeline-attestation:v1. First-time fork from armchairo/firstmate-upstream (this author's only PR on the repo). Workflows were action_required; after diff review they were approved this pass. CI run 32624507887 is now in_progress. Require no-mistakes run 32624507860 was approved this pass; the body still cannot attest this HEAD.

Workflows: first-time fork, approved this pass after diff review. Run IDs: 32624507887 (CI, in_progress), 32624507860 (Require no-mistakes, approved).

Land-eligible rec: NO (CI not green yet; no NM attestation; 12 behind; classify-lib overlap with #2877). Captain-flag NOW: no — waiting on CI and on the author for a no-mistakes pipeline that stamps THIS HEAD.

Waiting on CI (just approved) and on the author for a HEAD-matching no-mistakes attestation. Not a captain-decision hold.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fm-wake-drain: OPEN DECISIONS fold becomes multi-minute on large status logs

2 participants