Skip to content

feat(ledger): add decision.updated_at + idx_decision_updated_at (#87 precondition)#308

Merged
silongtan merged 3 commits into
devfrom
87-ledger-updated-at-precondition
May 13, 2026
Merged

feat(ledger): add decision.updated_at + idx_decision_updated_at (#87 precondition)#308
silongtan merged 3 commits into
devfrom
87-ledger-updated-at-precondition

Conversation

@silongtan

Copy link
Copy Markdown
Collaborator

Summary

Schema half of #87 — adds the updated_at revision marker that Phase 4 will key the preflight dedup cache on. The handler-side broadening lands as a follow-up on 87-preflight-dedup-key.

Per Kevin's signoff (B2 approach): additive schema bump, L1 risk, no tool contract change, falls back gracefully.

Why now (Phase 2 → 4 sequencing)

The preflight dedup cache currently keys on topic alone (handlers/preflight.py::_dedup_key_for). Same-topic calls within the 5-minute window hit the cache even if the underlying ledger state changed mid-session — these are the M7a/b/c xfailed cases in tests/eval/run_preflight_eval.py. Phase 4 broadens the key to (topic_norm, file_paths_hash, ledger_revision). ledger_revision derives from MAX(updated_at) over the decision table — that's what this PR provides.

B3 benchmark (scripts/bench_b3_*.py, untracked locally — will commit separately) showed any unindexed aggregate on decision is 8ms+ p50 at N=1000, 8× over the acceptance threshold. Hence the dedicated index.

Design decision: option<datetime> not datetime

The field is option<datetime> rather than non-optional datetime. SurrealDB v2's DEFINE FIELD against existing rows leaves them as NONE until the migration's backfill runs — a non-optional type fails on the very first UPDATE-against-legacy-row. Same precedent as v8→v9 where decision_level is option<string> for identical reasons.

Phase 4's MAX(updated_at) query can COALESCE(updated_at, created_at) if it wants strict-non-NULL semantics — post-backfill there are no NONE rows, but COALESCE is the conservative choice.

What changed

Schema (ledger/schema.py)

  • SCHEMA_VERSION 17 → 18 + compatibility-map entry pinned to 0.14.x (release-eng final value on merge)
  • DEFINE FIELD updated_at ON decision TYPE option<datetime> DEFAULT time::now()
  • DEFINE INDEX idx_decision_updated_at ON decision FIELDS updated_at
  • _migrate_v17_to_v18 — idempotent DEFINE pass + backfill UPDATE decision SET updated_at = created_at WHERE updated_at IS NONE

Call-site audits (7 UPDATEs now carry , updated_at = time::now())

File:Line What it updates
ledger/queries.py:602 upsert_decision canonical-dedup UPDATE path
ledger/queries.py:1072 update_decision_status
ledger/queries.py:1163 update_decision_level
ledger/adapter.py:1394 apply_ratify (signoff write)
ledger/adapter.py:1428 apply_supersede (old decision signoff-freeze)
handlers/resolve_collision.py:99 link_parent (cross-level parent link)
handlers/resolve_collision.py:128 collision_pending clear (proposed signoff)

CREATE in ledger/queries.py:638 needs no edit — DEFAULT time::now() fires on INSERT automatically.

Tests (tests/test_v18_decision_updated_at.py, 11 tests, all passing)

Sociable substrate over memory:// per CLAUDE.md — real SurrealDBLedgerAdapter + real LedgerClient, no mocks. The drift this guards against is exactly what solitary tests miss: a mock would happily return whatever updated_at the test expects; only a real ledger UPDATE proves the SQL actually carries the new column.

  • test_v18_schema_version_advanced
  • test_v18_create_decision_sets_updated_at_close_to_created_at
  • test_v18_update_decision_status_bumps_updated_at
  • test_v18_update_decision_level_bumps_updated_at
  • test_v18_apply_ratify_bumps_updated_at
  • test_v18_apply_supersede_bumps_updated_at_on_old_decision
  • test_v18_find_or_create_canonical_update_path_bumps_updated_at
  • test_v18_resolve_collision_signoff_clear_bumps_updated_at
  • test_v18_resolve_collision_link_parent_bumps_updated_at
  • test_v18_index_idx_decision_updated_at_exists
  • test_v18_migration_backfills_legacy_rows_with_none_updated_at

Regression check

  • tests/test_v15_migration.py, test_schema_persistence.py, test_schema_recoverable_errors.py, test_sync_middleware.py, test_codegenome_continuity_service.py, test_compliance_check_schema.py, test_ledger_bicameral_meta_migration.py50/50 pass
  • ⚠️ tests/test_alpha_flow.py::test_code_edit_without_rebind_marks_drifted fails — but reproduces on origin/dev with this PR's changes reverted, so pre-existing and not caused here. Worth filing separately.
  • ⚠️ Pre-existing env issues (no mcp package, no M6Case fixture export) prevent some tests from collecting in this environment — unchanged by this PR.

Test plan

Out of scope

🤖 Generated with Claude Code

…precondition)

Phase 4 of #87 broadens the preflight dedup cache key from `topic` alone
to `(topic_norm, file_paths_hash, ledger_revision)` so a same-topic call
within the 5-min window correctly invalidates when underlying ledger state
changed (M7a/b/c xfailed cases). `ledger_revision` derives from
`MAX(updated_at)` over the decision table — this PR is the schema half
of that contract; the handler-side broadening lands as a follow-up on
branch `87-preflight-dedup-key`.

Per Kevin's signoff (B2 approach, gh issue #87 comment thread): additive
schema bump, L1 risk, no tool contract change, falls back gracefully.
The field is `option<datetime>` rather than non-optional `datetime` because
DEFINE FIELD against existing rows leaves them as NONE until the migration
backfill runs — same precedent as v8→v9 (`decision_level` is `option<string>`
for identical reasons). Phase 4's MAX query can COALESCE(updated_at,
created_at) if it wants strict-non-NULL semantics.

