Define mpsc-log design and roadmap - #6
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
WalkthroughAdds the mpsc-log product boundary, shared terminology, detailed journal design, architectural decisions, event schema, sidecar example, phased roadmap, documentation index links, and typo configuration for Changesmpsc-log Documentation Suite
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 18 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (18 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
|
@coderabbitai Have the following now been resolved? docs/mpsc-log-design.md (2)
🛠️ Suggested edit- | `weekly` | UTC ISO week beginning, Monday 00:00:00 | `run.2026-06-29.jsonl` |
+ | `weekly` | UTC ISO week beginning, Monday 00:00:00 | `run.<week-start>.jsonl` |🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In
🛠️ Suggested edit- The first integration target is the df12-build Open Dynamic Workflows (ODW) workflow at `../df12-build.worktrees/codex-annex/workflows/df12-build-odw.js`.
+ The first integration target is the df12-build Open Dynamic Workflows (ODW) workflow.🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Lines 20-35 ---'
sed -n '20,35p' docs/mpsc-log-design.md
printf '%s\n' '--- Lines 285-302 ---'
sed -n '285,302p' docs/mpsc-log-design.md
printf '%s\n' '--- Relevant matches ---'
rg -n -C 2 --fixed-strings \
-e '../df12-build.worktrees/' \
-e 'first integration target' \
-e '| `weekly`' \
-e 'run.<week-start>.jsonl' \
-e 'run.2026-06-29.jsonl' \
docs/mpsc-log-design.md || trueLength of output: 3170
[resolve] |
81c45e0 to
a396136
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Roadmap item 3.3.2 ---'
rg -n -A26 -B6 '3\.3\.2\.' docs/roadmap.md || true
printf '%s\n' '--- Required duplicate-path criteria in docs/roadmap.md ---'
rg -n -i -C 3 \
-e 'parameteri[sz]' \
-e 'duplicate (write|path|key)|same object path' \
-e 'top-level (key|keys)' \
-e 'nested object path' \
-e 'sidecar default' \
-e '`-s`|`-n`|`-b`' \
-e 'later write|last-wins|replace.*earlier' \
docs/roadmap.md || true
printf '%s\n' '--- Current diff for docs/roadmap.md ---'
base="$(git merge-base HEAD origin/main 2>/dev/null || true)"
if [ -n "$base" ]; then
git diff --unified=5 "$base" HEAD -- docs/roadmap.md
else
git diff --unified=5 HEAD~1 HEAD -- docs/roadmap.md
fi
printf '%s\n' '--- Available documentation validation targets ---'
for file in AGENTS.md Makefile Justfile package.json Cargo.toml; do
if [ -f "$file" ]; then
printf '\n### %s ###\n' "$file"
rg -n -i -C 2 '^[[:alnum:]_.-]*(fmt|markdownlint|spelling|typos|docs)[[:alnum:]_.-]*:|markdownlint|spelling|typos' "$file" || true
fi
doneLength of output: 42019
Run You are interacting with an AI system. |
Add the initial terms of reference, technical design, glossary, event schema, sidecar example, and delivery roadmap for `mpsc-log`. Together these documents establish the CLI contract, JSONL record shape, sidecar configuration, concurrency model, rotation policy, validation strategy, and df12-build telemetry adoption path. Update the documentation index so reviewers can find the new artefacts. Fix the generated doctest import to use the Rust crate identifier so the workspace test gate passes.
Add ADR 001 to define how the repository's required testing prongs apply to `mpsc-log`'s CLI, record-building, filesystem, concurrency, repair, rotation, and telemetry contracts. Reference the ADR from the technical design, documentation index, and roadmap so implementation tasks inherit the testing strategy rather than treating it as an isolated testing phase.
Run `make fmt` across the repository and keep the resulting Markdown wrapping changes. Reshape the long companion-document link lists so the formatter target can complete without reintroducing markdownlint line-length failures.
Resolve the still-valid review findings in the design documentation. Spell out record merge and coercion precedence, restrict schema timestamps to canonical UTC form, align sidecar schema examples with integer event fields, and tighten roadmap success criteria for merge precedence and df12-build fixture coverage.
Record lock file naming as ADR 001 because concurrency and rotation safety depend on every writer deriving the same coordination artefact. Rename the testing strategy ADR to ADR 002, update the design and roadmap references, and mark the lock naming roadmap task as completed.
Add a roadmap task to benchmark flate2, gzp, and gzippy before selecting the gzip backend for rotated journal compression. Clarify that flate2 remains the conservative design baseline until measured throughput, atomic-output integration, dependency risk, and portability justify a faster backend.
Add ADR 003 to state that `mpsc-log` is `jo`-inspired rather than textually compatible with `jo` output. Document last-wins duplicate-path semantics, update the design and terms of reference to stop implying full `jo` compatibility, and mark the roadmap syntax decision as complete.
Clarify that partial-tail repair only applies to unterminated final bytes and that newline-terminated corrupt final records fail closed with `EX_DATAERR` without truncation. Update the roadmap, testing strategy, glossary, sidecar example, and terms of reference so implementation and operator guidance cover this case explicitly.
Specify that scheduled rotation finalizes the previous period before checking `max_bytes` for the pending record against the fresh active file. Remove the brittle df12-build repository-relative path from the terms of reference and correct the plural wording for the document title.
Clarify df12-build workflow wording and make the workflow phase list parallel. Use a week-specific scheduled rotation filename example.
Record the crate name in the repository spelling overlay and generated configuration so design documentation passes the Markdown spelling gate.
Keep the documentation index compliant with the Markdown blank-line rule.
Require complete telemetry for each initial df12-build event, validate timestamp overrides before append, and avoid scheduled archive collisions. Align the roadmap and terms of reference with these established contracts.
Address review feedback on the design and roadmap documents: - Add a comma after "Otherwise" in the scheduled-rollover final-archive naming sentence in docs/mpsc-log-design.md. - Expand roadmap item 2.2.1 to specify extension-replacement sidecar derivation (run, run.jsonl, and run.ndjson derive run.toml) and add an acceptance criterion plus tests rejecting .toml journal filenames whose derived sidecar path equals the journal path, while preserving the existing defaults and error-code requirements. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address a second round of review feedback on the design, roadmap, and terms of reference: - Guard the scheduled size-split step so an empty active file is never rotated into a spurious empty archive: when the active file holds no data, append the oversized pending record directly and only rotate a data-bearing active file into a collision-free archive name. - Expand roadmap item 2.2.3 to require validating defaults- and CLI-supplied timestamp values against the RFC 3339 UTC contract before append, preserve valid overrides, fail invalid overrides with EX_DATAERR, and generate an invocation-time timestamp only when no valid one exists. - Scope roadmap item 3.1.2's serialization guarantee to supported local filesystems and defer network-filesystem (NFS and CIFS) support to the named filesystem verification matrix in step 6.2.1, preserving the lock naming, contention, and EX_TEMPFAIL claims. - Clarify in the terms of reference that rotation and compression retention define local generation retention, and scope the non-goal to centralized or downstream retention policy so local retention is clearly in scope. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Refine roadmap item 2.2.3 so an invocation-time RFC 3339 UTC timestamp is generated only when no `timestamp` override exists, matching the write protocol's step 5. The previous "when no valid timestamp already exists" wording could be read as generating a timestamp after an invalid override, whereas an invalid default or CLI value fails with EX_DATAERR before append. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback across the design, roadmap, and terms of reference: - Present the size-only rotation threshold and plain/compressed generation counts in docs/mpsc-log-design.md as the `[rotation]` sidecar defaults (`max_bytes`, `plain_generations`, `compressed_generations`) that callers can override, and use the configured plain and compressed retention counts when describing scheduled-period retention. - Expand roadmap items 4.3.2 and 4.3.3 with observable success conditions: period-boundary rollover runs before the size check, collision-free suffix selection when archives already exist, and appending an oversized record directly to an empty active journal without creating an empty archive. - Reframe the terms-of-reference retention goals so four plain generations and four completed periods read as configurable defaults rather than universal limits, and align the compression hard-constraint bullet. - Standardise the terms of reference on "journal" and "journal path" for the target file and its argument, preserving the JSON Lines (JSONL) format name, the glossary log terms, and the distinct workflow sidecar directory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback on the rotation normative steps and sidecar naming: - Rewrite the size-only rotation algorithm in docs/mpsc-log-design.md so every generation index derives from the configured plain generation count P (`plain_generations`) and compressed generation count C (`compressed_generations`) rather than the hard-coded 36, 5, 4, 3, 2, and 1. Define both counts as non-negative integers (negatives rejected with EX_CONFIG) and specify the zero-count behaviour: P = 0 gzips the active file straight into generation 1, C = 0 deletes the oldest plain generation instead of gzipping, and both zero retains only the active file. Deletion, renaming, compression, and retention now target the computed generations. - Clarify the terms-of-reference sidecar filename description to state that the sidecar path replaces the journal's final extension with `.toml`, appending `.toml` only when the journal has no extension, replacing the ambiguous "same base filename" wording. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The size-only rotation rules allowed plain_generations and compressed_generations to both be zero, in which case rotation discarded the previous journal. That state transition cannot be made atomic and contradicts the terms-of-reference constraint that a failed invocation leaves the previous journal readable. - Require `P + C >= 1` in docs/mpsc-log-design.md, so semantic validation rejects zero total retention with EX_CONFIG alongside negative counts, and explain why discarding the rotated journal is a configuration error rather than a supported mode. A single zero count remains valid. - Make step 5 state that the previous contents always reach generation 1 before any append: a rename when P >= 1, and otherwise a gzip whose active file is removed only after the output is committed, so a failed compression aborts with the previous journal intact. - Note in step 6 that an append failure therefore leaves the previous contents readable in the rotated journal. - Add a roadmap 2.2.1 acceptance criterion covering both rejected count configurations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Size-only rotation deleted generation P + C as its first act, so a later failure in the rename or gzip sequence had already destroyed the oldest archive and left a partially shifted layout. That also contradicted roadmap item 4.2.3, which requires eviction only after newer retained files are safely in place. - Stage the eviction instead of deleting it: rename generation P + C aside to a reserved staging name in the same directory, so the rename stays within one filesystem and is atomic. - Unlink the staged eviction only in a new final commit step, after the pending record has been appended, so no failure path deletes a record-bearing file. - State the failure guarantee explicitly: steps 1 to 5 are atomic renames and atomic-write-file commits, so a failure aborts before the append and unwinds the completed renames in reverse order to the prior generation layout, and a process killed mid-rotation leaves a staged eviction that the next invocation reclaims under the journal lock. - Update step 3's C = 0 note, which previously referred to the deletion. - Add a roadmap 4.2.3 acceptance criterion for staging, commit-point unlinking, and staged-eviction reclaim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 3 removed plain generation P once its gzip was committed, so the claimed unwind "back to the prior generation layout" could not hold: a reverse rename cannot restore a removed plain source. Interruption also had no restart rule beyond reclaiming the staged eviction, leaving a partially shifted layout unspecified. - Defer every removal to the commit point. Superseded sources, including the gzipped plain generation P and the active file when P is zero, are renamed aside to staging names instead of being unlinked, so the prepare phase only creates and renames files and every source generation stays recoverable. - Record the planned layout transition in a rotation manifest written through atomic-write-file before any file is touched, and remove it at the commit point, so its presence means a rotation was interrupted. - State that each prepare step is idempotent, applied only when its source exists and its target does not, so replaying the manifest can neither duplicate nor skip a generation. - Specify the three failure paths so rotation always settles into either the pre-rotation or the post-rotation layout: a prepare error reverses the applied renames, an append error still commits because the rotation itself completed, and a killed process leaves the manifest for the next invocation to complete or reverse under the journal lock before handling its own record. - Expand roadmap item 4.2.3 to fault-inject each rotation commit point and assert recovery: partial renames, uncommitted and committed gzip output, staged source removal, append failure, and process interruption. Steps 1 to 7 are now one unbroken list so the phase split no longer trips MD029; the phase boundary is stated in the lead-in instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Address four review findings across the design, roadmap, and lock-naming ADR. Lock timeout. The timeout was documented as sidecar-configurable while the lock had to be held before the sidecar could be read, so no document said which timeout governed the only acquisition attempt. Resolve it with an unlocked advisory pre-read: the single attempt uses a five-second default that a pre-read of `[locking] timeout_ms` may override, the pre-read only chooses how long this invocation waits and never feeds repair, rotation, coercion, or defaults, an absent or invalid pre-read falls back to five seconds, and no second attempt is made. Apply the same wording to the design architecture and write-protocol sections, the sidecar table, the critical -section diagram, roadmap items 3.1.2 and 3.1.3, and ADR 001, whose ordering claims now refer to the authoritative sidecar read. Append commit point. The writer only defined rollback for a `write_all` error, although the exit-code table maps flush and filesystem metadata failures to EX_IOERR. Define a record as committed once its bytes and terminating newline are written and flushed, extend the restore-to-recorded -length rollback to every in-process failure after bytes reach the file, state that a failed rollback truncate still returns EX_IOERR and leaves the tail for partial-tail repair, and record that a retry after EX_IOERR cannot duplicate a committed record. Note the residual at-least-once caveat when a process dies between the write and the exit. Expand roadmap items 3.2.1 and 3.2.3 to fault-inject those paths. Size-only rotation predicate. The size-only subsection named `max_bytes` but never said when rotation triggers. State that rotation is evaluated after tail repair and before the append, compares the repaired active length plus the serialized pending record against `max_bytes`, and rotates only a non-empty active file, so an oversized record appends directly to an empty active file. The scheduled subsection already carried this rule. Non-goals. Record that `mpsc-log` is `jo`-inspired rather than `jo` compatible, cross-referencing the CLI contract and ADR 003 rather than duplicating the option list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The append contract contradicted itself. It assumed a failed rollback truncate leaves an unterminated tail, then concluded that a retry after EX_IOERR could not duplicate a committed record. Neither holds when write_all completes, flush fails, and the rollback truncate then fails: the active file ends with a complete newline-terminated record, which the repair rules preserve rather than remove, so a retry appends a second copy. - State that a failed rollback truncate leaves the commit status unknown, that an unterminated tail is removed by partial-tail repair while a complete newline-terminated record is preserved and therefore committed, and that the invocation returns EX_IOERR without knowing which occurred. - Replace the no-duplication guarantee with at-least-once whenever the writer cannot confirm the outcome, covering both the failed rollback truncate and process death between the write and the exit. - Record that `mpsc-log` keeps no unknown-commit reconciliation state, so callers needing exactly-once must carry an idempotency key and deduplicate when reading. - Reword roadmap items 3.2.1 and 3.2.3 so the acceptance criteria allow the duplication instead of asserting it cannot happen, and assert it as the documented outcome rather than a defect. The "exactly one record per successful invocation" claims elsewhere are unaffected, since at-least-once applies only to unconfirmed outcomes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The design put `serde_json::Map` in the domain core, so the record model was defined by a serialization crate and the module table did not say which modules were domain and which were adapters. Contributors had no documented architecture to place new code against. Design: - Replace the `serde_json::Map` core with domain `Record` and `Value` types. Add section 4.1 defining `Value` variants and the required behaviour for object paths, nested objects, arrays, scalars, coercion results, and last-wins replacement. - Add section 4.2 declaring the `Clock` and `JournalStore` ports, and state that no domain module names a filesystem, locking, TOML, JSON, or `sysexits` API. - Add section 4.3 defining the adapters, including the TOML adapter that converts sidecar defaults and schema data into domain types at the input boundary and the JSON adapter that emits one compact JSON object at the output boundary. - Give the section 12 module table a Layer column, and record that JSON output serialization and process exit mapping are boundary concerns rather than domain modules. - Reword the merge table and inference prose so sidecar defaults convert into domain `Value` equivalents rather than JSON equivalents. Developer guide: add an implementation architecture section covering the eight planned modules and their layers, the `src/main.rs` startup and `sysexits` responsibility, the adapter boundary and its concerns, the two ports, and links to the design and ADRs 001, 002, and 003. ADR 003: replace the `serde_json::Map` mandate with the domain `Record` object map of `Value` values serialized by the JSON adapter, keeping the accepted last-wins semantics and the textual duplicate-name non-goal. Contents: note that the developer guide now covers the architecture. Compact JSON output and last-wins duplicate handling are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The architecture prose, module table, and ADR already described the domain, its `Record` and `Value` types, and the `Clock` and `JournalStore` ports, but Figure 1 still drew the earlier topology: CLI to argument parser and sidecar loader, into a record builder, into a journal writer. A reader of the architecture section saw no domain, port, or adapter boundary in the diagram. Redraw Figure 1 with input adapters, the domain, the ports, and the output adapters as separate groups, showing the domain reaching the outside world only through `Clock` and `JournalStore`, the filesystem adapter implementing `JournalStore`, and `main.rs` owning the `sysexits` mapping. Extend the caption to state the boundary in prose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ADR 003 pairs the accepted last-wins decision with parameterized tests over top-level keys, nested object paths, sidecar defaults, and explicit coercion flags, but roadmap item 3.3.2 listed only the pairwise axes, so nothing in the delivery plan required that coverage. Add a success criterion to 3.3.2 requiring parameterized duplicate-path tests for duplicate writes at the same object path, across top-level keys, nested object paths, a sidecar default overridden by a CLI field, and the explicit `-s`, `-n`, and `-b` coercion flags, asserting that the later write replaces the earlier value at that object path. The existing pairwise axes criterion is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main is now a fixed point of `make fmt`, so bring this branch's own files to the same state instead of reverting the formatter on every run. docs/adr-001-lock-file-naming.md and docs/terms-of-reference.md are files this branch adds, and their mdformat drift was this branch's own. The rebase onto the reformatted main also left four stray blank lines before headings in docs/developers-guide.md, which mdformat removes. Formatting only: whitespace-normalised content is unchanged in all three files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
c63e97c to
810af68
Compare
Summary
This branch defines the initial product and delivery plan for
mpsc-logsoreviewers have a concrete contract before implementation begins. It adds the
terms of reference, technical design, lock-file naming ADR, selected
jofieldsyntax ADR, testing strategy ADR, glossary, event schema, sidecar example, and
GIST-aligned roadmap for the CLI, then indexes those artefacts from the
documentation contents page.
It also tracks the generated Cargo lockfile for this binary crate, corrects the
crate-name doctest import, applies the repository
make fmtoutput, andaddresses review feedback on merge precedence, UTC timestamp validation,
integer schema coercions, df12-build fixture coverage, lock naming, duplicate
key compatibility language, partial-tail versus corrupt-final-line handling,
scheduled rollover ordering, ToR wording, and compression backend exploration.
No issue, roadmap task, or execplan is associated with this branch. The branch
creates the initial roadmap rather than implementing a roadmap task.
Review walkthrough
jo-inspired syntax boundary, generalized df12-build use-case wording, and open questions..lockjournal suffix, rejected self-sidecar paths, and same-stem sidecar sharing.jo-inspired compatibility boundary and last-wins duplicate-path decision.repair_partial_tailscope note.josyntax ADR work, remaining ADRs, day-one JSONL append, contention safety, repair validation, rotation, theflate2/gzp/gzippycompression benchmark task, df12-build adoption, and deferred extensions.Validation
make fmt: passed.make check-fmt: passed.make markdownlint: passed.make nixie: passed.jq empty docs/mpsc-log-event-schema.json: passed.make lint: passed.make test: passed.main...docs/mpsc-log-design-roadmap: ten commits ahead, eighteen files changed.Notes
This branch establishes the design and roadmap before implementation. The
roadmap explicitly leaves query tooling, dashboards, network-filesystem
guarantees, full
joparity, public Rust library API support, andmax_ageretention out of the v1 implementation path unless a later ADR changes the
product boundary.
References
Summary by Sourcery
Establish the design contracts and implementation roadmap for
mpsc-logbefore feature development begins.New Features:
mpsc-logproduct contract, technical architecture, JSON event schema, TOML sidecar format, and implementation roadmap.jo-inspired object-field syntax with deterministic last-wins duplicate-path semantics.Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: