Preflight eval: §C cost/latency baseline

[silongtan]

Summary

Implements #88 — measurement infrastructure for the catalog's §C cost/latency baseline. Prerequisite for any optimization PR against #58 to have a regression target.

Scope evolved during review: original C3 latency measurement used mocked ledger queries, which Jin correctly flagged as not capturing real updates. Reworked to real memory:// SurrealDB seeded with synthetic data through the production ingest_payload path — every C2/C3 measurement now exercises the real SurrealDB query plan, handler iteration, and serialization.

Metrics + baselines

Metric	What	N=10	N=100	N=1000
C1	bicameral.history() payload tokens	7,574	79,025	795,982
C2	bicameral.preflight() response tokens	566	571	575
C2	bicameral.preflight() response bytes	2,303	2,303	2,303
C3	Handler latency p50	2.5ms	14.8ms	138.8ms
C3	Handler latency p95	3.0ms	15.9ms	141.7ms

Two punch lines from the data:

1. C1 N=1000 = 796K tokens — a single bicameral.history() call fills 80% of Sonnet 4.6's 1M context before the skill reasons about anything. The §C concern is concrete and material.
2. C3 N=1000 = ~140ms — real-ledger preflight at scale crosses into user-perceptible latency. That's the user-experience surface an optimization PR should reduce.

What this test actually catches

