kayg/calternal
Watch
1
0
Fork
You've already forked calternal
 0
Code Issues 190 Pull requests 61 Projects Releases Packages Wiki Activity Actions 26

events: pure event parser and describe rows (calendar step 3) #783

Closed
kayg wants to merge 8 commits from feat/calendar-event-parser into dev
pull from: feat/calendar-event-parser
merge into: kayg:dev
Conversation 8 Commits 8 Files changed 26 +2505 -14
kayg commented 2026-10-31 05:31:52 +00:00
Owner
Copy link

simplification for final review: use Markdown token spans as the visibility boundary for events, boxes and relationships, including inline HTML comments. hidden text never supplies a relationship. visibility costs O(n + r log r) for bytes/ranges; existing repeated child traversal remains.

Failure → effect → detection → recovery → test: unsafe input → explicit refusal with no note writes → boundary validation → edit the note or enable the completed UI contract later → awkward YAML / hidden syntax / stored replay / cross-folder transfer regressions. Gates: fmt, four-crate clippy with -D warnings, events/notes/tasks/calternal tests on dev-01.

Stacked on #781 (step 2). Branch feat/calendar-event-parser is based on feat/calendar-props-maps; until #781 lands, the diff here also shows its commits. Review the commits after "notes: clippy".

Plan (ADR 0067 build order, step 3)

Add the pure event parser and the describe rows for events: a new small crate crates/events (calternal-events). Nothing here writes files, allocates identity, or touches DAV. Step 4 (identity and promotion) builds on its marker module.

What it is

  • clock : the log clock grammar (superset of crates/daily/src/log.rs: it also reads real end seconds and fractions). Pure.
  • line : one clock-starting line (with or without a list marker) -> Head (clock range, start-only, all day), checkbox mark, title, tags, trailing block marker. Checkbox wins over clock (the row is task, not event).
  • marker : the one parser for trailing ^id markers (16 lowercase Crockford = event id; legacy 6-char kept as-is). Step 4 adds writing next to it.
  • scan : Markdown walk: skips frontmatter, code (fenced + indented), HTML blocks/comments, blockquotes; supplies the day (daily note stem or day:; else nearest whole ISO-date ATX heading); finds direct child sub-bullets (standalone link, checkbox).
  • note : note classification from frontmatter (event owner / series / per-day override), so an event note with status is not a todo task. calternal-tasks calls it.
  • describer : EventsDescriber (notes::describer::Describer), registered by EventsPlugin (no actions yet). Rows go in notes/meta.db in the note's own transaction, like tasks.

Rows (derived; version-bumped rebuild)

event_lines(note,line,kind,day,all_day,start_s,start_frac,end_s,end_frac,end_next,has_end,point,title,tags,area,block,event_id,children), event_children(note,parent,line,kind,target,mark), event_notes(note, event_id, start, end, all_day, zone, calendar, status, series, occurrence, repeat), journal_events(note,event_id, title, start, end, all_day, zone, calendar, status) (the events map read with the step-2 props::keys_at/get_at). Civil data only (day + clock seconds); instants/zones are a reader concern (frozen time-zone is stored, not interpreted).

Sizes and big-O

  • Note of N bytes, L lines, E event lines, C children. Prefilter: one pass over N bytes with no allocation; a note with no clock-shaped list line and no events/event-id/series key returns the shared empty answer (like tasks' nothing()), so ordinary notes cost O(N) scan only.
  • Parse: O(L) plus O(E + C); child lookup is a forward walk that each line is visited once for its parent (children of a line stop at the first line not deeper), so O(L) total.
  • Store: O(E + C) inserts per changed note, delete-by-note first (indexed on note); cold fill drops indexes and rebuilds like tasks.
  • Memory: E rows per note; a 100k-event Home is ~100k small rows.
  • Budget: extra describe <= 0.2 ms p95 per changed note (ADR 0067). Test describe_budget asserts it on realistic notes (ordinary prose, a 30-event daily note) in --release; debug runs use a 10x bound.

Failure table

Failure Effect Detection Recovery Test
Odd clock (0845, 25:00, 9am+letters, 13:00pm) would invent an event strict grammar identical to daily not a row, text kept shared_log_vectors, differential_vs_daily
Clock in code / frontmatter / HTML / comment / quote false event scan state not a row skips_code_frontmatter_html_quotes
Prose mentioning a clock, no list marker false event line grammar not a row vectors
Checkbox with clock counted as event and task boxed -> kind task kind='task' row only classification
Event note with status: listed as a todo task tasks found::task asks calternal_events::note::is_event not a task note, child checkboxes still tasks event_note_is_not_a_task
No date context in non-daily note event on wrong day no day -> no row nothing guessed needs_a_date
Duplicate / broken frontmatter wrong owner fields props refuses; owner row has broken-less empty fields rows from valid keys only, nothing written broken_frontmatter_reads_nothing_unsafe
Marker duplicates across notes two events one id rows keep both (describer never decides) step 4 identity actions decide duplicate_markers_are_kept
Describer panics on odd text note not indexed fuzz/proptest on random text parser is total (no unwrap on input) never_panics
Second scan drift between views rows read through Notes describe step only by construction rows_commit_with_the_note (real Notes home)
Slow describe typing latency budget test tighten prefilter describe_budget

Gate tests

Old shared log vectors (contracts/log-entries.json, every entries list, run through this parser and compared to calternal_daily::log::entries), new vectors (contracts/event-lines.json: sub-bullet, all-day, seconds/fractions, tags, markers, start-only, equal/reversed ends), event/task classification, no second scan (real Notes home, rows read back), describe budget.

Decisions (no questions asked)

  • New crate rather than editing daily: the clock grammar is a superset (end seconds, fractions) and daily/the editor share the vectors byte for byte; a differential test pins them together on random lines.
  • Explicit end-day syntax is not defined in 0068, so it is not parsed (a - 08;45 - tomorrow Walk stays start-only with the old text, per the old vector). Multi-day events carry bounds in their owner (start/end).
  • Non-daily notes: only a whole ISO-date ATX heading gives a day; a later heading of the same or higher level that is not a date ends it. Event-owner bounds do not date body lines (agenda lists in meeting notes would become events).
  • Blockquoted clock lines are unsupported prose (0068 O01).
  • Tasks keep reading the first link on the checkbox line; teaching the tasks index the sub-bullet form is the tasks amendment, not this step. Sub-bullet children of task lines are stored in event_children for it.

Results (calternal-dev-01-compute02, inside the infra/nix-devshell flake; flake files not committed)

  • cargo clippy -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --all-targets -j4 -- -D warnings : clean; cargo fmt applied.
  • cargo test -p calternal-events -j4 : 13 passed. Gates: old_shared_log_vectors (every case of contracts/log-entries.json on the legacy list-line subset, task boxes included), differential_vs_daily (5,000 random clock-shaped line sets, same accepted lines and fields as calternal_daily::log::entries), event_vectors (16 new cases in contracts/event-lines.json: sub-bullets, all-day, seconds and fractions, tags, markers, headings, lenient clocks; expectations generated once and reviewed by hand), checkbox_lines_are_tasks_and_plain_clocks_are_events and the note::kind tests (classification), rows_commit_with_the_note (real Notes home: rows appear with the note, change with an outside edit, vanish on delete, and a rebuild after clearing the describer version gives the same rows; no other scanner exists), never_panics.
  • cargo test --release -p calternal-events describe_budget -j4 -- --nocapture : prose note (6.5 KB) p95 0.098 ms; busy daily note (30 event lines with children) p95 0.092 ms; both under the 0.2 ms gate. (Debug builds only check a 5 ms bound.) A fresh quiet perf-VM measurement is recorded below; this older figure is from a shared VM.
  • cargo test --no-fail-fast -p calternal-notes -p calternal-tasks -p calternal : notes 435, tasks 29 (new an_event_note_with_a_status_is_not_a_task_but_its_child_tasks_are), calternal all green. contracts/actions.json is unchanged (the plugin has no actions yet).
  • Earlier tasks/notes run flaked once in a notes room test under VM load; it passed on rerun.

Also added to the notes crate: props::pairs_at (read the pairs of a nested map), used by note::journal_events.

Resume changes and verification (2026-10-31)

Bare 10:00 - 11:00 Going for a stroll #area/dailyliving lines are events too. Added parsing, direct-child relationships, hand-checked vectors and bare_clock_lines_are_separate_events; code/fences and time-shaped prose stay excluded. Describer version is 2 so existing indexes rebuild. The old daily grammar still requires list markers: its unchanged shared vectors and 5,000-case differential test run against that subset; bare clocks are explicitly covered by the new test and event vectors.

Fresh dev-01 checks, Nix shell; heavy commands wrapped in ~/bin/slot:

  • cargo fmt --all -- --check: passed.
  • cargo clippy -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --all-targets -j4 -- -D warnings: passed.
  • cargo test -p calternal-events -j4: 14 passed, including old/new vectors, classification, same-transaction index/rebuild, fuzz and describe budget.
  • cargo test --release -p calternal-events describe_budget -j4 -- --nocapture: passed; prose p95 0.1277 ms, busy day p95 0.1052 ms (limit 0.2 ms).
  • Before the bare-clock addition, cargo test -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --no-fail-fast -j4 passed: notes 435 / 6 ignored, tasks 29, calternal all green. Those crates are also checked on the final step-4 stack.

Raw logs: calternal-dev-01-compute02:~/cal-resume-s3.log, ~/cal-resume-s3-verified.log. No PRs merged.

Quiet describe measurement

Release test executables built on dev-01 were relayed through the control host to perf-01, SHA256 checked at both ends, then run as three alternating pairs under ~/bin/perf-lock. Head (1b6e5659, SHA256 4091973618e68fb46417a158d4b878b88d022dd00242c4b727c340d843e21b41) median of per-run p95: prose 0.0555 ms, busy daily 0.0461 ms, both below 0.2 ms. Comparator was the existing step-4 executable at 8e95a8b0 (unchanged pre-fix parser from 735106a0, SHA256 899f9ce8b4000340500ef4cb745bce302ff97b97d7352516d7681df0458cad10); median p95 0.0573 / 0.0453 ms. This is a parser/describe microbenchmark, not the 100k cold-index or DAV delivery gate.

Exact measurement: ~/bin/perf-lock bash -c 'set -e; for n in 1 2 3; do ~/cal-resume-parser-baseline-identity describe_budget --nocapture; ~/cal-resume-parser-head describe_budget --nocapture; done'. Raw output: calternal-perf-01-backup01:~/cal-resume-describe.log.

> simplification for final review: use Markdown token spans as the visibility boundary for events, boxes and relationships, including inline HTML comments. hidden text never supplies a relationship. visibility costs O(n + r log r) for bytes/ranges; existing repeated child traversal remains. Failure → effect → detection → recovery → test: unsafe input → explicit refusal with no note writes → boundary validation → edit the note or enable the completed UI contract later → awkward YAML / hidden syntax / stored replay / cross-folder transfer regressions. Gates: fmt, four-crate clippy with -D warnings, events/notes/tasks/calternal tests on dev-01. > **Stacked on #781 (step 2).** Branch `feat/calendar-event-parser` is based on `feat/calendar-props-maps`; until #781 lands, the diff here also shows its commits. Review the commits after "notes: clippy". # Plan (ADR 0067 build order, step 3) Add the **pure event parser** and the **describe rows** for events: a new small crate `crates/events` (`calternal-events`). Nothing here writes files, allocates identity, or touches DAV. Step 4 (identity and promotion) builds on its marker module. ## What it is - `clock` : the log clock grammar (superset of `crates/daily/src/log.rs`: it also reads real end seconds and fractions). Pure. - `line` : one clock-starting line (with or without a list marker) -> `Head` (clock range, start-only, `all day`), checkbox mark, title, tags, trailing block marker. Checkbox wins over clock (the row is `task`, not `event`). - `marker` : the one parser for trailing `^id` markers (16 lowercase Crockford = event id; legacy 6-char kept as-is). Step 4 adds writing next to it. - `scan` : Markdown walk: skips frontmatter, code (fenced + indented), HTML blocks/comments, blockquotes; supplies the day (daily note stem or `day:`; else nearest whole ISO-date ATX heading); finds direct child sub-bullets (standalone link, checkbox). - `note` : note classification from frontmatter (event owner / series / per-day override), so an event note with `status` is **not** a todo task. `calternal-tasks` calls it. - `describer` : `EventsDescriber` (`notes::describer::Describer`), registered by `EventsPlugin` (no actions yet). Rows go in `notes/meta.db` in the note's own transaction, like tasks. ## Rows (derived; version-bumped rebuild) `event_lines(note,line,kind,day,all_day,start_s,start_frac,end_s,end_frac,end_next,has_end,point,title,tags,area,block,event_id,children)`, `event_children(note,parent,line,kind,target,mark)`, `event_notes(note, event_id, start, end, all_day, zone, calendar, status, series, occurrence, repeat)`, `journal_events(note,event_id, title, start, end, all_day, zone, calendar, status)` (the `events` map read with the step-2 `props::keys_at/get_at`). Civil data only (day + clock seconds); instants/zones are a reader concern (frozen `time-zone` is stored, not interpreted). ## Sizes and big-O - Note of N bytes, L lines, E event lines, C children. Prefilter: one pass over N bytes with no allocation; a note with no clock-shaped list line and no `events`/`event-id`/`series` key returns the shared empty answer (like tasks' `nothing()`), so ordinary notes cost O(N) scan only. - Parse: O(L) plus O(E + C); child lookup is a forward walk that each line is visited once for its parent (children of a line stop at the first line not deeper), so O(L) total. - Store: O(E + C) inserts per changed note, delete-by-note first (indexed on `note`); cold fill drops indexes and rebuilds like tasks. - Memory: E rows per note; a 100k-event Home is ~100k small rows. - Budget: extra describe <= 0.2 ms p95 per changed note (ADR 0067). Test `describe_budget` asserts it on realistic notes (ordinary prose, a 30-event daily note) in `--release`; debug runs use a 10x bound. ## Failure table | Failure | Effect | Detection | Recovery | Test | |---|---|---|---|---| | Odd clock (`0845`, `25:00`, `9am`+letters, `13:00pm`) | would invent an event | strict grammar identical to daily | not a row, text kept | `shared_log_vectors`, `differential_vs_daily` | | Clock in code / frontmatter / HTML / comment / quote | false event | scan state | not a row | `skips_code_frontmatter_html_quotes` | | Prose mentioning a clock, no list marker | false event | line grammar | not a row | vectors | | Checkbox with clock | counted as event and task | `boxed` -> kind `task` | `kind='task'` row only | `classification` | | Event note with `status:` | listed as a todo task | tasks `found::task` asks `calternal_events::note::is_event` | not a task note, child checkboxes still tasks | `event_note_is_not_a_task` | | No date context in non-daily note | event on wrong day | no day -> no row | nothing guessed | `needs_a_date` | | Duplicate / broken frontmatter | wrong owner fields | `props` refuses; owner row has `broken`-less empty fields | rows from valid keys only, nothing written | `broken_frontmatter_reads_nothing_unsafe` | | Marker duplicates across notes | two events one id | rows keep both (describer never decides) | step 4 identity actions decide | `duplicate_markers_are_kept` | | Describer panics on odd text | note not indexed | fuzz/proptest on random text | parser is total (no unwrap on input) | `never_panics` | | Second scan | drift between views | rows read through `Notes` describe step only | by construction | `rows_commit_with_the_note` (real Notes home) | | Slow describe | typing latency | budget test | tighten prefilter | `describe_budget` | ## Gate tests Old shared log vectors (`contracts/log-entries.json`, every `entries` list, run through this parser and compared to `calternal_daily::log::entries`), new vectors (`contracts/event-lines.json`: sub-bullet, all-day, seconds/fractions, tags, markers, start-only, equal/reversed ends), event/task classification, no second scan (real `Notes` home, rows read back), describe budget. ## Decisions (no questions asked) - New crate rather than editing `daily`: the clock grammar is a superset (end seconds, fractions) and `daily`/the editor share the vectors byte for byte; a differential test pins them together on random lines. - Explicit end-day syntax is not defined in 0068, so it is **not parsed** (a `- 08;45 - tomorrow Walk` stays start-only with the old text, per the old vector). Multi-day events carry bounds in their owner (`start`/`end`). - Non-daily notes: only a whole ISO-date ATX heading gives a day; a later heading of the same or higher level that is not a date ends it. Event-owner bounds do **not** date body lines (agenda lists in meeting notes would become events). - Blockquoted clock lines are unsupported prose (0068 O01). - Tasks keep reading the first link on the checkbox line; teaching the tasks index the sub-bullet form is the tasks amendment, not this step. Sub-bullet children of task lines are stored in `event_children` for it. # Results (calternal-dev-01-compute02, inside the `infra/nix-devshell` flake; flake files not committed) - `cargo clippy -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --all-targets -j4 -- -D warnings` : clean; `cargo fmt` applied. - `cargo test -p calternal-events -j4` : 13 passed. Gates: `old_shared_log_vectors` (every case of `contracts/log-entries.json` on the legacy list-line subset, task boxes included), `differential_vs_daily` (5,000 random clock-shaped line sets, same accepted lines and fields as `calternal_daily::log::entries`), `event_vectors` (16 new cases in `contracts/event-lines.json`: sub-bullets, all-day, seconds and fractions, tags, markers, headings, lenient clocks; expectations generated once and reviewed by hand), `checkbox_lines_are_tasks_and_plain_clocks_are_events` and the `note::kind` tests (classification), `rows_commit_with_the_note` (real `Notes` home: rows appear with the note, change with an outside edit, vanish on delete, and a rebuild after clearing the describer version gives the same rows; no other scanner exists), `never_panics`. - `cargo test --release -p calternal-events describe_budget -j4 -- --nocapture` : prose note (6.5 KB) p95 0.098 ms; busy daily note (30 event lines with children) p95 0.092 ms; both under the 0.2 ms gate. (Debug builds only check a 5 ms bound.) A fresh quiet perf-VM measurement is recorded below; this older figure is from a shared VM. - `cargo test --no-fail-fast -p calternal-notes -p calternal-tasks -p calternal` : notes 435, tasks 29 (new `an_event_note_with_a_status_is_not_a_task_but_its_child_tasks_are`), calternal all green. `contracts/actions.json` is unchanged (the plugin has no actions yet). - Earlier tasks/notes run flaked once in a notes room test under VM load; it passed on rerun. Also added to the notes crate: `props::pairs_at` (read the pairs of a nested map), used by `note::journal_events`. ## Resume changes and verification (2026-10-31) Bare `10:00 - 11:00 Going for a stroll #area/dailyliving` lines are events too. Added parsing, direct-child relationships, hand-checked vectors and `bare_clock_lines_are_separate_events`; code/fences and time-shaped prose stay excluded. Describer version is 2 so existing indexes rebuild. The old daily grammar still requires list markers: its unchanged shared vectors and 5,000-case differential test run against that subset; bare clocks are explicitly covered by the new test and event vectors. Fresh dev-01 checks, Nix shell; heavy commands wrapped in `~/bin/slot`: - `cargo fmt --all -- --check`: passed. - `cargo clippy -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --all-targets -j4 -- -D warnings`: passed. - `cargo test -p calternal-events -j4`: 14 passed, including old/new vectors, classification, same-transaction index/rebuild, fuzz and describe budget. - `cargo test --release -p calternal-events describe_budget -j4 -- --nocapture`: passed; prose p95 0.1277 ms, busy day p95 0.1052 ms (limit 0.2 ms). - Before the bare-clock addition, `cargo test -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --no-fail-fast -j4` passed: notes 435 / 6 ignored, tasks 29, calternal all green. Those crates are also checked on the final step-4 stack. Raw logs: `calternal-dev-01-compute02:~/cal-resume-s3.log`, `~/cal-resume-s3-verified.log`. No PRs merged. ### Quiet describe measurement Release test executables built on dev-01 were relayed through the control host to perf-01, SHA256 checked at both ends, then run as three alternating pairs under `~/bin/perf-lock`. Head (`1b6e5659`, SHA256 `4091973618e68fb46417a158d4b878b88d022dd00242c4b727c340d843e21b41`) median of per-run p95: prose **0.0555 ms**, busy daily **0.0461 ms**, both below 0.2 ms. Comparator was the existing step-4 executable at `8e95a8b0` (unchanged pre-fix parser from `735106a0`, SHA256 `899f9ce8b4000340500ef4cb745bce302ff97b97d7352516d7681df0458cad10`); median p95 0.0573 / 0.0453 ms. This is a parser/describe microbenchmark, not the 100k cold-index or DAV delivery gate. Exact measurement: `~/bin/perf-lock bash -c 'set -e; for n in 1 2 3; do ~/cal-resume-parser-baseline-identity describe_budget --nocapture; ~/cal-resume-parser-head describe_budget --nocapture; done'`. Raw output: `calternal-perf-01-backup01:~/cal-resume-describe.log`.
notes: clippy
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details
cd8e5cbdc2
events: pure event parser and describe rows
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details
5c1447ab68
kayg commented 2026-10-31 16:20:05 +00:00
Author
Owner
Copy link

Independent review, round 1

Verdict: fix first. Reviewed 1b6e56594f21e696744fc0d508637036857d46d8 relative to #781, with ADRs 0067/0068/0056/0066. Findings are source-level traces; full suites were not rerun by this reviewer. Inherited #781 preservation blockers also remain.

  1. P1 — Indented code becomes events. crates/events/src/scan.rs:85 and :98. A daily note containing \t10:00 code\n produces an event because indentation counts bytes (one tab), not Markdown columns. Also - parent\n\n 10:00 code\n retains in_list=true and treats the list's indented code as an event. Fix: track actual Markdown containers and code indentation, including tab expansion and code inside list items. Add both vectors; downstream identity/promotion must never write markers into these code blocks.

  2. P1 — Frontmatter with CRLF/BOM is scanned as body. crates/events/src/scan.rs:50 (frontmatter::len only recognizes ---\n). For a daily file with ---\r\ndescription: |\r\n 10:00 YAML text\r\n---\r\n, the description is emitted as an event. Fix: recognize supported frontmatter envelopes consistently or suppress unsafe parsing when an unsupported/unclosed envelope is detected. Add CRLF/BOM block-scalar clocks and malformed envelopes to vectors.

  3. P1 — Relationship scan reads checkboxes/links inside HTML comments. crates/events/src/scan.rs:225 (children). For - 09:00 Event\n <!--\n - [ ] fake\n -->\n, the child scan emits a task relationship. It has its own fence state but no HTML/comment state, unlike the main walk. This can also make promotion treat a commented task-note link as an existing owner. Fix: derive relationships from the same parsed container/visibility state; test commented and raw-HTML children as well as code. Do not scan their text as relationships.

  4. P2 — Non-ASCII tags are truncated into different tags. crates/events/src/line.rs:165 (ASCII-only tag byte loop on this head). - 09:00 Walk #café indexes caf; #area/工作 loses its calendar tag. The editor recognizes Unicode letters/numbers. Fix: share the tag grammar or use Unicode-aware character iteration and add non-ASCII vectors, including promotion metadata.

  5. P2 — Child lookup is not linear as claimed. crates/events/src/scan.rs:106 calls children on each event's remaining subtree. With nested event lines, each ancestor traverses descendants again; MAX_CHILDREN limits returned direct children, not traversal. Fix: collect parent/direct-child relationships in one container walk, or explicitly bound supported nesting and account for skipped spans. Add a nested-note scaling case alongside the flat 30-event budget fixture.

Failure-table audit:

Claimed failure row Evidence / limitation
Odd clock Old vectors and 5,000-case differential test exist for legacy list lines; they deliberately exclude fractional/end-seconds extensions and bare lines. New vectors cover ordinary extensions; this is not exhaustive clock fuzzing.
Code/frontmatter/HTML/comment/quote Some vectors exist, but findings 1–3 are omitted. No function named skips_code_frontmatter_html_quotes exists on this head.
Prose mentioning clocks Explicit prose/bare-line vectors exist. Code/container exclusions are not proven by them.
Checkbox with clock Classification test checks task vs event rows. Unknown custom boxes are nevertheless accepted by checkbox, despite 0068 O01 saying unsupported.
Event note with status Notes and tasks classification tests cover this.
No date context no_day_no_event covers missing context and owner-body policy; whole-date headings have vectors.
Duplicate/broken frontmatter Top-level duplicate events has a test. The cited broken_frontmatter_reads_nothing_unsafe test does not exist. pairs_at does not validate the whole YAML block or reject duplicate returned child keys; add nested duplicates and malformed-owner fixtures before asserting “valid keys only.”
Duplicate markers Test retains both rows as intended; uniqueness is deferred to #785.
Panic on odd text never_panics exercises scan/read with random text; it does not prove semantic exclusions.
Second scan Code registers the describer in Notes and stores rows through its transaction. rows_commit_with_the_note exercises outside edit/delete/rebuild; it does not itself assert transaction ordering at journal publication or room-edit behavior. No independent watcher was introduced.
Slow describe describe_budget checks two small fixtures; debug allows 5 ms. PR reports quiet release p95 below 0.2 ms, but those numbers were not independently remeasured here. Nested complexity and 100k cold-index growth remain unproven.

Times/all-day/seconds/equal ends/next-day rollover have vectors; full multi-day bounds are retained in owner fields. I found no specified explicit end-day line grammar to require inventing here. Quoted event prose remains excluded; quoted checkbox support and unknown-box refusal still need reconciliation with O01.

Mergeability: fresh dev-01 git merge-tree --write-tree origin/dev 1b6e5659 exits 0 against dev=2f304154054ec68b7855bb1f2bd24b1fb15a386d (tree 53dcef917614148b2159e42717375afb01f668cd). Forgejo still says false. #781 ancestry explains the cumulative diff, not a content conflict; exact server cause remains unproven. No code pushed or merged.

Independent reproduction addendum: the dev-01 standalone Rust probe (/tmp/cal-independent-r1/repro.rs, binary /tmp/cal-independent-r1/repro), using parser modules copied directly from the reviewed stack and built inside Nix through ~/bin/slot, emitted an event for tab-indented code, an event for six-space code inside a list, an event for the CRLF YAML block-scalar clock, and a boxed child for the checkbox inside an HTML comment. All four bad outputs were observed. The focused probe links cached Notes dependencies; it is not a full gate or performance run.

# Independent review, round 1 **Verdict: fix first.** Reviewed `1b6e56594f21e696744fc0d508637036857d46d8` relative to #781, with ADRs 0067/0068/0056/0066. Findings are source-level traces; full suites were not rerun by this reviewer. Inherited #781 preservation blockers also remain. 1. **P1 — Indented code becomes events.** `crates/events/src/scan.rs:85` and `:98`. A daily note containing `\t10:00 code\n` produces an event because indentation counts bytes (one tab), not Markdown columns. Also `- parent\n\n 10:00 code\n` retains `in_list=true` and treats the list's indented code as an event. **Fix:** track actual Markdown containers and code indentation, including tab expansion and code inside list items. Add both vectors; downstream identity/promotion must never write markers into these code blocks. 2. **P1 — Frontmatter with CRLF/BOM is scanned as body.** `crates/events/src/scan.rs:50` (`frontmatter::len` only recognizes `---\n`). For a daily file with `---\r\ndescription: |\r\n 10:00 YAML text\r\n---\r\n`, the description is emitted as an event. **Fix:** recognize supported frontmatter envelopes consistently or suppress unsafe parsing when an unsupported/unclosed envelope is detected. Add CRLF/BOM block-scalar clocks and malformed envelopes to vectors. 3. **P1 — Relationship scan reads checkboxes/links inside HTML comments.** `crates/events/src/scan.rs:225` (`children`). For `- 09:00 Event\n <!--\n - [ ] fake\n -->\n`, the child scan emits a task relationship. It has its own fence state but no HTML/comment state, unlike the main walk. This can also make promotion treat a commented task-note link as an existing owner. **Fix:** derive relationships from the same parsed container/visibility state; test commented and raw-HTML children as well as code. Do not scan their text as relationships. 4. **P2 — Non-ASCII tags are truncated into different tags.** `crates/events/src/line.rs:165` (ASCII-only tag byte loop on this head). `- 09:00 Walk #café` indexes `caf`; `#area/工作` loses its calendar tag. The editor recognizes Unicode letters/numbers. **Fix:** share the tag grammar or use Unicode-aware character iteration and add non-ASCII vectors, including promotion metadata. 5. **P2 — Child lookup is not linear as claimed.** `crates/events/src/scan.rs:106` calls `children` on each event's remaining subtree. With nested event lines, each ancestor traverses descendants again; `MAX_CHILDREN` limits returned direct children, not traversal. **Fix:** collect parent/direct-child relationships in one container walk, or explicitly bound supported nesting and account for skipped spans. Add a nested-note scaling case alongside the flat 30-event budget fixture. Failure-table audit: | Claimed failure row | Evidence / limitation | |---|---| | Odd clock | Old vectors and 5,000-case differential test exist for legacy list lines; they deliberately exclude fractional/end-seconds extensions and bare lines. New vectors cover ordinary extensions; this is not exhaustive clock fuzzing. | | Code/frontmatter/HTML/comment/quote | Some vectors exist, but findings 1–3 are omitted. No function named `skips_code_frontmatter_html_quotes` exists on this head. | | Prose mentioning clocks | Explicit prose/bare-line vectors exist. Code/container exclusions are not proven by them. | | Checkbox with clock | Classification test checks task vs event rows. Unknown custom boxes are nevertheless accepted by `checkbox`, despite 0068 O01 saying unsupported. | | Event note with status | Notes and tasks classification tests cover this. | | No date context | `no_day_no_event` covers missing context and owner-body policy; whole-date headings have vectors. | | Duplicate/broken frontmatter | Top-level duplicate `events` has a test. The cited `broken_frontmatter_reads_nothing_unsafe` test does not exist. `pairs_at` does not validate the whole YAML block or reject duplicate returned child keys; add nested duplicates and malformed-owner fixtures before asserting “valid keys only.” | | Duplicate markers | Test retains both rows as intended; uniqueness is deferred to #785. | | Panic on odd text | `never_panics` exercises scan/read with random text; it does not prove semantic exclusions. | | Second scan | Code registers the describer in Notes and stores rows through its transaction. `rows_commit_with_the_note` exercises outside edit/delete/rebuild; it does not itself assert transaction ordering at journal publication or room-edit behavior. No independent watcher was introduced. | | Slow describe | `describe_budget` checks two small fixtures; debug allows 5 ms. PR reports quiet release p95 below 0.2 ms, but those numbers were not independently remeasured here. Nested complexity and 100k cold-index growth remain unproven. | Times/all-day/seconds/equal ends/next-day rollover have vectors; full multi-day bounds are retained in owner fields. I found no specified explicit end-day line grammar to require inventing here. Quoted event prose remains excluded; quoted checkbox support and unknown-box refusal still need reconciliation with O01. Mergeability: fresh dev-01 `git merge-tree --write-tree origin/dev 1b6e5659` exits 0 against `dev=2f304154054ec68b7855bb1f2bd24b1fb15a386d` (tree `53dcef917614148b2159e42717375afb01f668cd`). Forgejo still says false. #781 ancestry explains the cumulative diff, not a content conflict; exact server cause remains unproven. No code pushed or merged. Independent reproduction addendum: the dev-01 standalone Rust probe (`/tmp/cal-independent-r1/repro.rs`, binary `/tmp/cal-independent-r1/repro`), using parser modules copied directly from the reviewed stack and built inside Nix through `~/bin/slot`, emitted an event for tab-indented code, an event for six-space code inside a list, an event for the CRLF YAML block-scalar clock, and a boxed child for the checkbox inside an HTML comment. All four bad outputs were observed. The focused probe links cached Notes dependencies; it is not a full gate or performance run.
kayg commented 2026-10-31 16:39:32 +00:00
Author
Owner
Copy link

Round 1 implementation fixes

Head 3f94719e, rebased onto #781 f92341a6. No merge.

Finding Status / regression
P1 tab-indented and list-contained code emitted as events Fixed: Markdown parser container ranges exclude both exact reproductions. review_code_and_unsafe_yaml_are_not_events.
P1 CRLF/BOM/malformed YAML scanned as body Fixed: inherit strict envelope validation from #781 and suppress unsafe scans; regression includes block-scalar clocks and unclosed malformed envelope.
P1 commented/raw-HTML checkboxes emitted as relationships Fixed: event and child walkers share the same Markdown exclusions; review_relationships_share_visibility includes comments, raw HTML and list code.
P2 Unicode tags truncated Fixed: Unicode character iteration preserves café and area/工作; review_unicode_tags_are_whole.
P2 nested child lookup can revisit descendant subtrees Remaining: requires replacing the relationship traversal. Flat-note budget test passes, but no linear nested-note scaling claim is made.

Red evidence: all three new regression tests failed before changes in ~/cal-r1-s3-red.log. The first focused run also exposed an existing vector treating a bare clock lazily continuing a blockquote as an event. Under Markdown container rules it is quoted prose; the vector now explicitly excludes it. Ordinary bare clocks remain supported.

Fresh dev-01 Nix gates, heavy commands through ~/bin/slot, log ~/cal-r1-s3-gates.log:

  • cargo fmt --all && cargo fmt --all -- --check
  • cargo clippy -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --all-targets -j4 -- -D warnings
  • cargo test -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --no-fail-fast -j4

All passed: events 17, notes 438 / 6 ignored, tasks 29, plus calternal unit/integration gates. Existing ignored tests remain ignored. No new quiet perf measurement or 100k cold-index claim. Lookup adds one Markdown parse and sorted exclusion ranges; nested child traversal retains the documented worst-case quadratic limitation. No further independent review round is claimed.

Other review-audit gaps retained for follow-up: quoted-checkbox container support, unknown-box refusal, fuller malformed/nested-owner fixtures, independent journal publication ordering assertion, and large cold-index/perf validation. These do not waive the P0/P1 fixes above.

## Round 1 implementation fixes Head `3f94719e`, rebased onto #781 `f92341a6`. No merge. | Finding | Status / regression | |---|---| | P1 tab-indented and list-contained code emitted as events | Fixed: Markdown parser container ranges exclude both exact reproductions. `review_code_and_unsafe_yaml_are_not_events`. | | P1 CRLF/BOM/malformed YAML scanned as body | Fixed: inherit strict envelope validation from #781 and suppress unsafe scans; regression includes block-scalar clocks and unclosed malformed envelope. | | P1 commented/raw-HTML checkboxes emitted as relationships | Fixed: event and child walkers share the same Markdown exclusions; `review_relationships_share_visibility` includes comments, raw HTML and list code. | | P2 Unicode tags truncated | Fixed: Unicode character iteration preserves `café` and `area/工作`; `review_unicode_tags_are_whole`. | | P2 nested child lookup can revisit descendant subtrees | Remaining: requires replacing the relationship traversal. Flat-note budget test passes, but no linear nested-note scaling claim is made. | Red evidence: all three new regression tests failed before changes in `~/cal-r1-s3-red.log`. The first focused run also exposed an existing vector treating a bare clock lazily continuing a blockquote as an event. Under Markdown container rules it is quoted prose; the vector now explicitly excludes it. Ordinary bare clocks remain supported. Fresh dev-01 Nix gates, heavy commands through `~/bin/slot`, log `~/cal-r1-s3-gates.log`: - `cargo fmt --all && cargo fmt --all -- --check` - `cargo clippy -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --all-targets -j4 -- -D warnings` - `cargo test -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --no-fail-fast -j4` All passed: events 17, notes 438 / 6 ignored, tasks 29, plus calternal unit/integration gates. Existing ignored tests remain ignored. No new quiet perf measurement or 100k cold-index claim. Lookup adds one Markdown parse and sorted exclusion ranges; nested child traversal retains the documented worst-case quadratic limitation. No further independent review round is claimed. Other review-audit gaps retained for follow-up: quoted-checkbox container support, unknown-box refusal, fuller malformed/nested-owner fixtures, independent journal publication ordering assertion, and large cold-index/perf validation. These do not waive the P0/P1 fixes above.
kayg commented 2026-10-31 17:10:32 +00:00
Author
Owner
Copy link

Independent review, round 2

Verdict: fix first. Reviewed 3f94719e35a3ddabd39ec794de1f24dde77b49cf relative to #781, both round-1 comments, and ADRs 0067/0068/0056/0066. No prior round-2 comment existed when checked. No code pushed or merged. #781's remaining byte-preservation blocker is inherited.

Round-1 finding Round-2 status Does the test prove it?
P1 tab/list-contained code becomes events Verified for the reported cases. review_code_and_unsafe_yaml_are_not_events includes both exact code reproductions; Markdown code-block ranges now exclude them.
P1 CRLF/BOM/malformed frontmatter becomes events Verified for the reported envelopes. The same test includes CRLF/BOM block-scalar clocks and an unclosed malformed envelope. Validation suppresses scanning, rather than claiming support for those envelopes.
P1 commented/raw-HTML child relationships Not fixed for all comments; standalone comment/block cases are fixed. review_relationships_share_visibility proves standalone comment, div and list-code cases. It omits multiline inline comments; see below.
P2 Unicode tags truncated Verified for the reported Unicode cases. review_unicode_tags_are_whole checks exact café and area/工作 output; code advances by UTF-8 character lengths. This is not full parity with every editor tag form.
P2 repeated nested child traversal Not fixed, explicitly acknowledged. No nested scaling regression was added. Each ancestor still scans its remaining subtree; the flat budget fixture does not prove a linear bound.

New P1 — Multiline inline HTML comments still expose hidden relationships. crates/events/src/scan.rs:298 (the Event::Html exclusion match) and :254 (child exclusion lookup). With:

- 09:00 Event <!--
  - [ ] fake
  -->

scan(..., Day::Fixed("2026-10-26")) returns a child checkbox for fake. The same happens when text <!-- is on an indented continuation line before the hidden checkbox. Pulldown emits inline HTML for these comments, but the added exclusions handle Event::Html and HTML blocks, not Event::InlineHtml; the old line state only notices comments starting a line. This emits a derived task relationship for text the person cannot see; in #785 the plain-checkbox identity scanner uses the same exclusions, so hidden checkboxes can also become identity targets when writes are enabled. Fix: include multiline inline-comment spans in the shared visibility model, filtering by the candidate token's actual span so visible text before/after inline HTML remains usable. Add both vectors, hidden clock/checkbox/link variants, and a real promotion test proving a commented owner link is ignored. This is a surviving class of the round-1 problem, not a newly introduced syntax category.

Verification: dev-01 Nix shell, via ~/bin/slot, cargo test -p calternal-events review_ -j4 at stack head 771eb102 passed 17 tests (including the three parser regressions). A standalone Rust probe linked the current stack libraries and observed both inline-comment false relationships; the standalone-comment control had no child. The relevant exclusion/children helpers are unchanged from this PR. Probe/output: /tmp/cal-r2-probe.rs, /tmp/cal-r2-probe.out. No fresh release performance, full-suite or 100k cold-index result is claimed.

Other round-1 audit limitations remain: quoted-checkbox support, unknown-box refusal, malformed/nested-owner semantics, independent journal-publication ordering, and large cold-index validation. Changing the old lazy-blockquote vector agrees with Markdown container rules; it does not prove these other cases.

# Independent review, round 2 **Verdict: fix first.** Reviewed `3f94719e35a3ddabd39ec794de1f24dde77b49cf` relative to #781, both round-1 comments, and ADRs 0067/0068/0056/0066. No prior round-2 comment existed when checked. No code pushed or merged. #781's remaining byte-preservation blocker is inherited. | Round-1 finding | Round-2 status | Does the test prove it? | |---|---|---| | P1 tab/list-contained code becomes events | **Verified** for the reported cases. | `review_code_and_unsafe_yaml_are_not_events` includes both exact code reproductions; Markdown code-block ranges now exclude them. | | P1 CRLF/BOM/malformed frontmatter becomes events | **Verified** for the reported envelopes. | The same test includes CRLF/BOM block-scalar clocks and an unclosed malformed envelope. Validation suppresses scanning, rather than claiming support for those envelopes. | | P1 commented/raw-HTML child relationships | **Not fixed for all comments**; standalone comment/block cases are fixed. | `review_relationships_share_visibility` proves standalone comment, div and list-code cases. It omits multiline inline comments; see below. | | P2 Unicode tags truncated | **Verified** for the reported Unicode cases. | `review_unicode_tags_are_whole` checks exact `café` and `area/工作` output; code advances by UTF-8 character lengths. This is not full parity with every editor tag form. | | P2 repeated nested child traversal | **Not fixed**, explicitly acknowledged. | No nested scaling regression was added. Each ancestor still scans its remaining subtree; the flat budget fixture does not prove a linear bound. | **New P1 — Multiline inline HTML comments still expose hidden relationships.** `crates/events/src/scan.rs:298` (the `Event::Html` exclusion match) and `:254` (child exclusion lookup). With: ```markdown - 09:00 Event <!-- - [ ] fake --> ``` `scan(..., Day::Fixed("2026-10-26"))` returns a child checkbox for `fake`. The same happens when `text <!--` is on an indented continuation line before the hidden checkbox. Pulldown emits inline HTML for these comments, but the added exclusions handle `Event::Html` and HTML blocks, not `Event::InlineHtml`; the old line state only notices comments starting a line. This emits a derived task relationship for text the person cannot see; in #785 the plain-checkbox identity scanner uses the same exclusions, so hidden checkboxes can also become identity targets when writes are enabled. **Fix:** include multiline inline-comment spans in the shared visibility model, filtering by the candidate token's actual span so visible text before/after inline HTML remains usable. Add both vectors, hidden clock/checkbox/link variants, and a real promotion test proving a commented owner link is ignored. This is a surviving class of the round-1 problem, not a newly introduced syntax category. Verification: dev-01 Nix shell, via `~/bin/slot`, `cargo test -p calternal-events review_ -j4` at stack head `771eb102` passed 17 tests (including the three parser regressions). A standalone Rust probe linked the current stack libraries and **observed** both inline-comment false relationships; the standalone-comment control had no child. The relevant exclusion/children helpers are unchanged from this PR. Probe/output: `/tmp/cal-r2-probe.rs`, `/tmp/cal-r2-probe.out`. No fresh release performance, full-suite or 100k cold-index result is claimed. Other round-1 audit limitations remain: quoted-checkbox support, unknown-box refusal, malformed/nested-owner semantics, independent journal-publication ordering, and large cold-index validation. Changing the old lazy-blockquote vector agrees with Markdown container rules; it does not prove these other cases.
kayg commented 2026-10-31 17:32:28 +00:00
Author
Owner
Copy link

round 2 fixes for final review

Head 510610d6, based on #781 94807faa. Simplification: mask invisible Markdown spans before scanning event, checkbox or relationship candidates, retaining byte and line offsets. Raw HTML-comment delimiters cover comments split by Markdown list boundaries; code spans/blocks keep comment delimiters literal. No merge.

Finding Status
#70966 P1 multiline inline comments expose hidden relationships Fixed. Both inline-opening and continuation-opening comments hide clocks, checkboxes and links, while visible text before/after remains usable.
Single-line comments, fenced code and YAML must be invisible review_round2_comments_code_yaml_are_invisible covers single/multiline HTML comments, top-level/nested fences and YAML. review_round2_inline_comments_never_create_tasks checks the actual tasks describe pass.
Identity scanner / commented owner link The next stack layer uses the same visibility view for plain boxes; its service regression promotes a checkbox with a commented owner link and creates the correct new task instead of treating that hidden link as ownership.

Passed on dev-01 in Nix, log ~/cal-r2-s3-final3.log:

cargo fmt --all -- --check
~/bin/slot cargo clippy -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --all-targets -j4 -- -D warnings
~/bin/slot cargo test -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --no-fail-fast -j4

Events: 18 passed; notes: 439 passed / 6 existing ignored; tasks: 30 passed; calternal unit/integration suites passed with existing ignored tests. No build/test ran on the control host; Nix scaffolding was not committed.

Visibility uses O(n + r log r) work for bytes/ranges. The inherited repeated nested child traversal is unchanged; no new large-note or cold-index performance claim is made. Malformed frontmatter suppresses scanning conservatively; the old visibility fixture now quotes its YAML string correctly. Existing round-1 audit/performance limits remain. Final review round 3 is pending; no review approval is claimed.

## round 2 fixes for final review Head `510610d6`, based on #781 `94807faa`. Simplification: mask invisible Markdown spans before scanning event, checkbox or relationship candidates, retaining byte and line offsets. Raw HTML-comment delimiters cover comments split by Markdown list boundaries; code spans/blocks keep comment delimiters literal. No merge. | Finding | Status | |---|---| | #70966 P1 multiline inline comments expose hidden relationships | Fixed. Both inline-opening and continuation-opening comments hide clocks, checkboxes and links, while visible text before/after remains usable. | | Single-line comments, fenced code and YAML must be invisible | `review_round2_comments_code_yaml_are_invisible` covers single/multiline HTML comments, top-level/nested fences and YAML. `review_round2_inline_comments_never_create_tasks` checks the actual tasks describe pass. | | Identity scanner / commented owner link | The next stack layer uses the same visibility view for plain boxes; its service regression promotes a checkbox with a commented owner link and creates the correct new task instead of treating that hidden link as ownership. | Passed on dev-01 in Nix, log `~/cal-r2-s3-final3.log`: ```sh cargo fmt --all -- --check ~/bin/slot cargo clippy -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --all-targets -j4 -- -D warnings ~/bin/slot cargo test -p calternal-events -p calternal-notes -p calternal-tasks -p calternal --no-fail-fast -j4 ``` Events: 18 passed; notes: 439 passed / 6 existing ignored; tasks: 30 passed; calternal unit/integration suites passed with existing ignored tests. No build/test ran on the control host; Nix scaffolding was not committed. Visibility uses O(n + r log r) work for bytes/ranges. The inherited repeated nested child traversal is unchanged; no new large-note or cold-index performance claim is made. Malformed frontmatter suppresses scanning conservatively; the old visibility fixture now quotes its YAML string correctly. Existing round-1 audit/performance limits remain. Final review round 3 is pending; no review approval is claimed.
kayg commented 2026-10-31 17:43:48 +00:00
Author
Owner
Copy link

Independent review, round 3 (final)

Verdict: stop and simplify. Reviewed 510610d6c84675741c1ea66344b829c273f56c4a, round-2 review #70966, fix status #71021, simplification notes and ADRs 0067/0068. No code pushed or merged. A new P1 remains in this final round; #781's new P0 also blocks this stack layer.

Round-2 item Final status / evidence
Inline-opening / continuation-opening multiline comments expose relationships Verified for both reported vectors. Real task describe regression also passes.
Single-line comments, fenced/list-contained code, YAML invisibility Reported fixtures pass, including malformed envelope refusal. Code invisibility is incomplete, see new P1.
Visible text before/after comments Existing round-2 regression passes.
Unicode tags Existing exact regression passes; no regression found.
Repeated nested child traversal Not fixed, still the acknowledged P2. Each parent can rescan its remaining subtree; no new scaling evidence.
Inherited property preservation Blocked by #781 round-3 P0: interior quote in a plain scalar hides a real YAML comment from the guard.

New P1: multiline inline code still produces calendar events. crates/events/src/scan.rs:299, :302. For a daily note whose exact bytes are "\n10:00 hidden\n\n" (one backtick on the first and third lines), CommonMark treats the clock as inline code. visible_text returns those bytes unchanged and scan(text, Day::Fixed("2026-10-26")) emits a 10:00 event on line 1. The list-opening variant "- \n10:00 hidden\n\n" also emits it. The implementation records Event::Code only in code, used to ignore literal HTML-comment delimiters, but omits it from the actual exclusion ranges. Fenced/indented-code tests cannot catch this. This feeds false derived events through the production parser; marker generation remains release-gated in #785, so no enabled marker-writing claim is made.

Concrete simpler design / fix: construct one immutable exclusion view from original Markdown offsets, including inline code as well as code blocks, HTML/comments and YAML. For an initial conservative contract, skip any candidate/relationship line intersecting an excluded span; leave partially mixed lines to source editing rather than reparsing space-masked Markdown through a second parser and another fence state. Use that same candidate test for events, tasks and identity. Add complete multiline code-span fixtures with one/multiple backticks, clock-only and nested child variants, and check both derived rows and refusal of identity targets. Retain writes disabled until the common visibility contract is proven. This deliberately narrows mixed-line support instead of starting a fourth syntax-patch review round.

Fresh verification on dev-01, Nix shell, stack head eb7ffd2d (the implicated exclusion implementation is unchanged from this PR), heavy commands via ~/bin/slot:

  • cargo test -p calternal-events -p calternal-notes -p calternal-tasks review_ -j4: events 21, notes 25, tasks 7 passed, including all existing round-2 visibility regressions.
  • Independent standalone probe compiled with ~/bin/slot rustc --edition=2024 /tmp/cal-r3-probe.rs -L dependency=target/debug/deps --extern calternal_events="$events_lib" -o /tmp/cal-r3-probe, then executed. Observed unchanged visibility bytes and a false event for both multiline inline-code examples. $events_lib was the newest built events rlib. VM source/output: /tmp/cal-r3-probe.rs, /tmp/cal-r3-probe.out; test log /tmp/cal-r3-review-tests.log.

No fresh full-suite, clippy, release-perf, 100k index or browser result is claimed. Inherited round-1 audit limits remain; no additional new P2/P3 finding.

# Independent review, round 3 (final) **Verdict: stop and simplify.** Reviewed `510610d6c84675741c1ea66344b829c273f56c4a`, round-2 review #70966, fix status #71021, simplification notes and ADRs 0067/0068. No code pushed or merged. A new P1 remains in this final round; #781's new P0 also blocks this stack layer. | Round-2 item | Final status / evidence | |---|---| | Inline-opening / continuation-opening multiline comments expose relationships | Verified for both reported vectors. Real task describe regression also passes. | | Single-line comments, fenced/list-contained code, YAML invisibility | Reported fixtures pass, including malformed envelope refusal. **Code invisibility is incomplete**, see new P1. | | Visible text before/after comments | Existing round-2 regression passes. | | Unicode tags | Existing exact regression passes; no regression found. | | Repeated nested child traversal | **Not fixed**, still the acknowledged P2. Each parent can rescan its remaining subtree; no new scaling evidence. | | Inherited property preservation | **Blocked** by #781 round-3 P0: interior quote in a plain scalar hides a real YAML comment from the guard. | **New P1: multiline inline code still produces calendar events.** `crates/events/src/scan.rs:299`, `:302`. For a daily note whose exact bytes are `"`\n10:00 hidden\n`\n"` (one backtick on the first and third lines), CommonMark treats the clock as inline code. `visible_text` returns those bytes unchanged and `scan(text, Day::Fixed("2026-10-26"))` emits a 10:00 event on line 1. The list-opening variant `"- `\n10:00 hidden\n`\n"` also emits it. The implementation records `Event::Code` only in `code`, used to ignore literal HTML-comment delimiters, but omits it from the actual exclusion `ranges`. Fenced/indented-code tests cannot catch this. This feeds false derived events through the production parser; marker generation remains release-gated in #785, so no enabled marker-writing claim is made. **Concrete simpler design / fix:** construct one immutable exclusion view from original Markdown offsets, including inline code as well as code blocks, HTML/comments and YAML. For an initial conservative contract, skip any candidate/relationship line intersecting an excluded span; leave partially mixed lines to source editing rather than reparsing space-masked Markdown through a second parser and another fence state. Use that same candidate test for events, tasks and identity. Add complete multiline code-span fixtures with one/multiple backticks, clock-only and nested child variants, and check both derived rows and refusal of identity targets. Retain writes disabled until the common visibility contract is proven. This deliberately narrows mixed-line support instead of starting a fourth syntax-patch review round. Fresh verification on dev-01, Nix shell, stack head `eb7ffd2d` (the implicated exclusion implementation is unchanged from this PR), heavy commands via `~/bin/slot`: - `cargo test -p calternal-events -p calternal-notes -p calternal-tasks review_ -j4`: events 21, notes 25, tasks 7 passed, including all existing round-2 visibility regressions. - Independent standalone probe compiled with `~/bin/slot rustc --edition=2024 /tmp/cal-r3-probe.rs -L dependency=target/debug/deps --extern calternal_events="$events_lib" -o /tmp/cal-r3-probe`, then executed. Observed unchanged visibility bytes and a false event for both multiline inline-code examples. `$events_lib` was the newest built events rlib. VM source/output: `/tmp/cal-r3-probe.rs`, `/tmp/cal-r3-probe.out`; test log `/tmp/cal-r3-review-tests.log`. No fresh full-suite, clippy, release-perf, 100k index or browser result is claimed. Inherited round-1 audit limits remain; no additional new P2/P3 finding.
kayg commented 2026-10-31 17:50:34 +00:00
Author
Owner
Copy link

On hold under the three-round stop-and-simplify rule. The replacement plan is PR #802: #802

Plan: https://git.kayg.org/kayg/calternal/src/branch/docs/calendar-backend-v2-plan/docs/plans/0067-calendar-backend-v2.md

No further patches or fourth review round on this stack. The plan chooses a strict whole-frontmatter write allowlist, the editor's Lezer parser with an explicit backend bridge feasibility gate, and centralized marker writes disabled until full UI hiding. It lists salvage, independent tests and smaller replacement PRs. This comment changes no implementation code and requests no merge.

On hold under the three-round stop-and-simplify rule. The replacement plan is PR #802: https://git.kayg.org/kayg/calternal/pulls/802 Plan: https://git.kayg.org/kayg/calternal/src/branch/docs/calendar-backend-v2-plan/docs/plans/0067-calendar-backend-v2.md No further patches or fourth review round on this stack. The plan chooses a strict whole-frontmatter write allowlist, the editor's Lezer parser with an explicit backend bridge feasibility gate, and centralized marker writes disabled until full UI hiding. It lists salvage, independent tests and smaller replacement PRs. This comment changes no implementation code and requests no merge.
kayg commented 2026-11-07 19:10:06 +00:00
Author
Owner
Copy link

housekeeping #1009: needs-decision.

Calendar v1 parser/describe slice was held with #781/#785; #802 and pure converter #803 are the simpler replacement direction. Retain any uncovered parser vectors.

original head: 510610d6c84675741c1ea66344b829c273f56c4a. against dev cd7f84432febfc63b788fbb9b9f9950452982d74, git cherry reports 23 unmatched and 0 equivalent patches. the landing audit lists each candidate missing commit and stable patch match in pr-783.md; unmatched patches can include superseded behavior.

left open, source branch unchanged. no rebase or new test claim for this decision lane.

housekeeping #1009: needs-decision. Calendar v1 parser/describe slice was held with #781/#785; #802 and pure converter #803 are the simpler replacement direction. Retain any uncovered parser vectors. original head: `510610d6c84675741c1ea66344b829c273f56c4a`. against dev `cd7f84432febfc63b788fbb9b9f9950452982d74`, git cherry reports 23 unmatched and 0 equivalent patches. the [landing audit](https://git.kayg.org/attachments/fe3c09dc-dea3-47a0-92e2-b44a777083e5) lists each candidate missing commit and stable patch match in `pr-783.md`; unmatched patches can include superseded behavior. left open, source branch unchanged. no rebase or new test claim for this decision lane.
kayg commented 2026-11-07 21:21:18 +00:00
Author
Owner
Copy link

Superseded by calendar backend v2 (#802/#803). The branch and original tips are archived under archive/ (see #1009), so nothing is lost; useful regression vectors are noted there for the active lanes. Closing.

Superseded by calendar backend v2 (#802/#803). The branch and original tips are archived under `archive/` (see #1009), so nothing is lost; useful regression vectors are noted there for the active lanes. Closing.
kayg closed this pull request 2026-11-07 21:21:18 +00:00
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
backend
Rust server

  
design
Mockups, UI and TASTE

  
docs
ADRs, CONTEXT, TASTE

  
idea
Parked idea, never worked until promoted

  
infra
Deploy, CI, VMs

  
perf
Performance and budgets

  
reliability
Data integrity, corruption traps

  
web
Web client

No labels
backend
design
docs
idea
infra
perf
reliability
web
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal!783
No description provided.