Schema changes (ledger/schema.py):
- SCHEMA_VERSION 17 → 18 + compatibility-map entry
- DEFINE FIELD updated_at ON decision TYPE option<datetime> DEFAULT time::now()
- DEFINE INDEX idx_decision_updated_at ON decision FIELDS updated_at
- _migrate_v17_to_v18: idempotent DEFINE + backfill
  UPDATE decision SET updated_at = created_at WHERE updated_at IS NONE

Call-site audits (7 UPDATEs now carry `, updated_at = time::now()`):
- ledger/queries.py:602  upsert_decision canonical-dedup UPDATE path
- ledger/queries.py:1072 update_decision_status
- ledger/queries.py:1163 update_decision_level
- ledger/adapter.py:1394 apply_ratify (signoff write)
- ledger/adapter.py:1428 apply_supersede (old decision signoff-freeze)
- handlers/resolve_collision.py:99  link_parent (cross-level parent link)
- handlers/resolve_collision.py:128 collision_pending clear (proposed signoff)

CREATE in queries.py:638 needs no edit — the DEFAULT picks up time::now()
on INSERT automatically.

Tests (tests/test_v18_decision_updated_at.py, 11 tests, all passing):
- Schema version advanced to v18
- CREATE populates updated_at via DEFAULT
- Each of the 7 UPDATE call sites bumps updated_at (one test each)
- Index supports ORDER BY updated_at DESC
- Migration backfill: pre-v18 rows with NONE → created_at

Sociable substrate over memory:// per CLAUDE.md guidance — real
SurrealDBLedgerAdapter + real LedgerClient, no mocks. The drift this
guards against is the kind solitary tests miss: a mock would happily
return whatever updated_at the test expects; only a real ledger UPDATE
proves the SQL actually carries the new column.

Regression check passes: tests/test_v15_migration.py, test_schema_persistence.py,
test_schema_recoverable_errors.py, test_sync_middleware.py,
test_codegenome_continuity_service.py, test_compliance_check_schema.py,
test_ledger_bicameral_meta_migration.py — 50/50 pass. The single
test_alpha_flow.py failure (test_code_edit_without_rebind_marks_drifted)
reproduces on origin/dev without this PR's changes — pre-existing, not
introduced here.