• Future PR adds a verbose field to HistoryResponse → C1 token count grows → flag
• Future PR changes the SurrealDB query plan to scan instead of index lookup → C3 latency grows → flag
• Future PR bloats PreflightResponse shape → C2 byte count grows → flag
• Future optimization (semantic prefilter, lazy/two-pass history from M6 preflight handler retrieval: by-design split (handler structural, skill-layer covered by #306) #58) reduces C1/C3 → measurable win
• Asymmetric rule means improvements never alert; only regressions trip

What this test explicitly does NOT catch

• Real-world decision content distributions (synthetic generator, fixed templates) — that's Preflight: real-world eval coverage (real-transcript fixtures + CI dual-eval) #66's territory
• Skill-layer LLM cost — that's phase 2 of the eval harness + Preflight eval: phase 2 dataset expansion — M2, M3, FF3 rows #89
• Production user-perceived latency from real traffic — that's Preflight: telemetry capture loop for real-world failure feedback #65 (telemetry)

The three together form a measurement strategy: phase 3 is the synthetic baseline that holds optimization claims accountable; #65 + #66 cover what synthetic doesn't.

Architecture

Component	Lines	Purpose
tests/eval/_synthetic_ledger.py	~195	Deterministic generator producing HistoryResponse-shaped dicts
tests/eval/_seed_ledger.py	~120	Translates synthetic dict → real SurrealDB writes via adapter.ingest_payload
tests/eval/_token_count.py	~38	tiktoken cl100k_base wrapper
tests/eval/_baseline_io.py	~180	Load / write / find / upsert + asymmetric regression rule with noise floors
tests/eval/run_preflight_cost_eval.py	~270	Pytest runner — C1 / C2 / C3 measurements
tests/eval/test_cost_baseline_helpers.py	~360	35 unit tests (helpers + seeder + regression rule + IO)
tests/eval/cost_baseline.jsonl	9 rows	C1×3 + C2×3 platform-agnostic; C3×3 Darwin
.github/workflows/preflight-eval.yml	+18	Phase 3 step
docs/preflight-failure-scenarios.md	+6/-7	Catalog ticks
pyproject.toml	+1	tiktoken in [test] extras

Recording flow

BICAMERAL_EVAL_RECORD_BASELINE=1 pytest tests/eval/run_preflight_cost_eval.py
git add tests/eval/cost_baseline.jsonl
git commit -m "test(eval): re-record C* baselines after <intentional change>"

The harness regenerates rows for the current platform; diff is reviewable in the PR. C1/C2 use platform-agnostic baselines (token counts and response bytes are deterministic across OSes). C3 latency is per-platform — Linux baselines need to be recorded separately on a Linux runner before CI Linux validates C3.

CI behavior

• Phase 3 added to preflight-eval.yml, advisory (continue-on-error: true)
• On Linux CI today: C1 + C2 validate against recorded_on=any rows; C3 skips with re-record instructions until Linux baseline is added
• Phase 3 takes ~2.5 minutes (driven by N=1000 seeding + measurement). Non-blocking.

Test plan

• pytest tests/eval/run_preflight_cost_eval.py — 9 passed (3× C1, 3× C2, 3× C3) against new real-ledger baselines
• pytest tests/eval/test_cost_baseline_helpers.py — 35 passed (helpers + seeder + regression rule + IO)
• All other eval suites unchanged: phase 1 (6 passed / 4 xfailed), phase 2 (1 passed / 3 skipped), regression suite (25 passed / 1 skipped)
• Forced regression test: corrupt baseline by -30%, confirm pytest fails with the documented error message ✓
• Reproducibility: re-record + run normally → green; bit-identical token counts across runs

Closes / unblocks

• Closes Preflight eval: §C cost/latency baseline — bicameral.history() payload + handler latency #88
• Unblocks M6 preflight handler retrieval: by-design split (handler structural, skill-layer covered by #306) #58 — every optimization PR can now be evaluated against the committed real-I/O baselines

🤖 Generated with Claude Code

[coderabbitai]

Warning

Rate limit exceeded

@silongtan has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 53 minutes and 20 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d402957-f29c-4254-a330-4f135e217375

📥 Commits

Reviewing files that changed from the base of the PR and between 531067a and bbc5933.

📒 Files selected for processing (9)

• .github/workflows/preflight-eval.yml
• docs/preflight-failure-scenarios.md
• pyproject.toml
• tests/eval/_baseline_io.py
• tests/eval/_synthetic_ledger.py
• tests/eval/_token_count.py
• tests/eval/cost_baseline.jsonl
• tests/eval/run_preflight_cost_eval.py
• tests/eval/test_cost_baseline_helpers.py

📝 Walkthrough

Walkthrough

Adds an advisory CI step to run preflight cost/latency evaluations against a committed baseline. Introduces helper modules for baselines, token counting, and synthetic ledger generation; a pytest runner covering C1–C3; a committed baseline JSONL; documentation updates; a new test dependency; and a comprehensive helper test suite.

Changes

Cohort / File(s)	Summary
CI workflow (advisory preflight eval) .github/workflows/preflight-eval.yml	Adds a non-blocking phase running tests/eval/run_preflight_cost_eval.py and emitting test-results/preflight-cost-eval.xml with continue-on-error: true.
Docs: baseline rules and checklist docs/preflight-failure-scenarios.md	Updates §C to mark C1–C3 as baselined, defines asymmetric ±20% regression with noise floors, documents baseline recording via BICAMERAL_EVAL_RECORD_BASELINE=1, and links to CI workflow.
Project deps (tests) pyproject.toml	Adds tiktoken>=0.7.0,<1.0.0 to the optional test dependency group.
Eval helpers (baseline IO, ledger, tokenization) tests/eval/_baseline_io.py, tests/eval/_synthetic_ledger.py, tests/eval/_token_count.py	Introduces baseline storage/upsert and regression checking, a deterministic synthetic ledger generator, and token counting via tiktoken. Pay attention to platform/version keys and noise-floor logic.
Committed baselines tests/eval/cost_baseline.jsonl	Adds initial JSONL rows for C1 token counts, C2 size metrics, and C3 latency (p50/p95), tagged with platform and versions.
Preflight eval runner (C1–C3) tests/eval/run_preflight_cost_eval.py	Implements pytest-based evaluation: record-or-assert flow, platform/version matching, handler isolation, C1 token count over synthetic ledgers, C2 response token/byte sizes, C3 latency quantiles, JUnit output.
Helper test suite tests/eval/test_cost_baseline_helpers.py	Tests synthetic ledger determinism/shape, token counting invariants, regression-check behavior, and baseline IO find/upsert semantics.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Dev as Developer
    participant CI as GitHub Actions
    participant Py as Pytest Runner
    participant BL as Baseline IO
    participant LG as Synthetic Ledger
    participant TK as Tokenizer
    participant HD as Preflight Handler
    participant JR as JUnit Report

    Dev->>CI: Push/PR
    CI->>Py: Run preflight cost eval (advisory)
    Py->>BL: Load committed baselines
    alt C1: history payload tokens
        Py->>LG: Generate synthetic ledger (N features)
        LG-->>Py: Deterministic payload
        Py->>TK: Count tokens (cl100k_base)
        TK-->>Py: Token count
        Py->>BL: Regression check (C1)
    else C2: preflight response size
        Py->>HD: handle_preflight(mocked ctx)
        HD-->>Py: Response JSON
        Py->>TK: Count tokens/bytes
        TK-->>Py: Sizes
        Py->>BL: Regression check (C2)
    else C3: handler latency
        Py->>HD: Warm + timed calls
        HD-->>Py: Latency samples
        Py->>BL: Regression checks (p50, p95)
    end
    Py-->>JR: Write test results (JUnit XML)
    CI-->>Dev: Report (non-blocking)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Preflight failure-scenario catalog + eval harness (v0.10.x) #62 — Introduces the preflight evaluation catalog and groundwork that this PR extends with baselines and CI integration.

Suggested reviewers

• jinhongkuan

Poem

I thump my paw—baseline set, hooray!
Tokens tallied, latencies at play.
Synthetic fields in orderly rows,
A ledger garden where the data grows.
If numbers drift, I gently squeak—
Flip the flag, record this week.
Hop-hop! Our preflight’s sleek. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 39.62% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Preflight eval: §C cost/latency baseline' directly and concisely summarizes the main change: introducing cost/latency baseline infrastructure for §C metrics.
Linked Issues check	✅ Passed	The pull request fully implements all coding requirements from issue #88: deterministic synthetic ledger (C1), preflight response size measurement (C2), handler latency benchmarking (C3), baseline file with committed values, regression-check logic with ±20% rule, and CI workflow integration.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing C1–C3 baseline infrastructure per issue #88, with no unrelated modifications. C4 (LLM-in-the-loop), cross-model baselines, and promotion to gating CI are correctly deferred as out of scope.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch preflight-cost-baseline

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 0/1 reviews remaining, refill in 53 minutes and 20 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (5)

tests/eval/run_preflight_cost_eval.py (2)

29-29: Unused import: asyncio.

The asyncio module is imported but not used directly — the async tests rely on pytest-asyncio to run the event loop.

🧹 Remove unused import

-import asyncio
 import sys

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/eval/run_preflight_cost_eval.py` at line 29, Remove the unused asyncio
import from the top of the test file: delete the line importing asyncio in
run_preflight_cost_eval.py since pytest-asyncio provides the event loop and no
symbol from asyncio is referenced (there are no functions/classes in this diff
like run_preflight_cost_eval that require asyncio directly).

---

322-324: Minor: p95 index calculation is slightly off.

With _C3_SAMPLES=100, int(len(timings_ms) * 0.95) yields index 95, which accesses the 96th element in the sorted list. For a true p95, you'd typically use index 94 (the 95th element out of 100).

That said, the difference is negligible for this use case (benchmarking handler latency), and the value will still be close to p95.

💡 Standard percentile calculation

     timings_ms.sort()
     p50 = timings_ms[len(timings_ms) // 2]
-    p95 = timings_ms[int(len(timings_ms) * 0.95)]
+    p95 = timings_ms[int(len(timings_ms) * 0.95) - 1]  # 0-indexed: 95th of 100 is index 94

Or use statistics.quantiles for standard behavior.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/eval/run_preflight_cost_eval.py` around lines 322 - 324, The p95 index
calculation currently uses int(len(timings_ms) * 0.95) which yields index 95 for
_C3_SAMPLES=100 (the 96th element); change the p95 calculation to use a
zero-based 95th percentile index (e.g., idx = math.ceil(len(timings_ms) * 0.95)
- 1) and set p95 = timings_ms[idx]; ensure you import math if needed. Keep the
p50 logic as-is (timings_ms[len(timings_ms) // 2]) or alternatively replace both
with statistics.quantiles/two-line percentile helper if you prefer standard
behavior.

tests/eval/test_cost_baseline_helpers.py (1)

161-165: Potential test fragility: JSON string literal vs json.dumps output.

The direct string '{"foo": "bar", "n": 42}' assumes a specific key ordering and spacing that json.dumps produces. While CPython 3.7+ preserves dict insertion order, the assertion relies on json.dumps producing no trailing spaces and the exact same key order. This works today but could become fragile.

Consider using the JSON function for both sides to ensure consistency:

💡 More robust comparison

 def test_count_tokens_json_matches_direct_serialize():
     payload = {"foo": "bar", "n": 42}
-    direct = count_tokens('{"foo": "bar", "n": 42}')
+    import json
+    direct = count_tokens(json.dumps(payload, ensure_ascii=False))
     via_json = count_tokens_json(payload)
     assert direct == via_json

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/eval/test_cost_baseline_helpers.py` around lines 161 - 165, The test
uses a hard-coded JSON literal which can be fragile due to spacing/ordering
differences; update test_count_tokens_json_matches_direct_serialize to produce
the direct string via json.dumps(payload) (or json.dumps(payload,
separators=(',', ':'), sort_keys=True) for a canonical form) and then call
count_tokens on that string so both sides use the same JSON serialization;
reference functions: test_count_tokens_json_matches_direct_serialize,
count_tokens, count_tokens_json, and json.dumps.

tests/eval/_baseline_io.py (2)

65-75: Use atomic replace when writing baseline files.

Line [75] writes directly to the target path; an interrupted write can corrupt the JSONL. A temp-file + replace pattern is safer in record mode.

♻️ Proposed improvement

 def write_baselines(rows: list[dict], path: Path = BASELINE_PATH) -> None:
     """Sorted, stable-key JSONL output to keep diffs minimal."""
     def _sort_key(row: dict) -> tuple:
         return (
             row.get("metric", ""),
             row.get("recorded_on", ""),
             row.get("n_features", -1),
         )
     rows_sorted = sorted(rows, key=_sort_key)
     body = "\n".join(json.dumps(r, sort_keys=True, ensure_ascii=False) for r in rows_sorted)
-    path.write_text(body + "\n", encoding="utf-8")
+    tmp_path = path.with_suffix(path.suffix + ".tmp")
+    tmp_path.write_text(body + "\n", encoding="utf-8")
+    tmp_path.replace(path)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/eval/_baseline_io.py` around lines 65 - 75, The write_baselines
function currently writes directly to BASELINE_PATH which can corrupt the file
if interrupted; change write_baselines to write the JSONL content to a temporary
file in the same directory (e.g., using tempfile or Path with a unique suffix)
and then atomically replace the target with os.replace (or Path.replace) so the
final write is atomic; ensure the temp file is opened with utf-8 and that you
still write the newline-terminated body and clean up the temp on error.

---

54-62: Add line-context error reporting for malformed JSONL.

Lines [58]-[61] currently raise raw JSONDecodeError, which makes broken baseline rows harder to diagnose. Consider surfacing file + line number.

♻️ Proposed improvement

 def load_baselines(path: Path = BASELINE_PATH) -> list[dict]:
     if not path.exists():
         return []
     rows = []
-    for line in path.read_text(encoding="utf-8").splitlines():
+    for lineno, line in enumerate(path.read_text(encoding="utf-8").splitlines(), start=1):
         line = line.strip()
         if line:
-            rows.append(json.loads(line))
+            try:
+                rows.append(json.loads(line))
+            except json.JSONDecodeError as exc:
+                raise ValueError(f"Malformed baseline JSONL at {path} Line [{lineno}]") from exc
     return rows

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/eval/_baseline_io.py` around lines 54 - 62, Wrap the json.loads call
inside load_baselines with a try/except that catches json.JSONDecodeError, track
the current line number using enumerate(path.read_text(...).splitlines(),
start=1) so you can include the file path and line number in the error, and
re-raise a clearer error (e.g., raise ValueError(f"Malformed JSON in {path} at
line {lineno}: {e}") from e) so the original JSONDecodeError is preserved as the
__cause__; update the rows.append(json.loads(line)) call in load_baselines
accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/eval/_baseline_io.py`:
- Around line 65-75: The write_baselines function currently writes directly to
BASELINE_PATH which can corrupt the file if interrupted; change write_baselines
to write the JSONL content to a temporary file in the same directory (e.g.,
using tempfile or Path with a unique suffix) and then atomically replace the
target with os.replace (or Path.replace) so the final write is atomic; ensure
the temp file is opened with utf-8 and that you still write the
newline-terminated body and clean up the temp on error.
- Around line 54-62: Wrap the json.loads call inside load_baselines with a
try/except that catches json.JSONDecodeError, track the current line number
using enumerate(path.read_text(...).splitlines(), start=1) so you can include
the file path and line number in the error, and re-raise a clearer error (e.g.,
raise ValueError(f"Malformed JSON in {path} at line {lineno}: {e}") from e) so
the original JSONDecodeError is preserved as the __cause__; update the
rows.append(json.loads(line)) call in load_baselines accordingly.

In `@tests/eval/run_preflight_cost_eval.py`:
- Line 29: Remove the unused asyncio import from the top of the test file:
delete the line importing asyncio in run_preflight_cost_eval.py since
pytest-asyncio provides the event loop and no symbol from asyncio is referenced
(there are no functions/classes in this diff like run_preflight_cost_eval that
require asyncio directly).
- Around line 322-324: The p95 index calculation currently uses
int(len(timings_ms) * 0.95) which yields index 95 for _C3_SAMPLES=100 (the 96th
element); change the p95 calculation to use a zero-based 95th percentile index
(e.g., idx = math.ceil(len(timings_ms) * 0.95) - 1) and set p95 =
timings_ms[idx]; ensure you import math if needed. Keep the p50 logic as-is
(timings_ms[len(timings_ms) // 2]) or alternatively replace both with
statistics.quantiles/two-line percentile helper if you prefer standard behavior.

In `@tests/eval/test_cost_baseline_helpers.py`:
- Around line 161-165: The test uses a hard-coded JSON literal which can be
fragile due to spacing/ordering differences; update
test_count_tokens_json_matches_direct_serialize to produce the direct string via
json.dumps(payload) (or json.dumps(payload, separators=(',', ':'),
sort_keys=True) for a canonical form) and then call count_tokens on that string
so both sides use the same JSON serialization; reference functions:
test_count_tokens_json_matches_direct_serialize, count_tokens,
count_tokens_json, and json.dumps.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 19b3775b-88f2-43c6-bb46-3a151777aab0

📥 Commits

Reviewing files that changed from the base of the PR and between 92369b7 and 531067a.

📒 Files selected for processing (9)

• .github/workflows/preflight-eval.yml
• docs/preflight-failure-scenarios.md
• pyproject.toml
• tests/eval/_baseline_io.py
• tests/eval/_synthetic_ledger.py
• tests/eval/_token_count.py
• tests/eval/cost_baseline.jsonl
• tests/eval/run_preflight_cost_eval.py
• tests/eval/test_cost_baseline_helpers.py