feat: migrate chat format to frontmatter-per-message with indexed cursors (#19) - #52
feat: migrate chat format to frontmatter-per-message with indexed cursors (#19)#52olavostauros wants to merge 9 commits into
Conversation
…nickKnackLabs#19) Replace the flat `### sender — ts` markdown format and line-number cursors with a YAML-frontmatter block per message and message-index cursors, so editing a message body no longer invalidates anyone's read position. - lib/parse.py: frontmatter-block parser with the `---`-followed-by- id/from/ts disambiguation rule; Message.message_index replaces line_number; id comes from the `id:` field; write_chat/format_message emit the new layout and assign fresh sequential ids (fixes merge id collisions). - lib/chat.sh: chat_append writes a frontmatter block; new chat_message_count, chat_last_field, chat_participants, and index-based _chat_messages_after/_chat_count_after helpers; cursors store message counts; gum renderer parses frontmatter blocks. - lib/read_query.py: cursor filter uses message_index; JSON `line` -> `index`; new --no-header for markdown output. - tasks: read delegates display to read_query.py; wait/status/list/clear use the index/frontmatter helpers instead of grep '^### ' / line counts. - .mise/tasks/migrate (new): converts legacy files in place and remaps each legacy line-number cursor (and .prev backup) to its message index, preserving read state; idempotent and --dry-run aware. - tests: update format/cursor assertions; add test/migrate.bats. 188 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
chat_format_messages() used mapfile (bash 4+ builtin) to buffer stdin into an array. macOS ships bash 3.2 by default, causing the function to crash with 'command not found' on macOS CI runners. This broke all read/cursor tests on macOS (tests 86, 88-101, 117, 166-171) due to set -eo pipefail propagating the failure. Replaced with a standard while-read loop, which works on bash 3.2+. Refs chat#19
quick-ricon
left a comment
There was a problem hiding this comment.
Requested changes after a design-first adversarial pass.
The core design direction (message-index cursors instead of line-number cursors) is right, but the implementation splits the new format contract between the Python parser and Bash line greps/awks. That makes the claimed frontmatter/body disambiguation false for several operational paths, and it can also silently mishandle existing legacy chats before users run chat migrate.
Findings:
-
lib/chat.sh:84-90,lib/chat.sh:194,lib/chat.sh:224-230— the Bash side treats any line beginningfrom:/id:as frontmatter, even when it is message body. A one-message channel whose body containsfrom: body lineis counted as two messages; the next send gets id3; unread/wait/status/list/participants can all drift from the real parser. This defeats the design premise that the parser disambiguates frontmatter blocks from markdown bodies. The fix needs a single source of truth for message boundaries (call the parser/helper, or make the Bash scanner delimiter-state-aware with tests forfrom:,id:, andts:inside bodies). -
.mise/tasks/read:53-57pluslib/chat.sh:86-90— existing legacy channels are not detected before new-format reads. On a legacy file,chat_count_newreturns 0,chat readprints “No new messages”, and thenchat_set_cursorrewrites the user's line-number cursor to new message count0. That hides the channel and clobbers read state until someone notices and runschat migrate. New-format-only commands should either auto-migrate intentionally or fail closed with a clear “run chat migrate” diagnostic before touching cursors. -
README.md:50-69andREADME.md:326-353still document line-number cursors and the old###layout as the operating model. For a format migration PR, stale docs are part of the migration boundary: users need to understand that cursors are message indices and that existing data must be migrated.
Stress checks performed locally:
mise trust,mise install- syntax/compile:
bash -non touched Bash paths andpython3 -m py_compile lib/parse.py lib/read_query.py .mise/tasks/migrate - full suite:
mise run test→ 195/195 passing - adversarial smoke: sent one message with body lines
from: body line/ts: body ts;parse_messagesreturned one message butchat_message_countreturned2 - legacy smoke:
chat read old --as aliceon an unmigrated legacy channel printed no new messages and rewrote cursor4to0
I did not open a fix-it PR because finding #1 is an architectural boundary, not a localized typo: the branch needs a deliberate choice about whether Bash helpers call the parser or implement a real stateful scanner. I can review that follow-up once it exists.
…ectness Implements chat#19. - chat_message_count, chat_participants, chat_last_field, _chat_messages_after, _chat_count_after now delegate to lib/chat_query.py (single source of truth with parse.py's disambiguation rule) - Legacy auto-migration on read (chat_require_file) and write (chat_append) paths prevents cursor clobbering - README docs updated for frontmatter format and message-index cursors
Response to reviewThanks for the thorough adversarial pass — all three findings have been addressed in commit Finding 1: Bash-side parsing mismatchProblem: Fix: All five helpers now delegate to a new
Performance note: Python is invoked for each query, but these are all O(1)-per-command calls (read, send, wait poll every 3s). For a small markdown file this is negligible. Finding 2: Legacy detection before new-format operationsProblem: On an unmigrated file, Fix: Added
Detection is conservative: looks for Finding 3: README stalenessProblem: Banner art, "How it works" diagram, cursor description, data layout, and test count all still showed the old Fix: Updated all five locations to reflect the frontmatter-per-message format and message-index cursors. Verification
|
|
The fix commit (f540427) has been pushed to the branch, fully addressing all three findings from quick-ricon's review. Here's the breakdown: Finding #1: Bash side treats body lines as frontmatterAll Bash helpers that previously used
Finding #2: Legacy channels clobber cursors silentlyAdded
The migrate script is idempotent and Finding #3: Stale README docsREADME.md fully updated:
Test results (2026-06-11)
Ready for re-review. @quick-ricon |
…to-migration Adds 3 tests to verify the three CHANGES_REQUESTED findings from quick-ricon on PR KnickKnackLabs#52 are properly addressed: 1. Body containing frontmatter-like lines (from:, id:, ts:, ---) is not falsely parsed into multiple messages 2. Legacy format files are auto-migrated on the read path 3. Auto-migration is idempotent chat#19 test/chat_lib.bats | 66 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)
Re-review requested — regression tests addedAll three findings from the initial review were already addressed in commit
Test results: 198/198 pass (up from 195). Adversarial smoke-test verification (manual, reproducible via
@quick-ricon — ready for re-review when you have time. |
…ormat # Conflicts: # README.md
|
Design pivot from review discussion with Or: The frontmatter/message-index direction still looks right, but we should not bake a permanent legacy migration system into Reasoning:
Requested direction for this PR:
I’m going to take a direct-fix pass on the branch in that direction. |
quick-ricon
left a comment
There was a problem hiding this comment.
Approved after direct-fix pivot to a clean format cutover.
What changed from the earlier review:
- merged current
maininto the branch, bringing in the explicitsend --msgchange and the stronger template-style CI gates; - removed the long-lived
chat migratetask and its migration tests; - removed automatic migration from read/write paths;
- removed product knowledge of the old message syntax;
- added generic invalid-format fail-closed behavior in the parser/query path;
- made send/append fail instead of appending to malformed existing channel files;
- fixed the
MISE_CONFIG_ROOTlibrary fallback and|| truelint failures exposed by current codebase lint; - regenerated README so
chat migrateis no longer advertised.
Validation:
bash -nover changed shell tasks/libspython3 -m py_compile lib/parse.py lib/chat_query.py lib/read_query.py- targeted invalid-format read/send/library tests
- targeted adversarial body-content parser test
- full
mise run test— 194 BATS codebase lint "$PWD"— all 9 configured rulesmise run readme --checkgit diff --check origin/main...HEAD- hosted PR checks green on Ubuntu and macOS
Remaining product note: this is now a deliberate clean cutover to the frontmatter format. Existing old local chat data should be archived/deleted or handled out-of-band, not migrated by the long-lived product CLI.
zeke-ricon
left a comment
There was a problem hiding this comment.
Requesting changes after an independent adversarial pass. The code direction is much cleaner than the migration-framework version, but two cutover edges are still not closed.
Findings:
-
README.md:50-87andREADME.md:303-332still document the old### sender — timestampchannel shape and line-number cursors as the operating model. This PR is a format cutover, so leaving the generated README's primary model on the legacy format is long-lived product knowledge in the most visible surface. It also contradicts the new message-index cursor semantics. The source isREADME.tsx:48-70andREADME.tsx:128-241. -
lib/parse.py:153-157accepts malformedid:values by silently substituting the message index, andlib/parse.py:60-67lets malformedts:values escape as a Python traceback. For a fail-closed cutover, malformed required frontmatter should be rejected generically and consistently beforesend/read/list/statustreat the channel as valid. I reproduced this with a channel containingid: banana:chat status --jsonreturned one valid message and unread state. Withts: definitely-not-a-date,chat status --jsonfailed closed but printed a traceback rather than the generic invalid-format diagnostic.
I opened a fix-it PR with the concrete patch: olavostauros#1
Validation on the reviewed branch:
git diff --check origin/main...HEADpassed.python3 -m py_compile lib/parse.py lib/chat_query.py lib/read_query.pypassed.mise run testpassed (194/194).mise run readme --checkpassed, but this only proves README.md matches README.tsx; it did not catch the stale content above.codebase lint "$PWD"passed.- Targeted malformed-file smoke found the
id:andts:behavior above.
Local note: rg is not installed in this runner, so my legacy-term scan used grep -RInE as a fallback after surfacing that tool failure.
… validation
zeke-ricon's review identified two cutover edges that were not closed:
1. Stale README.tsx still documented the old ### sender -- timestamp
format and line-number cursors. Updated:
- chatSnippet example uses frontmatter blocks
- cursorDiagram shows message-index cursors not line numbers
- 'How it works' references message indices, not line numbers
- Data layout shows message-index cursor values
- Design section says message-index based, not line counting
- Logo box art updated to frontmatter format
2. lib/parse.py silently accepted malformed id: and ts: values:
- id: banana fell through to int() ValueError then silently fell
back to message_index — now raises ChatFormatError
- ts: definitely-not-a-date escaped as a Python traceback via
fromisoformat — now raises ChatFormatError
Both produce clear, fail-closed diagnostics with the malformed
value shown in the error message.
README.md was out of date after the stale-format fix in README.tsx. Regenerated via readme build to match the new frontmatter format.
…gation, diagram alignment
Closes chat#19
Summary
Replace the flat
### sender — tsmarkdown format and line-number cursors with a YAML-frontmatter block per message and message-index cursors, so editing a message body no longer invalidates anyone's read position.What changed
lib/parse.py---disambiguation rule;Message.message_indexreplacesline_number;idfromid:field; fresh sequential ids on mergelib/chat.shchat_appendwrites frontmatter blocks; new index-based helpers (chat_message_count,_chat_messages_after,_chat_count_after); cursors store message counts; gum renderer parses frontmatter blockslib/read_query.pymessage_index; JSONline→index; new--no-headerfor markdown output.mise/tasks/readread_query.py.mise/tasks/waittail/grep '^### '/sed.mise/tasks/status,.mise/tasks/list,.mise/tasks/cleargrep '^### '/ line-count patterns with frontmatter-field reads.mise/tasks/migrate--dry-runawaretest/migrate.bats(140 lines)Stats
main(PR read: clear diagnostic when no channel can be inferred (#38) #50 merged)