Repository navigation
refactor: read the report's marker and sentinel in one lib instead of copies in six scripts - #160
Merged
Merged
Conversation
… copies in six scripts bin/report-format.sh gains report_marker_count (strict), report_has_marker (loose), report_has_sentinel, report_body, report_after_sentinel and report_pin_lines. notify.sh, question-streaks.sh, triage-dream.sh and run.sh call them instead of keeping their own sed, grep and awk. The strict and loose marker checks stay different on purpose and are now documented as such. run.sh stops at startup if the lib is missing. tests/report-format.sh compares each helper with the replaced code, kept as a reference implementation, over 16 fixtures for output and exit status (119 assertions), and fails if another script keeps an inline copy. Full suite: 2819 passed, 9 failed = the 4 sandbox-only baseline failures plus 5 in test_fanout_stats, which fail only because the machine timezone is HKT (the fixture's 18:05Z falls on the next local day) and pass with TZ=America/New_York.
… test prove its claims Review of acc9245: a missing lib would let question-streaks.sh read every report as marker-less and freeze the streaks behind a misleading message (N1); the leftover-copy scan caught only the exact sed and awk spellings and the sources check passed on any mention of the file name (N2); the test's own EXIT trap replaced the one suite_tmp installs and left an empty temp dir per run (N3, the leak issue 63 is about). notify.sh exits 1 and question-streaks.sh exits 1 (0 for update, as its header promises) with a message naming the lib. The test now requires each caller to call its helper, scans for any grep, sed or awk read of the marker or sentinel outside the lib, and pins both guards. Swept the other suites that use suite_tmp for the same trap override: none.
3 of 6 tasks
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.
Summary
The two contracts a report carries, the open-questions marker (
<!-- autodream:open-questions=N -->) and L2'sAUTODREAM_REPORT_ENDsentinel, were read by inline sed, grep and awk copies in six scripts. They are now six helpers inbin/report-format.sh:report_marker_count,report_has_marker,report_has_sentinel,report_body,report_after_sentinel,report_pin_lines.run.sh(7 sites),notify.sh,question-streaks.shandtriage-dream.shcall them.The refactor changes no behaviour. The strict marker count (a well-formed comment) and the loose presence check (any line with the key) stay different on purpose, now documented in the lib:
question-streaks.shrefuses to clear a streak on a malformed marker whilerun.shstill accepts a truncated report that carries the key.One idea, from the cleanup pass that produced #158 and #159.
Status
tests/report-format.shtests/run-all.shon a clean export ofcfbad54mainin this sandbox (adapter-claudeneedsps, tworeview-cmuxinteractive assertions, theprune-worktreeswatchdog timing). Run withTZ=America/New_York.run.shunchanged at 20acc9245bash -c, and no function name collides. Its three nits are fixed incfbad54.Review follow-ups in
cfbad54:notify.shandquestion-streaks.shnow say so whenreport-format.shis missing (the latter still exits 0 forupdate, as its header promises); the test requires each caller to call its helper and scans for any grep, sed or awk read of the marker or sentinel outside the lib; the test no longer overrides the cleanup trap thatsuite_tmpinstalls, which left an empty temp dir per run.Open follow-ups
test_fanout_statsintests/run-all.shfails on any machine outside a US timezone. Its orphan fixture uses18:05Z, which is the next local day in HKT, so the day-window logic correctly drops it (5 assertions; they pass withTZ=America/New_York). The fixture should use a timestamp that is mid-day in every zone. Not touched here.refactor/l2-system-prompt-file) edits the sameCHANGELOG.mddate section, so whichever merges second needs a trivial rebase.Test plan
bash tests/report-format.shtests/run-all.shon a clean export of the head,TZ=America/New_Yorkgit pullin the install checkout (the installed scripts are symlinks into it)