[pmoust]

ADR-3492 decision 7, the ravel-logseg half. RlogWriter::finish_on and
finish_with_stats_on hand a block's per-column page encodes to a
PageEncodeExecutor as Send + 'static jobs that own their inputs, so a
caller can run them on a blocking pool. The writer assembles the results by
job index into the same object finish builds: a property test, the
logseg_encode shape, a multi-block row-group test and the golden v5 fixture
pin byte identity for both page choices with jobs completing out of order.
A block with fewer than PAGE_JOB_FLOOR (8) column pages stays one job.
finish is unchanged.

Each result carries its block id and column ids. assemble_pages refuses,
with LogSegError::PageEncode, a result from another block, one whose
columns differ from its job's, a repeated or out-of-range index, a wrong
result count, or an empty slot; a stager that seals a page itself on the
jobs path is refused too. Each check has a test shown failing against the
version that breaks it.

Byte identity rests on page order within each column and on the row-group
dictionary decision running once per column; the order of a block's column
jobs never reaches the object, because a row group lays pages out by column
id.

On the 7.5k-record flush shape, four threads cut the encode by 8 to 12%
(181.7 ms to 169.9 ms, columnar writer, estimated page choice); most of a
flush is still serial. Nothing calls finish_on yet: the ingest flush and
the write gate wiring follow under #3499.

Also: insert_text uses tokenizer::for_each_token, so building the bloom
filter no longer allocates per token (36,000 tokens: 41,000 allocations
before, 0 after, filter bytes unchanged); the format doc says the floor and
zstd envelope apply to every stored page under both page choices; and
i64_raw_sizes moves into the test module.

Refs: #3499
Refs: #3492

[pmoust]

@claude-fleet review

[claude-fleet]

Review task queued.

Task: d431944f-d953-47d9-bc88-c2e91b81c82f
Commit: 64d9abf
Model: claude-opus-5 (effort medium)
Executor: rp2
Status: done. Review posted.

[claude-fleet]

Reviewed the full diff, read block.rs, writer.rs, the shape functions in rlog_codec.rs, and ADR-3492 D7, and ran the new tests plus two mutation probes.

The design holds up: every staging entry point (single, i64, strings, dict_strings, and observed_ts through them) is intercepted in deferred mode, nothing else calls Stager::column, so deferred_jobs's "staged outside the jobs" refusal cannot fire on a legitimate input. Byte identity rests on compact_dict preserving the per-row value sequence, and both dict shape functions (rlog_codec.rs:564, :642) derive their output from the referenced values only, so compaction is invisible to them. One should_fix: that is the one input kind no test ever runs.

Request changes: one should_fix finding.

Findings:

• should_fix, crates/ravel-logseg/src/block.rs:460 — compact_dict / OwnedInput::DictStrings has no test coverage at all

Verified:

• Assembly really is in staging order, and the unit tests pin it: reversing the slot loop (block.rs:705) fails jobs_returned_in_reverse_assemble_in_staging_order, synthetic_results_in_any_order_assemble_in_job_order and results_of_another_block_with_the_same_jobs_are_refused.
• The integration byte-identity tests are insensitive to that same reversal, which is exactly the reason the description gives (a row group lays pages out by column id), so the layering is right, not vacuous.
• compact_dict is byte-identity preserving: string_dict_shape (rlog_codec.rs:564-589) sorts and dedups by value over referenced ids only, and string_dict_shape_if_dict (rlog_codec.rs:642-657) counts distinct values into a HashSet in first-reference order — compaction changes neither sequence.
• for_each_token is behaviourally identical to tokens: tokens is implemented as for_each_token(text, |tok| out.push(tok.to_vec())) (ravel-codec/src/tokenizer.rs:27-29).
• i64_raw_sizes has no caller outside the property test it moved next to (only hits are rlog_codec.rs:823, :1027, :1032).
• PAGE_JOB_FLOOR = 8 against the seven always-staged value pages (block.rs:972-986) and against ADR-3492 D7's "a page-count floor stated in the code" (docs/adrs/3492-ingest-cpu-per-log-line.md:245-250).
• The format-doc edit at docs/log-segment-format.md:885 is accurate: seal_page (page.rs:70-88) applies the floor and zstd envelope on every path, both page choices.

Checks run:

• cargo test -p ravel-logseg --test parallel_page_encode → 8 passed
• cargo test -p ravel-logseg --test golden_bytes_v5 --test bloom_insert_alloc_pin → 5 passed
• cargo test -p ravel-logseg --lib block::tests → 41 passed
• Reverted: slots.into_iter().enumerate() → .rev() at block.rs:705 → 3 lib tests fail, integration tests still green (see Verified). Restored; tree clean.
• Probe: panic!() as the first statement of compact_dict → parallel_page_encode (8 tests, property test included) and golden_bytes_v5 both still pass. Restored.

