M1: golden decision tables (42 cases), plus two engine fixes #3

Merged
SellPL merged 1 commits from m1/golden-tables into m1/storage 2026-10-02 13:03:50 +00:00
Owner

What changed

The parity contract from CLAUDE.md section 7 now exists: 42 golden cases
across 11 scenario groups, a runner that asserts every case, and a coverage check
that asserts every mandatory scenario is claimed.

Stacked. This branch is m1/golden-tables based on m1/storage. Merge #2
(#1 first).

Which requirement this covers

Scenario Where
overnight time ranges table, 7 cases
DST changes table, partial — see below
timezone changes table, 5 cases
clock tampering fixtures in purity.rs
strictest-wins table, 4 cases
allowlist mode table, 3 cases
invalid condition combos table, all five section 1.2 pairings
strict mode lock rules table
never-block list is unblockable table, 2 cases
worst case 24h fixtures in purity.rs

Test evidence

$ scripts/ci.sh

  PASS  formatting
  PASS  clippy

  13 passed   blocker-core      (engine)
  22 passed   blocker-storage
  15 passed   blocker-tests     (golden runner, coverage, purity fixtures)
  PASS  tests

binary : /home/sp/.blocker-target/release/blocker-daemon
size   : 1430920 bytes (1.364 MB)
budget : 5242880 bytes (5 MB)
within budget, 3811960 bytes headroom
  PASS  size gate

  SKIP  android (no SDK)         — not a pass
  SKIP  gitleaks (not installed) — not a pass

all executed checks passed
EXIT=0

50 tests total, up from 44.

Two engine bugs the tables found

Both real, and both invisible to the existing unit tests.

1. Overnight windows woke the daemon immediately, forever.
time_window_boundary computed the end as end - minute and clamped at zero. For
a 22:00-06:00 window active at 23:00, "06:00 today" is already in the past, so
the boundary became now. The daemon would wake, re-evaluate, get the same
answer, and wake again — a busy loop spending the section 5 idle budget forever.
Not a missed block, which is why the existing tests missed it: they checked
whether the app is blocked and never checked when the engine wanted to wake.

2. An inactive schedule woke a day early.
The next-start search iterated day offsets to test membership but computed the
delta as start - minute alone, wrapping by 1440 without advancing the day. A
Monday-Friday schedule evaluated on Saturday reported Monday's start as 21 hours
away — Sunday 09:00, a day not in the set. The block still fired, because the
next evaluation recomputed it correctly, but the daemon woke a day early for
nothing.

Both are now pinned by cases that fail on the original code.

Four of my own cases were wrong too

The other half of why these are worth having. In the timezone group I asserted
that 10:00 UTC at UTC+5 falls outside a 09:00-17:00 window. It is 15:00 local,
which is inside. Same for both DST cases. The engine was right in all four; the
test was wrong.

One design change, flagged

ADR-16 originally specified JSON tables, justified by "serde_json will already
be in the tree". That was falsified in practice: rusqlite does not pull serde
by default, and uniffi has not landed. serde is not a section 3 dependency and
rule 0.3 requires asking before adding one.

So the tables are a Rust builder DSL. Same declarations, but a typo in a field
name is a compile error rather than a test that silently asserts the wrong thing.
For a document whose entire job is to be a contract, compile-time checking is
worth more than diffability. Converting to JSON later, if serde is approved, is
mechanical — only tests/src/golden.rs changes.

ADR-16 records the original reasoning and why it failed, rather than being quietly
rewritten.

One API change

BlockReason now carries its target. A schedule may name twenty targets and
they do not all become blocked — some are paused, some belong to an inactive
condition, some are on the never-block list. Without carrying which target a
reason is about, two blocked apps under one schedule produce two identical
reasons, and the UI has to re-derive the pairing. That is exactly the drift
BlockReason exists to prevent, and the tables could not assert per-app
attribution without it.

Notes for the reviewer

Coverage is checked, not promised. coverage::Location records whether a
scenario is covered by a table or a fixture, and both directions fail: an
uncovered scenario is an error, and so is a covers tag not on the mandatory
list — which is how a case quietly stops testing what its name claims.

Clock tampering and the 24-hour cap are deliberately not table-shaped. They
are about the passage of time and a bound across lock levels, not a decision at an
instant, so they live as code fixtures in purity.rs.

The DST gap is real and recorded. CivilTime is an epoch plus an offset with
no timezone database behind it. The tables pin that a fixed local window stays
fixed as the offset moves, including across a transition — but they cannot
express a spring-forward time that does not exist, or a fall-back time that is
ambiguous. Those are section 7 cases that are genuinely uncovered.
LIMITATIONS.md now says so, and it is the strongest argument for a
timezone-aware time type.

Wakeup::Any is the default. A case about blocking behaviour should not fail
because a window end moved. The three cases that carry the idle budget declare
their wakeup explicitly, and idle_budget_cases_declare_their_wakeup fails if one
of them stops doing so.

Checklist:

  • Cases added, and the two engine bugs they found fixed in the same commit
  • Conventional commit; scripts/ci.sh passes, skips stated
  • No fallible OS/IO call unwrapped outside tests
  • No new dependency — this PR adds none
  • No credentials in the diff
  • Full DST coverage (needs a timezone-aware time type)
  • Platform conformance runners (M2-Linux)

Do not merge this yourself

Squash or rebase. Merge #2 first — this branch is based on it.

## What changed The parity contract from CLAUDE.md section 7 now exists: **42 golden cases** across 11 scenario groups, a runner that asserts every case, and a coverage check that asserts every mandatory scenario is claimed. **Stacked.** This branch is `m1/golden-tables` based on `m1/storage`. Merge #2 (#1 first). ## Which requirement this covers | Scenario | Where | |---|---| | overnight time ranges | table, 7 cases | | DST changes | table, partial — see below | | timezone changes | table, 5 cases | | clock tampering | fixtures in `purity.rs` | | strictest-wins | table, 4 cases | | allowlist mode | table, 3 cases | | invalid condition combos | table, all five section 1.2 pairings | | strict mode lock rules | table | | never-block list is unblockable | table, 2 cases | | worst case 24h | fixtures in `purity.rs` | ## Test evidence ``` $ scripts/ci.sh PASS formatting PASS clippy 13 passed blocker-core (engine) 22 passed blocker-storage 15 passed blocker-tests (golden runner, coverage, purity fixtures) PASS tests binary : /home/sp/.blocker-target/release/blocker-daemon size : 1430920 bytes (1.364 MB) budget : 5242880 bytes (5 MB) within budget, 3811960 bytes headroom PASS size gate SKIP android (no SDK) — not a pass SKIP gitleaks (not installed) — not a pass all executed checks passed EXIT=0 ``` **50 tests total**, up from 44. ## Two engine bugs the tables found Both real, and both invisible to the existing unit tests. **1. Overnight windows woke the daemon immediately, forever.** `time_window_boundary` computed the end as `end - minute` and clamped at zero. For a 22:00-06:00 window active at 23:00, "06:00 today" is already in the past, so the boundary became *now*. The daemon would wake, re-evaluate, get the same answer, and wake again — a busy loop spending the section 5 idle budget forever. Not a missed block, which is why the existing tests missed it: they checked *whether* the app is blocked and never checked when the engine wanted to wake. **2. An inactive schedule woke a day early.** The next-start search iterated day offsets to test membership but computed the delta as `start - minute` alone, wrapping by 1440 without advancing the day. A Monday-Friday schedule evaluated on Saturday reported Monday's start as 21 hours away — Sunday 09:00, a day not in the set. The block still fired, because the next evaluation recomputed it correctly, but the daemon woke a day early for nothing. Both are now pinned by cases that fail on the original code. ## Four of my own cases were wrong too The other half of why these are worth having. In the timezone group I asserted that 10:00 UTC at UTC+5 falls *outside* a 09:00-17:00 window. It is 15:00 local, which is inside. Same for both DST cases. The engine was right in all four; the test was wrong. ## One design change, flagged ADR-16 originally specified **JSON** tables, justified by "serde_json will already be in the tree". That was falsified in practice: `rusqlite` does not pull `serde` by default, and `uniffi` has not landed. `serde` is not a section 3 dependency and rule 0.3 requires asking before adding one. So the tables are a **Rust builder DSL**. Same declarations, but a typo in a field name is a compile error rather than a test that silently asserts the wrong thing. For a document whose entire job is to be a contract, compile-time checking is worth more than diffability. Converting to JSON later, if serde is approved, is mechanical — only `tests/src/golden.rs` changes. ADR-16 records the original reasoning and why it failed, rather than being quietly rewritten. ## One API change `BlockReason` now carries its `target`. A schedule may name twenty targets and they do not all become blocked — some are paused, some belong to an inactive condition, some are on the never-block list. Without carrying which target a reason is about, two blocked apps under one schedule produce two identical reasons, and the UI has to re-derive the pairing. That is exactly the drift `BlockReason` exists to prevent, and the tables could not assert per-app attribution without it. ## Notes for the reviewer **Coverage is checked, not promised.** `coverage::Location` records whether a scenario is covered by a table or a fixture, and both directions fail: an uncovered scenario is an error, and so is a `covers` tag not on the mandatory list — which is how a case quietly stops testing what its name claims. Clock tampering and the 24-hour cap are deliberately **not** table-shaped. They are about the passage of time and a bound across lock levels, not a decision at an instant, so they live as code fixtures in `purity.rs`. **The DST gap is real and recorded.** `CivilTime` is an epoch plus an offset with no timezone database behind it. The tables pin that a fixed local window stays fixed as the offset moves, including across a transition — but they **cannot** express a spring-forward time that does not exist, or a fall-back time that is ambiguous. Those are section 7 cases that are genuinely uncovered. `LIMITATIONS.md` now says so, and it is the strongest argument for a timezone-aware time type. **`Wakeup::Any` is the default.** A case about blocking behaviour should not fail because a window end moved. The three cases that carry the idle budget declare their wakeup explicitly, and `idle_budget_cases_declare_their_wakeup` fails if one of them stops doing so. **Checklist:** - [x] Cases added, and the two engine bugs they found fixed in the same commit - [x] Conventional commit; `scripts/ci.sh` passes, skips stated - [x] No fallible OS/IO call unwrapped outside tests - [x] No new dependency — this PR adds none - [x] No credentials in the diff - [ ] Full DST coverage (needs a timezone-aware time type) - [ ] Platform conformance runners (M2-Linux) ## Do not merge this yourself Squash or rebase. Merge **#2 first** — this branch is based on it.
SellPL added 1 commit 2026-10-01 15:51:08 +00:00
feat(tests): add the golden decision tables, and fix two engine bugs they found
CI / ci (pull_request) Has been cancelled
CI / android (pull_request) Has been cancelled
f05836f528
The parity contract from CLAUDE.md section 7 now exists: 42 cases across 11
scenario groups, with a runner that asserts every case and a coverage check that
asserts every mandatory scenario is claimed.

**A design change, and why.** ADR-16 originally specified JSON tables, justified
by "serde_json will already be in the tree". That was wrong — `rusqlite` does not
pull `serde` by default and `uniffi` has not landed, so nothing provided it, and
`serde` is not a section 3 dependency (rule 0.3). The tables are therefore a Rust
builder DSL: the same declarations, but a typo in a field name is a compile error
rather than a test that silently asserts the wrong thing. For a document whose
entire job is to be a contract, compile-time checking beats diffability.
Converting to JSON later is mechanical if serde is approved.

**Two engine bugs the tables caught, both real:**

1. **Overnight windows woke the daemon immediately.** `time_window_boundary`
   computed the end as `end - minute` and clamped at zero. For a 22:00–06:00
   window active at 23:00, "06:00 today" is in the past, so the boundary became
   *now* — and the daemon would wake, re-evaluate, get the same answer, and wake
   again, forever. Not a missed block, but a busy loop burning the section 5 idle
   budget. Fixed by detecting the late half of an overnight window and crossing
   midnight properly.

2. **An inactive schedule woke a day early.** The next-start search iterated day
   offsets to test membership but computed the delta as `start - minute` only,
   wrapping by 1440 without advancing the day. A Monday–Friday schedule
   evaluated on Saturday reported Monday's start as 21 hours away — Sunday 09:00,
   a day not in the set. The block still fired, because the next evaluation
   recomputed it, but the daemon woke a day early for nothing. Fixed by folding
   the day offset into the delta.

**Four of my own table cases were wrong too**, which is the other half of why
they are worth having: a UTC+5 offset is 15:00 local and therefore *inside*
09:00–17:00 (I had asserted outside); the same for the DST cases. The engine was
right and the test was wrong in all four.

**Coverage bookkeeping is checked, not promised.** `coverage::Location` records
whether a scenario is covered by a table or by a code fixture — clock tampering
and the 24-hour cap are about the passage of time and a bound across lock levels,
not a decision at an instant, so they cannot be expressed as data. Both
directions fail: an uncovered scenario is an error, and so is a `covers` tag that
is not on the mandatory list, which is how a case quietly stops testing what its
name claims.

**`BlockReason` now carries its target.** A schedule may name twenty targets and
they do not all become blocked — some are paused, some belong to a condition that
is not currently active, some are on the never-block list. Without carrying which
target a reason is about, two blocked apps under one schedule produce two
identical reasons and the UI has to re-derive the pairing, which is exactly the
drift `BlockReason` exists to prevent. The tables could not assert per-app
attribution without it.

**The DST gap is recorded, not papered over.** `CivilTime` is an epoch plus an
offset with no timezone database behind it, so the tables pin that a fixed local
window stays fixed as the offset moves — including across a DST transition — but
cannot express a spring-forward time that does not exist or a fall-back time that
is ambiguous. Those are section 7 cases that are genuinely not covered, and
LIMITATIONS.md now says so.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Author
Owner

Review approval: checked against test suite and approved per user instruction.

Review approval: checked against test suite and approved per user instruction.
SellPL merged commit 1318515103 into m1/storage 2026-10-02 13:03:50 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: SellPL/blocker#3