fix(llm-access-store): address deferred duckdb review nits

[acking-you]

What

Applies the genuinely-actionable findings deferred from the duckdb split (#16) review, now that the structural move has merged.

Change	Why
segment.rs spawn_segment_sealer	let _ = thread::Builder::spawn(...) swallowed spawn failures — on resource exhaustion the segment would never seal/archive with no diagnostic. Now captures the result and logs on failure (matching the closure's existing eprintln! idiom). Real reliability gap.
append.rs append_usage_events_to_tiered	Early-return on empty rows before taking the write gate — a no-op empty append must not reopen the writer or flip active_has_rows (which could trigger an empty rollover). Callers guard upstream, so defensive.
util.rs gzip_json_bytes	serde_json::to_writer straight into the GzEncoder, dropping the intermediate Vec<u8> (and the now-unused std::io::Write import).

Behavior-preserving notes

The gzip change is safe: detail-pack bytes are sha'd and verified per-blob against their own stored bytes, never compared across writers — so a different compressor write pattern can't break anything. Confirmed by the detail round-trip integration tests (round_trips_error_payloads_in_usage_detail, writes_heavy_detail_payloads_into_shared_pack).

Declined (with reasons)

• clear_stale_compacting_files missing-dir guard — its sole caller creates the compacting dir first (repository.rs), so the guarded condition is unreachable. Not a bug.
• gunzip_json_bytes → from_reader — serde_json's own guidance: from_reader over an unbuffered reader is slower than read-to-slice + from_slice. Current code is the recommended form; the suggestion is a regression.

Verification

• cargo clippy -p llm-access-store --all-targets -- -D warnings → clean
• cargo test -p llm-access-store → 68 passed
• cargo build -p llm-access (reverse dep) → ok
• rustfmt on the 3 changed files only; deps/lance/deps/lancedb untouched

🤖 Generated with Claude Code

[acking-you]

/gemini review

[acking-you]

@codex review

[gemini-code-assist]

Code Review

This pull request introduces an early return for empty appends to avoid unnecessary write locks, adds error handling for thread spawning failures in the segment sealer, and refactors JSON serialization in gzip_json_bytes to write directly to the gzip encoder. Feedback suggests wrapping the unbuffered GzEncoder in a BufWriter when using serde_json::to_writer to prevent performance degradation from frequent small writes.

[gemini-code-assist]

Writing directly to an unbuffered GzEncoder using serde_json::to_writer can lead to severe performance degradation. serde_json::to_writer performs many tiny, individual write operations (often single bytes or small tokens for punctuation, keys, etc.). Since GzEncoder does not perform internal buffering, each of these tiny writes forces the underlying compression algorithm to run, resulting in high CPU overhead and potentially worse compression ratios.

To avoid this while still dropping the intermediate Vec<u8> allocation of the uncompressed JSON, wrap the encoder in a std::io::BufWriter before passing it to serde_json::to_writer.

pub fn gzip_json_bytes<T: serde::Serialize>(value: &T) -> anyhow::Result<Vec<u8>> {
    let mut encoder = GzEncoder::new(Vec::new(), Compression::default());
    {
        let mut writer = std::io::BufWriter::new(&mut encoder);
        serde_json::to_writer(&mut writer, value).context("serialize and gzip usage detail json")?;
        std::io::Write::flush(&mut writer).context("flush gzip usage detail payload")?;
    }
    encoder.finish().context("finish gzip usage detail payload")
}

[acking-you]

Adopted in 15985e3 — wrapped the encoder in BufWriter so serde's tiny writes are batched before the compressor, while still dropping the intermediate uncompressed Vec. One deviation from the snippet: I drain with into_inner() instead of flush(). BufWriter::flush() propagates to GzEncoder::flush() and injects a DEFLATE sync-flush, which changes the output bytes (still valid gzip, slightly worse ratio); into_inner() only writes the buffered bytes, so the encoder sees the same stream as a single bulk write and the output stays byte-identical to the pre-#22 to_vec+write_all form. Verified across empty/small/8KB-overflow payloads.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[gemini-code-assist]

Code Review

This pull request introduces an early return for empty appends in the DuckDB backend, adds error handling for spawning the segment sealer thread, and refactors gzip_json_bytes to stream serialization directly into the Gzip encoder. The review feedback highlights a potential performance regression in the serialization refactor, where writing directly to an unbuffered GzEncoder via serde_json::to_writer causes many small writes. The reviewer recommends wrapping the encoder in a BufWriter to batch writes efficiently.

[gemini-code-assist]

To support buffered writing in gzip_json_bytes and avoid performance overhead from unbuffered writes, let's import BufWriter and Write from std::io.

Suggested change

	use std::{
	io::{Read, Write},
	io::Read,
	time::{SystemTime, UNIX_EPOCH},
	};
	use std::{
	io::{BufWriter, Read, Write},
	time::{SystemTime, UNIX_EPOCH},
	};

[acking-you]

Imported BufWriter in 15985e3. I left out Write though: since I drain with BufWriter::into_inner() (an inherent method) and never call a Write trait method directly in this fn, importing Write would be an unused import (-D warnings would reject it). Final import is use std::io::{BufWriter, Read};.

[gemini-code-assist]

While streaming directly to the encoder avoids allocating the entire uncompressed JSON payload in a Vec<u8>, calling serde_json::to_writer directly on an unbuffered GzEncoder introduces a performance regression.

serde_json::to_writer performs many small, unbuffered writes (often single bytes or short fragments). Passing these tiny slices repeatedly to the compression encoder incurs significant overhead.

As noted in your PR description regarding gunzip_json_bytes (where unbuffered reader deserialization is slower), the same principle applies to serialization: we should wrap the encoder in a BufWriter to batch the writes into larger, more efficient chunks before they reach the compressor.

pub fn gzip_json_bytes<T: serde::Serialize>(value: &T) -> anyhow::Result<Vec<u8>> {
    let mut encoder = GzEncoder::new(Vec::new(), Compression::default());
    {
        let mut writer = BufWriter::new(&mut encoder);
        serde_json::to_writer(&mut writer, value).context("serialize and gzip usage detail json")?;
        writer.flush().context("flush buffered json writer")?;
    }
    encoder.finish().context("finish gzip usage detail payload")
}

[acking-you]

Fixed in 15985e3 via BufWriter, same reasoning you give (batch serde's small writes before the compressor, keep the streaming/no-uncompressed-Vec property). Used into_inner() rather than flush() to drain — see the reply on the sibling thread: flush() would inject a DEFLATE sync-flush and alter the output bytes, into_inner() keeps it byte-identical to the bulk-write baseline.