Smaller things:

• block.rs:707: job {index} never returned is unreachable. With done.len() == jobs enforced, every index in range, and duplicates refused, the slots are full by pigeonhole. The description lists "an empty slot" among the refused cases with "a test shown failing against the version that breaks it"; there is no such test and no input can produce one. Either drop the branch or say in the comment that it is a belt-and-braces check.
• parallel_page_encode.rs:109 and golden_bytes_v5.rs:197 assert out_of_order_calls == 1 exactly. That is a scheduling property of ScopedPool: workers pop from the tail, so results almost always come back out of job order, but nothing makes it certain. > 0 on the multi-block test already carries the intent; the == 1 form can only ever flake, never find a defect.
• bloom_insert_alloc_pin.rs:73 hardcodes 64 for the private EXACT_BLOOM_MAX (writer.rs:238). If that constant moves, the reference filter silently stops matching what insert_text builds and the failure points at the wrong thing.
• PageEncodeJob::page_count is public with no caller outside job_shape in the test; presumably the #3499 gate wiring needs it. Fine to land ahead of its caller, just noting it.

Not checked:

• The 8-12% encode figure (181.7 ms to 169.9 ms). I did not run logseg_encode; this host is a 4-core aarch64 box and would not reproduce the quoted basis.
• benches/logseg_encode.rs arm wiring beyond reading the diff (benches do not build under cargo test).

Not checked:

• The 8-12% encode speedup (181.7 ms to 169.9 ms): benches/logseg_encode.rs was not run; this host is 4-core aarch64 and would not reproduce the quoted basis.
• benches/logseg_encode.rs beyond reading the diff (bench targets are not built by cargo test).

Task d431944f-d953-47d9-bc88-c2e91b81c82f on rp2, model claude-opus-5, effort medium.

[claude-fleet]

should_fix: compact_dict and the OwnedInput::DictStrings branch are never executed by any test, so the one deferred input kind that rewrites its input is the one nothing pins.
Failure: a remap bug in compact_dict (ids pointing at the wrong kept entry, order not preserved) makes finish_on write a different object than finish for any batch carrying dyn_col_dicts, breaking the byte-identity contract this PR exists to establish and the deterministic-encode assumption compaction's probe-byte reuse rests on. Every test in the PR stays green.
Evidence: I replaced the body of compact_dict with panic!() and ran cargo test -p ravel-logseg --test parallel_page_encode (8 passed) and --test golden_bytes_v5 (4 passed, 1 ignored); nothing panicked. finish_on has exactly three users repo-wide (tests/parallel_page_encode.rs, tests/golden_bytes_v5.rs, benches/logseg_encode.rs), all of which build batches with ColumnarLogBatch::try_from_records, which leaves dyn_col_dicts empty, so block.rs:1299-1303 takes the non-dict branch and writer.rs:1872 produces None for every plan. The block.rs unit tests drive stage_rows only, which has no dict path.
Fix: add one case that calls .with_dictionaries() on the batch before push_columnar (the bridge columnar_batch.rs:1216 documents for exactly this, used by tests/rlog_v5_rowgroup_dict.rs:355) and asserts finish_on == finish, ideally also in the proptest arm.

[pmoust]

@claude-fleet review

[claude-fleet]

Review task queued.

Task: 229f6779-0ce5-40ab-9b54-1f8daac32757
Commit: 7042db4
Model: claude-opus-5 (effort medium)
Executor: rp2
Status: done. Review posted.

[claude-fleet]

Previous round at 64d9abf: 1 finding.

Previous finding	Classification	Cited line
crates/ravel-logseg/src/block.rs:460 (should_fix): compact_dict and the OwnedInput::DictStrings branch are never executed by any test, so the one deferred input kind that rewrites its input is the one nothing pins. Failure: a remap bug in compact_dict (ids pointing at the wrong kept entry, order not preserved) makes finish_on write a different object than finish for any batch carrying dyn_col_dicts, breaking the byte-identity contract this PR exists to establish and the deterministic-encode assumption compaction's probe-byte reuse rests on. Every test in the PR stays green. Evidence: I replaced the body of compact_dict with panic!() and ran cargo test -p ravel-logseg --test parallel_page_encode (8 passed) and --test golden_bytes_v5 (4 passed, 1 ignored); nothing panicked. finish_on has exactly three users repo-wide (tests/parallel_page_encode.rs, tests/golden_bytes_v5.rs, benches/logseg_encode.rs), all of which build batches with ColumnarLogBatch::try_from_records, which leaves dyn_col_dicts empty, so block.rs:1299-1303 takes the non-dict branch and writer.rs:1872 produces None for every plan. The block.rs unit tests drive stage_rows only, which has no dict path. Fix: add one case that calls .with_dictionaries() on the batch before push_columnar (the bridge columnar_batch.rs:1216 documents for exactly this, used by tests/rlog_v5_rowgroup_dict.rs:355) and asserts finish_on == finish, ideally also in the proptest arm.	fixed	crates/ravel-logseg/tests/parallel_page_encode.rs:204

No findings.

Approve: the one previous finding is fixed, and I found nothing new.

Previous round:

• Finding 1 (compact_dict untested) — fixed. tests/parallel_page_encode.rs:52 adds a Path::Dictionaries arm that calls b.with_dictionaries() before push_columnar, PATHS (line 49) puts it into the two shape tests and the proptest, and dictionary_columns_are_byte_identical (line 204) is a dedicated case whose input is built so each 100-row block references under a quarter of each column's object-wide dictionary (asserted at line 218), across low- and high-cardinality, fully- and partly-present, Str and Bytes columns, under both page choices and against both finish and the row path.

Findings: none.

Verified:

• compact_dict's remap is now pinned. I replaced its tail with kept.sort_unstable(); before OwnedStrings::new (exactly "ids pointing at the wrong kept entry, order not preserved") and ran the suite: dictionary_columns_are_byte_identical, generated_records_are_byte_identical, ingest_shape_flush_is_byte_identical and multi_block_row_groups_are_byte_identical all FAILED. Restored; git status --porcelain clean.
• Compaction is byte-neutral by construction, not only by test: string_dict_shape (rlog_codec.rs:564) builds distinct from referenced ids only and string_dict_shape_if_dict (rlog_codec.rs:642) takes limit from present_ids.len() and dedups by value over the referenced set, so dropping unreferenced entries and renumbering in first-use order changes neither shape nor the dict-vs-plain decision.
• Every page in both staging functions goes through single/i64/strings/dict_strings (block.rs:972-1088 and 1213-1315 call nothing else on the stager), which is what makes deferred_jobs' "staged outside the jobs" refusal (block.rs:480) a real backstop rather than a comment.
• Doc claim at docs/log-segment-format.md:885 that the floor and zstd envelope apply to every stored page under both choices: seal_page (page.rs:70-88) runs compress_if_smaller unconditionally, and Estimated reaches it through the same call.
• The bloom change is behaviour-preserving by construction: tokens is already for_each_token collecting into a Vec (ravel-codec/src/tokenizer.rs:27-31), so writer.rs:3226 swaps the buffer, not the token definition.
• LogSegError::PageEncode is additive: no exhaustive match on LogSegError exists outside ravel-logseg (only the test helper at tests/stream_block_rows.rs:147, updated).
• i64_raw_sizes has no caller outside the test module it moved into (rlog_codec.rs:823, used at 1032).
• assert_eq!(pool.out_of_order_calls, 1) in the golden test is deterministic, not timing-dependent: ScopedPool pops LIFO, so job 0 is the tenth popped and at least six results are already in finished before it starts.

Checks run:

• cargo test -p ravel-logseg --test parallel_page_encode --test golden_bytes_v5 --test bloom_insert_alloc_pin → 9 + 4 (1 ignored) + 1 passed
• cargo test -p ravel-logseg --lib → 441 passed
• cargo clippy -p ravel-logseg --all-targets → no warnings
• revert check above

Smaller things:

• Deferred mode copies each column's input (values.to_vec(), OwnedStrings) before any encode, so a block's peak memory is roughly its column data twice. That is inherent to Send + 'static jobs and fine here, but it is the figure worth stating when #3499 wires this into the ingest flush, where block size is caller-controlled.
• assemble_pages' "job {index} never returned" arm (block.rs:407) is unreachable once the count, range and duplicate checks pass: n results into n slots with no repeat fills every slot. Harmless as a defensive arm; it just has no test and cannot have one.

Not checked:

• The ADR-3492 decision 7 text itself: docs/adrs/ was not in the diff and I did not open the ADR, so the PR's mapping onto it rests on the PR description and the code.
• The 8-12% encode figure (181.7 ms to 169.9 ms) from the PR description: I did not run benches/logseg_encode.rs.
• Feature-lane and workspace gates (CI's job); I ran only ravel-logseg's lib and the three relevant integration targets.

Task 229f6779-0ce5-40ab-9b54-1f8daac32757 on rp2, model claude-opus-5, effort medium.