Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
armchairo wants to merge 1 commit into
base: main
Choose a base branch
from
+297 −8
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 132 additions & 8 deletions bin/fm-classify-lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,121 @@ _fm_decision_fold_line() { # <open-set> <status-line> <resolve-verb> <held-verb
printf '%s' "$open"
}

# One-awk-process re-derivation of _fm_decision_fold_line, for a FULL fold of
# an entire status file from byte 0. _fm_decision_fold_line's own per-line
# bash loop forks several subshells per line (status_line_verb,
# _fm_decision_key and its _fm_key_before_colon/_fm_key_at_note_head/
# _fm_decision_slug_ok helpers, _fm_decision_key_transition_allowed,
# status_line_note) plus an O(open-count) _fm_decision_drop rescan on every
# open/close - cheap per call, but CPU-bound-shell-slow summed across a
# few-hundred-KB status log (issue #2808). status_open_decisions and the
# cursor-invalidated full-refold branch of status_open_decisions_incremental
# below both call this instead of looping _fm_decision_fold_line themselves;
# the steady-state incremental branch keeps the bash loop, where per-call
# overhead already dwarfs the handful of newly appended lines.
#
# This function's rule MUST stay byte-identical to _fm_decision_fold_line
# (verbs, both key positions, the drop rules, the reserved-key-prefix
# namespace check, and the "default" one-open-per-task key) - it is a second
# implementation of the one contract that function documents, not a second
# owner of it. tests/fm-classify-decision-fold-awk-equivalence.test.sh drives
# both engines over the same fixture logs, including a generated ~330KB one,
# and asserts byte-identical output; re-run it after touching either engine.
_fm_decision_fold_awk() { # <status-file> <resolve-verb> <held-verb>
local f=$1 resolve=$2 held=$3
LC_ALL=C awk -v resolve="$resolve" -v held="$held" \
-v reserved="${FM_CLASSIFY_RESERVED_KEY_PREFIXES:-$FM_CLASSIFY_RESERVED_KEY_PREFIXES_DEFAULT}" '
function ltrim(s) { sub(/^[[:space:]]+/, "", s); return s }
function rtrim(s) { sub(/[[:space:]]+$/, "", s); return s }
function trim(s) { return ltrim(rtrim(s)) }
BEGIN {
seq = 0
n_reserved = split(reserved, rprefix, /[[:space:]]+/)
}
{
line = $0
blank = line
gsub(/[[:space:]]/, "", blank)
if (blank == "") next

colon = index(line, ":")
if (colon > 0) {
precolon = substr(line, 1, colon - 1)
postcolon = substr(line, colon + 1)
has_colon = 1
} else {
precolon = line
postcolon = ""
has_colon = 0
}

br = index(precolon, "[")
verb = trim(br > 0 ? substr(precolon, 1, br - 1) : precolon)

before_colon_matched = 0
kpos = index(precolon, "[key=")
if (kpos > 0) {
after = substr(precolon, kpos + 5)
bpos = index(after, "]")
if (bpos > 0) { before_colon_matched = 1; key = substr(after, 1, bpos - 1) }
}
note_head_matched = 0
if (!before_colon_matched) {
if (has_colon) {
rest = ltrim(postcolon)
if (substr(rest, 1, 5) == "[key=") {
after2 = substr(rest, 6)
bpos2 = index(after2, "]")
if (bpos2 > 0) { note_head_matched = 1; key = substr(after2, 1, bpos2 - 1) }
}
}
if (!note_head_matched) key = "default"
}
if (key !~ /^[A-Za-z0-9._-]+$/) next

if (has_colon) {
note = ltrim(postcolon)
if (note_head_matched) {
token = "[key=" key "]"
if (substr(note, 1, length(token)) == token) note = ltrim(substr(note, length(token) + 1))
}
} else {
note = line
}

allowed = 1
for (ri = 1; ri <= n_reserved; ri++) {
p = rprefix[ri]
if (p == "" || substr(key, 1, length(p)) != p) continue
allowed = (substr(note, 1, length(p)) == p && index(substr(note, length(p) + 1), ":") > 0) ? 1 : 0
break
}
if (!allowed) next

if (verb == "needs-decision" || verb == "blocked") {
seq++
openset[key] = verb "\t" note
seqof[key] = seq
isopen[key] = 1
} else if (verb == resolve || verb == held) {
delete openset[key]
delete seqof[key]
delete isopen[key]
}
}
END {
m = 0
for (k in isopen) { m++; klist[m] = k }
for (i = 2; i <= m; i++) {
kk = klist[i]; sv = seqof[kk]; j = i - 1
while (j >= 1 && seqof[klist[j]] > sv) { klist[j + 1] = klist[j]; j-- }
klist[j + 1] = kk
}
for (i = 1; i <= m; i++) printf "%s\t%s\n", klist[i], openset[klist[i]]
}
' "$f"
}

# Fold the WHOLE status stream into the set of decisions still open. Prints one
# TAB-separated "<key>\t<verb>\t<summary>" line per still-open decision, in
# most-recently-opened-last order; prints nothing when none are open. Pure read of
Expand All @@ -367,15 +482,14 @@ _fm_decision_fold_line() { # <open-set> <status-line> <resolve-verb> <held-verb
# before any read - a cheap builtin, unlike fm_wake_latest_event's O_NOFOLLOW
# subprocess read, which exists for that function's much narrower payload-driven
# path resolution rather than this directory-local glob.
# The whole-file fold is always a fold from byte 0, so it always goes through
# _fm_decision_fold_awk above rather than looping _fm_decision_fold_line itself.
status_open_decisions() { # <status-file>
local f=$1 line resolve held open=''
local f=$1 resolve held
[ -f "$f" ] && [ -r "$f" ] && [ ! -L "$f" ] || return 0
resolve=${FM_CLASSIFY_RESOLVE_VERB:-$FM_CLASSIFY_RESOLVE_VERB_DEFAULT}
held=${FM_CLASSIFY_CAPTAIN_HELD_VERB:-$FM_CLASSIFY_CAPTAIN_HELD_VERB_DEFAULT}
while IFS= read -r line || [ -n "$line" ]; do
open=$(_fm_decision_fold_line "$open" "$line" "$resolve" "$held")
done < "$f"
printf '%s' "$open"
_fm_decision_fold_awk "$f" "$resolve" "$held"
}

# 0 when <key> has a record in a folded "<key>\t<verb>\t<note>" open set.
Expand Down Expand Up @@ -667,9 +781,19 @@ status_open_decisions_incremental() { # <status-file> [<captured-end-offset>]
&& printf '%s\t%s\n' "$f" "$chunk_size" >> "$FM_OPEN_DECISIONS_READ_PROBE"
resolve=${FM_CLASSIFY_RESOLVE_VERB:-$FM_CLASSIFY_RESOLVE_VERB_DEFAULT}
held=${FM_CLASSIFY_CAPTAIN_HELD_VERB:-$FM_CLASSIFY_CAPTAIN_HELD_VERB_DEFAULT}
while IFS= read -r line || [ -n "$line" ]; do
open=$(_fm_decision_fold_line "$open" "$line" "$resolve" "$held")
done < "$chunk_file"
if [ "$offset" -eq 0 ]; then
# This chunk starts at byte 0, so it IS the full-file fold (a fresh
# cursor, or the invalidation above) - the CPU-bound-shell case issue
# #2808 reports. Route it through the one-pass awk fold instead of
# looping _fm_decision_fold_line; any prior $open is already empty here
# (set '' by the invalidation branch, or never populated on a first
# call), so there is nothing to fold onto.
open=$(_fm_decision_fold_awk "$chunk_file" "$resolve" "$held")
else
while IFS= read -r line || [ -n "$line" ]; do
open=$(_fm_decision_fold_line "$open" "$line" "$resolve" "$held")
done < "$chunk_file"
fi
rm -f "$chunk_file"
offset=$size
cursor_dirty=1
Expand Down
165 changes: 165 additions & 0 deletions tests/fm-classify-decision-fold-awk-equivalence.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
#!/usr/bin/env bash
# tests/fm-classify-decision-fold-awk-equivalence.test.sh - equivalence tests
# for _fm_decision_fold_awk (bin/fm-classify-lib.sh), the one-pass awk
# re-derivation of the per-line bash fold that status_open_decisions and the
# cursor-invalidated full-refold branch of status_open_decisions_incremental
# now call for a full fold from byte 0 (issue #2808: the bash while-read loop
# over _fm_decision_fold_line is CPU-bound-shell-slow across a large status
# log). tests/fm-classify-decision-key.test.sh already pins many exact
# open-set values through status_open_decisions, so it now exercises the awk
# engine too and would catch a semantic regression there. This file adds the
# proof the awk rewrite itself demands: the awk engine and a bash engine built
# from the SAME still-live _fm_decision_fold_line rule must agree byte for
# byte, including at the ~330KB scale the issue reports, not just on the
# small hand-pinned cases.
set -u

# shellcheck source=tests/lib.sh
. "$(dirname "${BASH_SOURCE[0]}")/lib.sh"

# shellcheck source=bin/fm-classify-lib.sh
. "$ROOT/bin/fm-classify-lib.sh"

TMP_ROOT=$(fm_test_tmproot fm-classify-decision-fold-awk-equivalence-tests)

case_dir() { # <name>
local d="$TMP_ROOT/$1"
mkdir -p "$d"
printf '%s' "$d"
}

# The pre-awk whole-file fold, rebuilt here from the SAME _fm_decision_fold_line
# rule status_open_decisions itself used before this fix (and still uses for
# the incremental steady-state chunk fold, and status_key_closing_verb). This
# is the bash-engine half of the equivalence proof, not a frozen duplicate of
# removed logic - it calls the one live implementation of the fold rule.
bash_reference_fold() { # <status-file>
local f=$1 line resolve held open=''
resolve=${FM_CLASSIFY_RESOLVE_VERB:-$FM_CLASSIFY_RESOLVE_VERB_DEFAULT}
held=${FM_CLASSIFY_CAPTAIN_HELD_VERB:-$FM_CLASSIFY_CAPTAIN_HELD_VERB_DEFAULT}
while IFS= read -r line || [ -n "$line" ]; do
open=$(_fm_decision_fold_line "$open" "$line" "$resolve" "$held")
done < "$f"
printf '%s' "$open"
}

# Assert the awk engine (status_open_decisions, and the incremental fold's
# first-ever/cursor-invalidated call, both of which route through
# _fm_decision_fold_awk) agrees byte for byte with the bash reference fold.
assert_engines_agree() { # <status-file> <label>
local f=$1 label=$2 want got incr
want=$(bash_reference_fold "$f")
got=$(status_open_decisions "$f")
[ "$got" = "$want" ] \
|| fail "$label: awk fold diverged from the bash reference fold: got '$got' want '$want'"
incr=$(status_open_decisions_incremental "$f")
[ "$incr" = "$want" ] \
|| fail "$label: incremental full-refold diverged from the bash reference fold: got '$incr' want '$want'"
pass "$label: awk engine matches the bash reference fold byte for byte"
}

# Deterministic (no $RANDOM) ~<min_bytes>-byte fixture at <path>: a realistic
# mix of routine filler plus every decision-line shape the fold must treat
# identically - both key positions, corr+key tag combinations, mid-note prose
# mentions, malformed slugs, reserved pending-reply- keys under both the
# rejected and the correct vocabulary, and repeated open/close/reopen cycles
# (round-robin close on needs-decision/blocked/captain-held so the final open
# set is large but not the whole file). Reproducible byte for byte on every
# run, so a benchmark against it is reproducible too.
gen_fixture() { # <path> <min-bytes>
local path=$1 min_bytes=$2 i=0 size=0
: > "$path"
while [ "$size" -lt "$min_bytes" ]; do
i=$((i + 1))
{
printf 'working: routine progress note number %d in the fixture log\n' "$i"
printf 'needs-decision [key=topic-%d]: pick an option for round %d\n' "$i" "$i"
printf 'working: still thinking, mentions [key=topic-%d] as prose only\n' "$i" "$i"
printf 'blocked: [key=creds-%d] waiting on a credential for round %d\n' "$i" "$i"
printf 'needs-decision [corr=corr%06d] [key=combo-%d]: a corr-tagged decision\n' "$i" "$i"
printf 'needs-decision [key=bad key %d]: malformed slug never opens\n' "$i"
printf 'blocked [key=pending-reply-%d]: not the reserved vocabulary, rejected\n' "$i"
printf 'blocked [key=pending-reply-%d]: pending-reply-%d: correct vocabulary opens\n' "$i" "$i"
printf 'working: more routine filler padding out the log to size, round %d\n' "$i"
if [ $((i % 3)) -eq 0 ]; then
printf 'resolved [key=topic-%d]: answered round %d\n' "$i" "$i"
fi
if [ $((i % 5)) -eq 0 ]; then
printf 'captain-held: [key=creds-%d] handed to a durable captain-held task\n' "$i"
fi
if [ $((i % 7)) -eq 0 ]; then
printf 'resolved [corr=corr%06d] [key=combo-%d]: closed the corr-tagged one\n' "$i" "$i"
fi
if [ $((i % 11)) -eq 0 ]; then
printf 'resolved [key=pending-reply-%d]: pending-reply-%d: closed correctly\n' "$i" "$i"
fi
} >> "$path"
size=$(LC_ALL=C wc -c < "$path" | tr -d '[:space:]')
done
}

# The bash reference engine is exactly the per-line, per-subshell loop issue
# #2808 reports as CPU-bound-shell-slow at ~330KB, so pinning the committed
# suite's fixture at that full scale would make an ordinary test run take
# minutes. This case proves equivalence at a scale that still exercises
# hundreds of concurrently open decisions (multiple keys reopened, closed, and
# left open across every shape gen_fixture produces) while staying CI-fast;
# the PR evidence separately runs the SAME generator at the full ~330KB the
# issue reports and diffs both engines' output there too, alongside the
# before/after wall-clock benchmark.
test_generated_fixture_matches() {
local dir f bytes
dir=$(case_dir large-fixture)
f="$dir/task.status"
gen_fixture "$f" 20000
bytes=$(LC_ALL=C wc -c < "$f" | tr -d '[:space:]')
[ "$bytes" -ge 20000 ] || fail "fixture generator undershot its target: $bytes bytes"
assert_engines_agree "$f" "generated ${bytes}-byte fixture (same generator as the #2808-scale PR benchmark)"
}

test_empty_file_matches() {
local dir f
dir=$(case_dir empty)
f="$dir/task.status"
: > "$f"
assert_engines_agree "$f" "empty status file"
}

test_blank_and_whitespace_only_lines_matches() {
local dir f
dir=$(case_dir blank-lines)
f="$dir/task.status"
printf 'needs-decision [key=a]: first\n\n \n\t\nworking: filler\nresolved [key=a]: done\n' > "$f"
assert_engines_agree "$f" "blank and whitespace-only lines interleaved"
}

test_no_colon_lines_matches() {
local dir f
dir=$(case_dir no-colon)
f="$dir/task.status"
printf 'PR ready\nmerged\nneeds-decision [key=b]: pick one\nchecks green\n' > "$f"
assert_engines_agree "$f" "legacy colon-free free-text lines"
}

test_multiple_key_tags_on_one_line_matches() {
local dir f
dir=$(case_dir multi-key-tag)
f="$dir/task.status"
printf 'needs-decision [key=first] [key=second]: only the first tag should count\n' > "$f"
assert_engines_agree "$f" "two [key=...] tags before the colon on one line"
}

test_file_without_trailing_newline_matches() {
local dir f
dir=$(case_dir no-trailing-newline)
f="$dir/task.status"
printf 'needs-decision [key=x]: opened\nworking: filler' > "$f"
assert_engines_agree "$f" "final line has no trailing newline"
}

test_generated_fixture_matches
test_empty_file_matches
test_blank_and_whitespace_only_lines_matches
test_no_colon_lines_matches
test_multiple_key_tags_on_one_line_matches
test_file_without_trailing_newline_matches
Loading