Refs #87 (Phase 4 precondition per spec signoff). Out of scope: dedup
key broadening itself (#87 Phase 4), telemetry (#87 Phase 5).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9289dfe0-a62e-4de1-871f-b72ae5065c4e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 87-ledger-updated-at-precondition

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

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

…d_at (#308 CI fix)

CI surfaced two issues on PR #308:

1. Ruff I001 — tests/test_v18_decision_updated_at.py import block was
   not alphabetically sorted within the `from ledger.queries import (...)`
   group. Auto-fixed.

2. tests/test_legacy_ledger_fixtures.py::test_legacy_ledger_fixture_reaches_clean_state[v3_yields_source_span]
   blew up on the v17→v18 backfill:

     SurrealDB rejected query: Found NONE for field `created_at`,
     with record `decision:dec_1`, but expected a datetime
     SQL: UPDATE decision SET updated_at = created_at WHERE updated_at IS NONE

   The v3 fixture creates `decision:dec_1` via raw CREATE without
   setting `created_at`. Once init_schema applies
   `DEFINE FIELD created_at ON decision TYPE datetime`, ANY UPDATE on
   that row re-validates the row and trips the type assertion — even
   one that doesn't touch created_at. The earlier draft used
   `SET updated_at = created_at` which read the corrupt field directly;
   even after switching to time::now() in the SET clause, the implicit
   re-validation on UPDATE still failed.

## Fix

Switch the backfill from a single bulk UPDATE to a per-row loop with
try/except, mirroring `_clean_yields_legacy_rows` (which uses the same
tolerance pattern for v3-era stale yields edges):

```python
ids = await client.query("SELECT id FROM decision WHERE updated_at IS NONE")
for row in ids:
    try:
        await client.execute(f"UPDATE {row['id']} SET updated_at = time::now()")
        healed += 1
    except Exception:
        skipped += 1  # row has other corrupt non-optional fields
        logger.warning(...)
```

Rows that fail stay with `updated_at=NONE` and MAX(updated_at) skips
them. Harmless for the dedup-cache marker (#87) since the marker only
needs monotonicity, not coverage — the new decisions created post-v18
get DEFAULT time::now() and dominate MAX().

The SELECT itself reads only `id`, so it doesn't trip the type
assertion on `created_at`. The WHERE clause on `updated_at IS NONE` is
safe because `updated_at` is `option<datetime>` (intentionally
optional — same precedent as v8→v9 `decision_level`).

## Files

- ledger/schema.py — _migrate_v17_to_v18: per-row UPDATE with
  try/except; emits healed/skipped counts to the logger
- tests/test_v18_decision_updated_at.py
  - Import sort fix (ruff I001)
  - test_v18_migration_backfills_legacy_rows_with_none_updated_at:
    call _migrate_v17_to_v18 directly instead of inlining the (now
    multi-statement) backfill body
  - test_v18_migration_backfill_tolerates_legacy_rows_with_none_created_at
    (NEW): inspects the migration source to guard against future
    drafts that reintroduce a created_at reference in the SET clause

## Verification

- tests/test_legacy_ledger_fixtures.py::test_legacy_ledger_fixture_reaches_clean_state[v3_yields_source_span] PASS
- tests/test_v18_decision_updated_at.py — 13/13 PASS (12 originals + 1 new regression guard)
- 94/94 in the broader schema/migration/dedup cluster
- `python3 -m ruff check` — clean on all touched files

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@silongtan silongtan had a problem deploying to recording-approval May 13, 2026 01:39 — with GitHub Actions Failure
silongtan added a commit that referenced this pull request May 13, 2026
Closes the M7 family of preflight-dedup misses by broadening the per-
session cache key from `(topic)` alone to the 3-tuple `(normalized_topic,
normalized_file_paths, ledger_revision)`. Companion to the v18 schema
precondition (#308) which gave us the `decision.updated_at` marker.

Per Kevin's signoff (B2, issue #87 comment thread): if `ledger_revision`
lookup fails, BYPASS the dedup check entirely with loud telemetry rather
than degrade to a partial key. Correctness over saving a preflight call —
a stale partial key could silently suppress a valid call, whereas a
bypass only costs one extra preflight run.

## What changed

handlers/preflight.py
- _dedup_key_for() — now takes (topic, file_paths, ledger_revision)
  and returns "topic_norm||paths_norm||rev" so any one component
  differing produces a fresh key.
- _normalize_file_paths_for_key() — new helper. Sorted + lowercased +
  deduplicated + whitespace-stripped; empty / None collapse to "".
- _check_dedup() — same broadened signature; ctx._sync_state cache
  keyed on the 3-tuple.
- handle_preflight() — looks up ledger_revision before dedup. None
  return (transient SurrealDB error, ctx without _inner._client, etc.)
  triggers the BYPASS branch: loud warning, no cache touch, fall
  through to full preflight evaluation.

ledger/queries.py
- get_ledger_revision(client) — new. `MAX(coalesce(updated_at,
  created_at)) FROM decision` via the v18 index. Returns ISO string,
  "" for empty table (stable sentinel preserving cache hits), or None
  on query failure. COALESCE handles pre-v18 rows whose updated_at
  is still NULL between DEFINE FIELD and the migration backfill.

tests/eval/run_preflight_eval.py
- _apply_setup — monkeypatches get_ledger_revision with a value
  derived from the setup contents (or an explicit setup.ledger_revision
  if provided). This makes M7a (decision lands), M7c (HITL clears),
  and any future row's between-call ledger-state change naturally
  invalidate the cache, matching production semantics.

tests/eval/preflight_dataset.jsonl
- M7a/b/c rows: xfail → null. Notes updated to describe the new
  contract.

tests/test_preflight_dedup_v2.py (NEW, 18 tests)
- Pin _normalize_file_paths_for_key contract (5 tests):
  empty, order-insensitive, case-insensitive, dedupe, whitespace.
- Pin _dedup_key_for shape (5 tests): 3 components, topic phrasing
  collapses, file_paths differ → different keys, revision differs →
  different keys, all match → same key.
- Pin _check_dedup behavior (6 tests): first call misses, identical
  second call hits, revision change misses (M7a/c), file_paths
  change misses (M7b), no _sync_state never dedups, topic too short
  never dedups.
- End-to-end bypass path (2 tests): revision=None forces both
  successive calls through full evaluation AND leaves the cache
  untouched; revision="rev-1" restores legacy dedup behavior.

docs/preflight-failure-scenarios.md
- M7 row: ❌ open → ✅ closed by #87 Phase 4. Notes Kevin's amendment.
- Catalog header: dedup-key description updated.
- M7 checklist items ticked.

## Test results

- 18/18 new unit tests pass (test_preflight_dedup_v2.py)
- 10/10 eval-harness tests pass (run_preflight_eval.py) — M7a/b/c
  flipped xfail → pass with strict-xfail catching any silent
  regression
- 128/128 in the broader preflight + schema + dedup cluster

## Out of scope

- Telemetry counter `dedup_invalidated_by_revision_bump` — #87 Phase 5
- Per-key TTL tuning — deferred until Phase 5 telemetry shows it matters
- Cross-session dedup cache — deferred until per-session is shown
  insufficient

Refs #87 Phase 4. Stacked on #308 (v18 precondition).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
silongtan added a commit that referenced this pull request May 13, 2026
Adds the production-attribution counter Kevin asked for at #87 signoff:
*"so we can tell the new key is doing useful work in production"*.

Two outcomes are emitted to ~/.bicameral/preflight_events.jsonl with
event_type=preflight_dedup_decision:

  reason=invalidated_by_revision_bump
    A same-(topic, file_paths) call missed the cache because
    ledger_revision advanced. This is the M7a/M7c signal — proves
    Phase 4's new key shape is invalidating correctly on real ledger
    mutations and HITL state changes.

  reason=bypassed_revision_unknown
    Revision lookup returned None and the handler short-circuited
    the dedup check per Kevin's amendment. A sustained spike here
    means transient SurrealDB faults / schema mismatch / connection
    issues — actionable ops signal.

Cache hits, first-call misses, topic-changed misses, and file_paths-
shift misses are intentionally NOT instrumented. Phase 5's scope is
the change-detection signal; hit/miss baselines are derivable from
existing write_preflight_event rows (reason=recently_checked) if a
follow-up wants the denominator.

## What changed

preflight_telemetry.py
- write_dedup_event(reason, session_id, preflight_id=None) — new.
  Matches the existing per-event API pattern (write_fallback_event,
  write_bypass_event). No-op when telemetry disabled. preflight_id
  is omitted from the record when None for a cleaner schema.

handlers/preflight.py
- Import write_dedup_event.
- _dedup_miss_was_revision_bump() — new classifier. Returns True iff
  ctx._sync_state holds a same-(topic, file_paths) prefix with a
  different revision suffix within TTL. Skips identical keys (would
  have been a HIT, not a miss) and expired entries (TTL boundary).
- handle_preflight() — emits at the two decision points:
    * BYPASS branch (ledger_revision=None) → bypassed_revision_unknown
    * MISS-then-classified-as-revision-bump → invalidated_by_revision_bump
  Classification happens AFTER _check_dedup writes the current key;
  the classifier's skip-identical-keys rule prevents false positives.

tests/test_preflight_dedup_telemetry.py (NEW, 15 tests)
- 7 pin _dedup_miss_was_revision_bump classification:
  first call, revision differs only (M7a/c), file_paths differ
  (M7b — must NOT classify as revision bump), topic differs, TTL
  expiry, identical keys, missing _sync_state.
- 5 pin end-to-end telemetry emission:
  bypass emits one bypassed_revision_unknown; two-call revision-bump
  scenario emits one invalidated_by_revision_bump; first call alone
  emits nothing; cache hit emits nothing; file_paths shift does NOT
  emit revision_bump.
- 3 pin write_dedup_event direct contract:
  no-op when telemetry disabled, full-record shape when enabled,
  preflight_id omitted when None.

## Test results

- 15/15 new Phase 5 tests pass
- 143/143 in the broader Phase 4+5 + schema + dedup cluster

## Out of scope

- Dashboard wiring (separate operations PR)
- File-paths-shift attribution (M7b) — derivable from existing
  preflight_events.jsonl rows; not in Kevin's signoff
- Per-key TTL tuning — defer until telemetry shows it matters

Refs #87 Phase 5. Stacked on #309 (Phase 4) which is stacked on
#308 (v18 precondition).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI's ruff job runs BOTH `ruff check` AND `ruff format --check`. The
former was clean after the import-sort fix, but the latter flagged
ledger/schema.py and tests/test_v18_decision_updated_at.py for
reformatting. Applied `ruff format` in place — pure whitespace /
line-length normalization, no semantic change.

Verified: `ruff format --check` clean on both files locally; 14/14
v18 + legacy-fixture tests still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@silongtan silongtan had a problem deploying to recording-approval May 13, 2026 01:41 — with GitHub Actions Failure
silongtan added a commit that referenced this pull request May 13, 2026
Closes the M7 family of preflight-dedup misses by broadening the per-
session cache key from `(topic)` alone to the 3-tuple `(normalized_topic,
normalized_file_paths, ledger_revision)`. Companion to the v18 schema
precondition (#308) which gave us the `decision.updated_at` marker.

Per Kevin's signoff (B2, issue #87 comment thread): if `ledger_revision`
lookup fails, BYPASS the dedup check entirely with loud telemetry rather
than degrade to a partial key. Correctness over saving a preflight call —
a stale partial key could silently suppress a valid call, whereas a
bypass only costs one extra preflight run.

## What changed

handlers/preflight.py
- _dedup_key_for() — now takes (topic, file_paths, ledger_revision)
  and returns "topic_norm||paths_norm||rev" so any one component
  differing produces a fresh key.
- _normalize_file_paths_for_key() — new helper. Sorted + lowercased +
  deduplicated + whitespace-stripped; empty / None collapse to "".
- _check_dedup() — same broadened signature; ctx._sync_state cache
  keyed on the 3-tuple.
- handle_preflight() — looks up ledger_revision before dedup. None
  return (transient SurrealDB error, ctx without _inner._client, etc.)
  triggers the BYPASS branch: loud warning, no cache touch, fall
  through to full preflight evaluation.

ledger/queries.py
- get_ledger_revision(client) — new. `MAX(coalesce(updated_at,
  created_at)) FROM decision` via the v18 index. Returns ISO string,
  "" for empty table (stable sentinel preserving cache hits), or None
  on query failure. COALESCE handles pre-v18 rows whose updated_at
  is still NULL between DEFINE FIELD and the migration backfill.

tests/eval/run_preflight_eval.py
- _apply_setup — monkeypatches get_ledger_revision with a value
  derived from the setup contents (or an explicit setup.ledger_revision
  if provided). This makes M7a (decision lands), M7c (HITL clears),
  and any future row's between-call ledger-state change naturally
  invalidate the cache, matching production semantics.

tests/eval/preflight_dataset.jsonl
- M7a/b/c rows: xfail → null. Notes updated to describe the new
  contract.

tests/test_preflight_dedup_v2.py (NEW, 18 tests)
- Pin _normalize_file_paths_for_key contract (5 tests):
  empty, order-insensitive, case-insensitive, dedupe, whitespace.
- Pin _dedup_key_for shape (5 tests): 3 components, topic phrasing
  collapses, file_paths differ → different keys, revision differs →
  different keys, all match → same key.
- Pin _check_dedup behavior (6 tests): first call misses, identical
  second call hits, revision change misses (M7a/c), file_paths
  change misses (M7b), no _sync_state never dedups, topic too short
  never dedups.
- End-to-end bypass path (2 tests): revision=None forces both
  successive calls through full evaluation AND leaves the cache
  untouched; revision="rev-1" restores legacy dedup behavior.

docs/preflight-failure-scenarios.md
- M7 row: ❌ open → ✅ closed by #87 Phase 4. Notes Kevin's amendment.
- Catalog header: dedup-key description updated.
- M7 checklist items ticked.

## Test results

- 18/18 new unit tests pass (test_preflight_dedup_v2.py)
- 10/10 eval-harness tests pass (run_preflight_eval.py) — M7a/b/c
  flipped xfail → pass with strict-xfail catching any silent
  regression
- 128/128 in the broader preflight + schema + dedup cluster

## Out of scope

- Telemetry counter `dedup_invalidated_by_revision_bump` — #87 Phase 5
- Per-key TTL tuning — deferred until Phase 5 telemetry shows it matters
- Cross-session dedup cache — deferred until per-session is shown
  insufficient

Refs #87 Phase 4. Stacked on #308 (v18 precondition).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
silongtan added a commit that referenced this pull request May 13, 2026
Sibling of #308's format-fix commit. CI's `ruff format --check`
flagged Phase 4's additions:
- ledger/queries.py — get_ledger_revision() helper
- tests/test_preflight_dedup_v2.py — 18 unit tests

Pure whitespace / line-length normalization, no semantic change.
42/42 preflight + dedup + v18 tests still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
silongtan added a commit that referenced this pull request May 13, 2026
Adds the production-attribution counter Kevin asked for at #87 signoff:
*"so we can tell the new key is doing useful work in production"*.

Two outcomes are emitted to ~/.bicameral/preflight_events.jsonl with
event_type=preflight_dedup_decision:

  reason=invalidated_by_revision_bump
    A same-(topic, file_paths) call missed the cache because
    ledger_revision advanced. This is the M7a/M7c signal — proves
    Phase 4's new key shape is invalidating correctly on real ledger
    mutations and HITL state changes.

  reason=bypassed_revision_unknown
    Revision lookup returned None and the handler short-circuited
    the dedup check per Kevin's amendment. A sustained spike here
    means transient SurrealDB faults / schema mismatch / connection
    issues — actionable ops signal.

Cache hits, first-call misses, topic-changed misses, and file_paths-
shift misses are intentionally NOT instrumented. Phase 5's scope is
the change-detection signal; hit/miss baselines are derivable from
existing write_preflight_event rows (reason=recently_checked) if a
follow-up wants the denominator.

## What changed

preflight_telemetry.py
- write_dedup_event(reason, session_id, preflight_id=None) — new.
  Matches the existing per-event API pattern (write_fallback_event,
  write_bypass_event). No-op when telemetry disabled. preflight_id
  is omitted from the record when None for a cleaner schema.

handlers/preflight.py
- Import write_dedup_event.
- _dedup_miss_was_revision_bump() — new classifier. Returns True iff
  ctx._sync_state holds a same-(topic, file_paths) prefix with a
  different revision suffix within TTL. Skips identical keys (would
  have been a HIT, not a miss) and expired entries (TTL boundary).
- handle_preflight() — emits at the two decision points:
    * BYPASS branch (ledger_revision=None) → bypassed_revision_unknown
    * MISS-then-classified-as-revision-bump → invalidated_by_revision_bump
  Classification happens AFTER _check_dedup writes the current key;
  the classifier's skip-identical-keys rule prevents false positives.

tests/test_preflight_dedup_telemetry.py (NEW, 15 tests)
- 7 pin _dedup_miss_was_revision_bump classification:
  first call, revision differs only (M7a/c), file_paths differ
  (M7b — must NOT classify as revision bump), topic differs, TTL
  expiry, identical keys, missing _sync_state.
- 5 pin end-to-end telemetry emission:
  bypass emits one bypassed_revision_unknown; two-call revision-bump
  scenario emits one invalidated_by_revision_bump; first call alone
  emits nothing; cache hit emits nothing; file_paths shift does NOT
  emit revision_bump.
- 3 pin write_dedup_event direct contract:
  no-op when telemetry disabled, full-record shape when enabled,
  preflight_id omitted when None.

## Test results

- 15/15 new Phase 5 tests pass
- 143/143 in the broader Phase 4+5 + schema + dedup cluster

## Out of scope

- Dashboard wiring (separate operations PR)
- File-paths-shift attribution (M7b) — derivable from existing
  preflight_events.jsonl rows; not in Kevin's signoff
- Per-key TTL tuning — defer until telemetry shows it matters

Refs #87 Phase 5. Stacked on #309 (Phase 4) which is stacked on
#308 (v18 precondition).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@silongtan silongtan merged commit c045f9b into dev May 13, 2026
8 of 10 checks passed
@silongtan silongtan deleted the 87-ledger-updated-at-precondition branch May 13, 2026 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant