milestone 24: persist and surface the unsupported-line count
This commit is contained in:
@@ -0,0 +1,497 @@
|
||||
# Milestone 24: `skipped_unsupported` is persisted, surfaced and explained
|
||||
|
||||
Goal: close the compile-pipeline silent drop that misleads the operator. `Counts.
|
||||
skipped_unsupported` (`src/filter/compiler.zig:34`) is counted on every compile
|
||||
(compiler.zig:92) and then discarded on the happy path — it reaches the
|
||||
compiled-file header and the `NoValidEntries` error text, but no database
|
||||
column, no API field and no UI cell. Its sibling `skipped_regex` reaches all
|
||||
three. A blocklist made almost entirely of cosmetic browser filters therefore
|
||||
compiles to almost nothing and looks, in the UI, exactly like a clean list.
|
||||
AGENTS.md forbids exactly this ("no silent drops"). The milestone persists the
|
||||
count, surfaces it everywhere `skipped_regex` is surfaced, and explains to the
|
||||
operator why the two skip counters are different facts.
|
||||
|
||||
Design written 2026-08-13 against HEAD `21571e4`, revised after a Codex review
|
||||
of the first draft.
|
||||
|
||||
## Implementation contract (read first)
|
||||
|
||||
- Read `AGENTS.md`, then this spec whole, before session work starts.
|
||||
- **The v1 baseline is editable and there is no migration step.** As of commit
|
||||
`21571e4`, `src/storage/migrations.zig` holds exactly one step and
|
||||
`target_version` is 1 (migrations.zig:29-36). nxdns has zero installs; the
|
||||
baseline freezes at v0.1 (`config_schema.zig:6-12`, PLAN §3.7, §11.2). The
|
||||
new column is one line edited into `config_schema.ddl_v1` plus the identical
|
||||
line in PLAN §11.2, which is kept byte-identical to it. A diff that adds a
|
||||
`ddl_v2` or a second `Step` is wrong and must be reverted, not merged.
|
||||
- After the API shape change, regenerate the contract samples
|
||||
(`web/src/lib/contractSamples.gen.ts`; procedure in AGENTS.md — the
|
||||
`zig build test -Dintegration -Dcontract-samples-out=...` command, never a
|
||||
hand edit) and update `web/src/lib/types.ts` to match.
|
||||
- No new `src/**.zig` files, so `src/tests.zig` and `build.zig` are untouched.
|
||||
|
||||
## Rulings (binding)
|
||||
|
||||
### 1. The column is `skipped_unsupported_count`, one edited line, no step
|
||||
|
||||
`skipped_regex_count` (config_schema.zig:63) sets the convention; the new
|
||||
column follows it. In `config_schema.ddl_v1`, directly after the
|
||||
`skipped_regex_count` line inside `CREATE TABLE blocklist_sources`:
|
||||
|
||||
```sql
|
||||
skipped_unsupported_count INTEGER NOT NULL DEFAULT 0,
|
||||
```
|
||||
|
||||
The identical line goes into PLAN §11.2 after PLAN.md:421 — that section is
|
||||
kept byte-identical to `ddl_v1` and currently is (verified: the fenced SQL
|
||||
matches the Zig string literal line for line).
|
||||
|
||||
Consequence to accept, not to fix: a development database stamped version 1
|
||||
before this edit will fail the first `SELECT` naming the column with "no such
|
||||
column", because the stamped version equals `target_version` and the step never
|
||||
reruns. That is the documented pre-v0.1 contract (`config_schema.zig:6-12`) —
|
||||
delete the scratch database. Do not add fallback SQL, `PRAGMA table_info`
|
||||
probing, or a migration step to paper over it.
|
||||
|
||||
The baseline test "a fresh database reaches the baseline with every v1 column
|
||||
and rule kind" (migrations.zig:266-283) gains
|
||||
`try testing.expect(try columnExists(&database, "blocklist_sources", "skipped_unsupported_count"));`
|
||||
beside the existing `exception_count` probe.
|
||||
|
||||
### 2. The repo persists it beside `skipped_regex_count`
|
||||
|
||||
`src/storage/repositories/sources_repo.zig`, mirroring `skipped_regex_count`
|
||||
exactly (no default on either struct field, so every construction site fails
|
||||
to compile until it names the count — that is the visibility this milestone is
|
||||
about):
|
||||
|
||||
- `SourceRow` (sources_repo.zig:80-97) gains `skipped_unsupported_count: i64`
|
||||
after `skipped_regex_count`.
|
||||
- `SourceStats` (sources_repo.zig:99-110) gains the same field.
|
||||
- `row_columns_sql` (sources_repo.zig:112-117) appends
|
||||
`skipped_unsupported_count` to the SELECT list (index 11);
|
||||
`readSourceRow` (sources_repo.zig:128-148) reads it with
|
||||
`stmt.columnInt(11)`.
|
||||
- `update_stats_sql` (sources_repo.zig:159-164) adds
|
||||
`skipped_unsupported_count = ?8`; `updateSourceStats`
|
||||
(sources_repo.zig:168-179) binds it.
|
||||
- The module doc comment (sources_repo.zig:3-5) adds the column to its list of
|
||||
server-produced facts, and so does the `src/config/model.zig:14` doc comment
|
||||
that repeats that list.
|
||||
|
||||
**The sum at sources_repo.zig:318 includes the new column.** That sum lives in
|
||||
the test "insertBlocklistSource leaves the runtime columns at their defaults";
|
||||
it exists to prove an insert leaves every runtime counter at 0, and the new
|
||||
column is a runtime counter. This is a deliberate decision, not an oversight:
|
||||
no production query sums these columns into a "total entries" figure, and none
|
||||
may start to — `skipped_regex_count` and `skipped_unsupported_count` count
|
||||
lines *not* written, unlike `domain_count`, `wildcard_count` and
|
||||
`exception_count`, so any future entries total must exclude both. The test sum
|
||||
asserts defaults, which is the one context where adding them is correct.
|
||||
|
||||
Existing `SourceStats` / assertion sites in this file (the literals at
|
||||
sources_repo.zig:386-393, 420-427, 483-493 and the zero-default loop at
|
||||
sources_repo.zig:365-374) carry the new field with distinct non-zero values
|
||||
where their siblings have them, and the round-trip assertions extend to it.
|
||||
|
||||
### 3. The manager writes it, and rehydration restores it
|
||||
|
||||
`src/filter/manager.zig`. The header already prints
|
||||
`# skipped_unsupported {d}` (manager.zig:243) and `SourceStatus.counts` is a
|
||||
full `compiler.Counts` (manager.zig:178), so within one process the count
|
||||
already reaches the status table. What is missing is the database, which is
|
||||
the only thing a restart reads — rehydration never reparses headers.
|
||||
|
||||
- The fresh-publish path (manager.zig:854-861) passes
|
||||
`.skipped_unsupported_count = compiled.result.counts.skipped_unsupported` to
|
||||
`updateSourceStats`.
|
||||
- **The checksum-unchanged path (manager.zig:821-836) carries a bug this
|
||||
milestone fixes.** It writes the *stored* `row.skipped_regex_count` back to
|
||||
the database (manager.zig:830) while handing the *fresh*
|
||||
`compiled.result.counts` to the in-memory status (manager.zig:833). The
|
||||
checksum covers the `.list`, `.wild` and `.allow` bodies only, and a skipped
|
||||
line lands in none of them — so a list that changes only its regex or
|
||||
browser-syntax lines keeps its checksum, the running server shows the new
|
||||
number, the database keeps the old one, and the next restart silently reverts
|
||||
what the operator saw. That is milestone 21's column, wrong today.
|
||||
|
||||
Both skip counters take `compiled.result.counts` on this path:
|
||||
`.skipped_regex_count = compiled.result.counts.skipped_regex` and
|
||||
`.skipped_unsupported_count = compiled.result.counts.skipped_unsupported`.
|
||||
`domain_count`, `wildcard_count` and `exception_count` keep reading from
|
||||
`row` — they count written entries, so an unchanged checksum does mean an
|
||||
unchanged value for them.
|
||||
|
||||
This needs a test the author has watched fail with the fix reverted: two
|
||||
compiles of bodies whose written entries are identical but whose skipped
|
||||
lines differ, asserting the same checksum, then asserting the database holds
|
||||
the second compile's counts, then rehydrating through `applyLoadOutcomes` to
|
||||
prove the restored status carries them. Report the observed failure output.
|
||||
- `applyLoadOutcomes` rehydration (manager.zig:1545-1550) adds
|
||||
`.skipped_unsupported = countOf(row.skipped_unsupported_count)` to the
|
||||
`Counts` it rebuilds, so a restarted server reports what the last compile
|
||||
skipped instead of 0.
|
||||
- Test literals name the field: one `SourceStats` (manager.zig:1928) and two
|
||||
`SourceRow` (manager.zig:2051, 2225). The rehydration test that feeds a row
|
||||
through
|
||||
`applyLoadOutcomes` asserts a non-zero `skipped_unsupported` lands in
|
||||
`status.counts`. The header pin test (manager.zig:2030-2031) already covers
|
||||
the `# skipped_unsupported` line and is extended only if its fixture counts
|
||||
change.
|
||||
- `rejectedWithoutEntries` (manager.zig:1640-1643) and `failNoValidEntries`
|
||||
(manager.zig:1127-1128) already read the count and are unchanged.
|
||||
|
||||
`src/filter/filter_integration_test.zig` names `.skipped_regex_count` in six
|
||||
`SourceStats` literals (:826, :858, :1075, :1145, :1257, :2158) — each gains
|
||||
the sibling field. The stats round-trip assertion at :915 gains a sibling
|
||||
assertion for the new column.
|
||||
|
||||
**Do not add an unsupported line to the existing fixture.** The fixture at
|
||||
filter_integration_test.zig:112 is hosts-shaped, and `detectFormat`
|
||||
(`src/filter/parsers.zig:118`) assigns one format to a whole source. A single
|
||||
`##.ad-banner` or `$`-modifier line flips it to ABP, which re-parses every
|
||||
existing line: the `*.wild` entry becomes unsupported and address fields
|
||||
tokenize as domains (`src/filter/parser_abp.zig:68`). Every expected count in
|
||||
that test would move, for a reason unrelated to this milestone.
|
||||
|
||||
Add a **separate ABP-format fixture and test** carrying `##.ad-banner` and
|
||||
`||ads.example^$third-party` beside two blockable names, and assert
|
||||
`skipped_unsupported_count = 2` through the compile-persist-read round trip
|
||||
there.
|
||||
|
||||
`src/config/reconcile.zig`: the runtime-column preservation test seeds stats
|
||||
at reconcile.zig:1096-1110 and asserts survival at reconcile.zig:1173. The
|
||||
seed gains `.skipped_unsupported_count` with a distinct value and the
|
||||
assertion block gains its expectation. No reconcile logic changes: the engine
|
||||
updates declarative columns by name and never touches runtime columns, so the
|
||||
new column survives with zero code change — the test is the proof.
|
||||
|
||||
### 4. The API speaks it in both shapes
|
||||
|
||||
Two shapes carry blocklist counters, and the new count joins both under the
|
||||
names its siblings set:
|
||||
|
||||
- **`Blocklist`** (rows of `GET /api/blocklists`, serialized straight from
|
||||
`SourceRow`): the field arrives automatically once `SourceRow` has it, as
|
||||
`skipped_unsupported_count`. `src/web/openapi.yaml:1877-1895` adds it to
|
||||
`required` and `properties`.
|
||||
- **`SourceStatus`** (rows of `POST /api/blocklists/update`): `StatusView`
|
||||
(`src/web/handlers/blocklists.zig:52-80`) gains
|
||||
`skipped_unsupported: u32` after `skipped_regex`, mapped from
|
||||
`status.counts.skipped_unsupported` in `from`.
|
||||
`src/web/openapi.yaml:1920-1938` adds it to `required` and `properties`.
|
||||
The handler test literals at blocklists.zig:323 (`SourceStats`) and :386
|
||||
(`counts`) carry the field with non-zero values and the response assertions
|
||||
extend to it.
|
||||
|
||||
Contract fallout, all in the same session (ruling 6 explains why):
|
||||
|
||||
- Regenerate `web/src/lib/contractSamples.gen.ts` with the AGENTS.md command.
|
||||
- `web/src/lib/types.ts`: `Blocklist` gains
|
||||
`skipped_unsupported_count: number` (types.ts:154-166); `SourceStatus`
|
||||
gains `skipped_unsupported: number` (types.ts:183-195).
|
||||
- The web test mocks are **not** typed against these interfaces, so `tsc` will
|
||||
not force them. `BLOCKLISTS` in
|
||||
`web/src/features/blocklists/BlocklistsPage.test.tsx` is an inferred object
|
||||
literal handed to a `Record<string, unknown>` (BlocklistsPage.test.tsx:40),
|
||||
and the same untyped-fetch pattern holds in
|
||||
`web/src/features/groups/GroupsPage.test.tsx:16` and
|
||||
`web/src/features/settings/authority.test.tsx:64` — both of which already
|
||||
omit `exception_count` without failing. Updating a mock here is a semantic
|
||||
fixture change, not a typecheck fix, and the implementer must not expect a
|
||||
compiler error to point at them.
|
||||
|
||||
So: the BlocklistsPage mocks (:21, :34 blocklist rows; :104, :155, :168
|
||||
status rows) gain the field because S2's rendering assertions read it. The
|
||||
groups and settings mocks are left alone — they render no counter column,
|
||||
and widening them buys nothing. S1 picks values no other cell in the same
|
||||
table already shows (`3` and `7` are taken), e.g.
|
||||
`skipped_unsupported_count: 21` and `skipped_unsupported: 17`.
|
||||
|
||||
### 5. The UI shows it always, in both tables, and says what it means
|
||||
|
||||
Both tables gain a `Skipped unsupported` column directly after
|
||||
`Skipped regex`, rendered unconditionally:
|
||||
|
||||
- `web/src/features/blocklists/BlocklistsPage.tsx`: header after :157, cell
|
||||
`{b.skipped_unsupported_count}` after :190, same
|
||||
`shared.td, shared.tabularNums` props as its neighbours.
|
||||
- `web/src/features/blocklists/SourceStatusSection.tsx`: header after :91,
|
||||
cell `{source.skipped_unsupported}` after :114.
|
||||
|
||||
Always-shown is a decision, not a default: every other counter column here is
|
||||
unconditional, including `Skipped regex` and `Exceptions`, which are 0 for
|
||||
every plain hosts list, and the tutorial already explains those zeros. A
|
||||
column that appears only when non-zero would make two lists' tables disagree
|
||||
in shape, would hide the header that gives the number its meaning, and would
|
||||
special-case exactly the counter this milestone exists to make visible. The
|
||||
zero cell is not noise; it states that nothing in this list was classified as
|
||||
unsupported — which is narrower than "clean", since `invalid` and `long_lines`
|
||||
stay unsurfaced (ruling 7).
|
||||
|
||||
The two counters mean different things and the page must say so once. A muted
|
||||
paragraph (the existing `styles.empty`-style muted text, matching
|
||||
`SourceStatusSection`'s `note` treatment) rendered under the sources table in
|
||||
`BlocklistsPage.tsx`, inside the same `else` branch as the table — the note
|
||||
describes the two skip columns, so it appears exactly when they do. "Always
|
||||
visible" above means unconditional on values, never hidden at 0; it does not
|
||||
mean the empty state (`blocklists.length === 0`) carries a paragraph about
|
||||
columns that are not on screen. The empty state stays one instruction. Exact
|
||||
copy:
|
||||
|
||||
> Both “Skipped” columns count lines nxdns read and did not take. Skipped
|
||||
> regex lines are patterns nxdns accepts only from you — adopt one you trust
|
||||
> as a regex rule. Skipped unsupported lines are syntax nxdns cannot translate
|
||||
> into a DNS decision: cosmetic element hiding, browser-only modifiers. A
|
||||
> skipped unsupported count that dwarfs the domain count usually means the
|
||||
> list is written for a browser extension, and its DNS or hosts variant will
|
||||
> block more here.
|
||||
|
||||
`BlocklistsPage.test.tsx` asserts: both new headers render, the mock values
|
||||
(`21`, and `17` after an update snapshot) render, and the note text is
|
||||
present. No threshold logic, no badge, no coloring by magnitude (see
|
||||
anti-requirements).
|
||||
|
||||
### 6. The docs explain both counters without contradicting what stands
|
||||
|
||||
- `docs/reference/configuration.md`, section `### blocklist_sources`, after
|
||||
the exceptions paragraph (configuration.md:282-288), a new paragraph:
|
||||
|
||||
> Two counters report what a compile skipped, and they are different facts.
|
||||
> `skipped_regex` counts regex lines: nxdns has a regex engine, but it takes
|
||||
> patterns only from the operator, so a regex line in a downloaded list is
|
||||
> counted, skipped and surfaced — adopt the ones you trust as `regex` rules.
|
||||
> `skipped_unsupported` counts lines nxdns cannot safely translate into a DNS
|
||||
> decision: cosmetic element hiding (`##`, `#@#`, `#?#`), rules carrying a
|
||||
> `$` modifier (except `$important` on an exception line, tolerated above),
|
||||
> scheme anchors, non-anchored `@@` forms — and, in a
|
||||
> `domains`-format list, a line holding more than one field before its
|
||||
> inline comment, which usually means the list is really a hosts file that
|
||||
> was declared as `domains`. Neither is an error, and the two are never one
|
||||
> number. A large `skipped_unsupported` beside a small `domain_count`
|
||||
> usually means the list is written for browser extensions, and its DNS or
|
||||
> hosts variant will block more here. Both appear per source in the
|
||||
> blocklists UI and on `/api/blocklists`.
|
||||
|
||||
- `docs/tutorial/first-run.md`: the recorded JSON at first-run.md:223 gains
|
||||
`"skipped_unsupported":0` after `"skipped_regex":0` (the endpoint now
|
||||
returns it; the recorded values themselves stand). The paragraph at
|
||||
first-run.md:226-231 becomes four zeros, keeps its `skipped_regex` sentence
|
||||
as written, and **drops the "parts of this list that a hosts file cannot
|
||||
have" framing** — a deliberate small widening ruled during implementation
|
||||
review. The framing was false before this milestone touched it:
|
||||
`parser_hosts.zig` recognizes regex lines (:16), reports a bare sink
|
||||
address as unsupported (:21), and a `*.`-prefixed name compiles to a
|
||||
wildcard in any format. Only `exceptions` is Adblock-Plus-only. The
|
||||
paragraph now presents the zeros as facts about this particular download
|
||||
and says so.
|
||||
- The `$` modifier is named as skipped **with its one exception**: `$important`
|
||||
on an anchored `@@` exception line is accepted and lands in
|
||||
`exception_count` (`parser_abp.zig`, PLAN §2.2). The configuration.md
|
||||
paragraph above, and the `SourceRow.skipped_unsupported_count` doc comment
|
||||
in `sources_repo.zig` that mirrors it, both carry the qualifier — an
|
||||
unqualified "rules carrying a `$` modifier" contradicts the parser.
|
||||
- PLAN.md:101 (§3.8) currently reads "regex lines are counted + skipped
|
||||
(counts in metadata → UI)". It becomes: regex lines *and* browser-syntax
|
||||
lines are counted and skipped, both counts in metadata → UI. One sentence;
|
||||
§2.2 (PLAN.md:36-37) already says unsupported forms "stay unsupported and
|
||||
counted" and needs no edit.
|
||||
- `docs/reference/files-and-directories.md` is untouched: the compiled-file
|
||||
header already carried `# skipped_unsupported` before this milestone and
|
||||
the file table there does not enumerate header lines.
|
||||
|
||||
### 7. The other three counters stay out, and the reasons differ per counter
|
||||
|
||||
The first draft claimed `invalid`, `long_lines` and `duplicates` were already
|
||||
"header-and-error-path only". That is false:
|
||||
|
||||
- `invalid` reaches the compiled-file header (manager.zig:244) and the
|
||||
`NoValidEntries` text (manager.zig:1127).
|
||||
- `long_lines` reaches the `NoValidEntries` text only — not the header.
|
||||
- `duplicates` reaches **nothing**: counted at compile, discarded on every
|
||||
path.
|
||||
|
||||
They stay out of scope, and not for one shared reason — which is why the goal
|
||||
above says "the silent drop that misleads the operator" rather than "the one
|
||||
silent drop left":
|
||||
|
||||
- `duplicates` hides no failure. A deduplicated name still blocks; the count
|
||||
measures input redundancy, not lost coverage. Discarding it discards a
|
||||
curiosity, so "no silent drops" does not reach it — there is no drop.
|
||||
- `invalid` and `long_lines` do measure dropped lines, and they remain only
|
||||
partially surfaced. The catastrophic form — a download that is all rejects —
|
||||
already fails loudly (`rejectedWithoutEntries`, state `no_valid_entries`);
|
||||
the residual is a list that loses some lines and still loads, visible in
|
||||
the header file and nowhere the UI reaches. That residual is real, it is
|
||||
recorded here deliberately, and it is not this milestone: the two skip
|
||||
counters name an action the operator can take (adopt the patterns; fetch
|
||||
the DNS variant), while these two say only "the list is malformed", which
|
||||
no column makes more actionable.
|
||||
|
||||
The decision is reversible; a later milestone can widen the table. What this
|
||||
spec may not do is claim the three are visible when `duplicates` is not.
|
||||
|
||||
### 8. Export, import and reconcile change nothing
|
||||
|
||||
`nxdns export` / `import` carry configuration; the compile counters are facts
|
||||
a running server produces. `model.BlocklistSource` holds only the four
|
||||
configuration columns, the insert leaves runtime columns at their defaults
|
||||
(sources_repo.zig:1-9, :48-61), and the dump helpers used by the import and
|
||||
reconcile suites are `SELECT *` compared dump-to-dump
|
||||
(`src/config/import.zig:167-186`, `src/config/reconcile.zig:988`), so no
|
||||
golden text names columns and the new column appears in both sides of every
|
||||
comparison. Ruling 3's reconcile-test extension is the only touch in
|
||||
`src/config/`, plus the model.zig:14 comment from ruling 2.
|
||||
|
||||
## Sessions
|
||||
|
||||
Two sessions, strictly sequential: S1 then S2. No parallelism — deliberately.
|
||||
The regenerated `contractSamples.gen.ts` typechecks only against a `types.ts`
|
||||
that already carries the new fields, and the samples are produced by a Zig
|
||||
integration run, so the generator and the interface it must satisfy sit on
|
||||
opposite sides of the language boundary and cannot land independently. S2's
|
||||
rendering assertions then read fields that only exist once S1 has landed both.
|
||||
|
||||
(The mocks are *not* part of this argument: they are untyped, so widening
|
||||
`types.ts` does not break them. The first draft claimed otherwise.)
|
||||
|
||||
### Session S1: column, persistence, API, contract
|
||||
|
||||
Owns: `src/storage/config_schema.zig`, `src/storage/migrations.zig` (test
|
||||
only), `src/storage/repositories/sources_repo.zig`, `src/filter/manager.zig`,
|
||||
`src/filter/filter_integration_test.zig`, `src/config/reconcile.zig` (test
|
||||
only), `src/config/model.zig` (comment only),
|
||||
`src/web/handlers/blocklists.zig`, `src/web/openapi.yaml`,
|
||||
`web/src/lib/types.ts`, `web/src/lib/contractSamples.gen.ts` (regenerated,
|
||||
never hand-edited), `web/src/features/blocklists/BlocklistsPage.test.tsx`
|
||||
(mock fields only — no rendering assertions),
|
||||
`PLAN.md` (the §11.2 line and
|
||||
the §3.8 sentence).
|
||||
|
||||
- S1.1 rulings 1 and 2: the DDL line in both copies, the repo columns, the
|
||||
migrations and repo tests.
|
||||
- S1.2 ruling 3: both `updateSourceStats` call sites, rehydration, test
|
||||
literals, the reconcile preservation assertion.
|
||||
- S1.3 ruling 4: `StatusView`, openapi.yaml, sample regeneration, `types.ts`,
|
||||
mock fields.
|
||||
|
||||
Acceptance (S1):
|
||||
- [ ] `zig build test` passes; the migrations baseline test proves
|
||||
`blocklist_sources.skipped_unsupported_count` exists.
|
||||
- [ ] A compile of the new **ABP-format** fixture carrying `##.ad-banner` and
|
||||
`||ads.example^$third-party` persists `skipped_unsupported_count = 2`
|
||||
through `updateSourceStats` and reads it back through `listSourceRows`.
|
||||
The existing hosts-shaped fixture is unchanged, and every count it
|
||||
already asserts still holds.
|
||||
- [ ] A second compile whose written entries are byte-identical but whose
|
||||
skipped lines differ produces the same checksum and still updates both
|
||||
skip counters in the database; the test was watched failing with the
|
||||
manager.zig:830 fix reverted, and the failure output is recorded.
|
||||
- [ ] `applyLoadOutcomes` fed a row with `skipped_unsupported_count = 5` and
|
||||
no live refresh yields `status.counts.skipped_unsupported == 5`.
|
||||
- [ ] `zig build test -Dintegration` passes, and the regenerated
|
||||
`contractSamples.gen.ts` carries `skipped_unsupported_count` in the
|
||||
Blocklist sample and `skipped_unsupported` in the SourceStatus sample.
|
||||
- [ ] `cd web && npm run typecheck && npm test` pass with the widened types.
|
||||
|
||||
### Session S2: UI and docs (needs S1)
|
||||
|
||||
Owns: `web/src/features/blocklists/BlocklistsPage.tsx`,
|
||||
`web/src/features/blocklists/SourceStatusSection.tsx`,
|
||||
`web/src/features/blocklists/BlocklistsPage.test.tsx` (rendering assertions),
|
||||
`docs/reference/configuration.md`, `docs/tutorial/first-run.md`.
|
||||
|
||||
- S2.1 ruling 5: both columns, the note paragraph, the rendering assertions.
|
||||
- S2.2 ruling 6: the two doc edits.
|
||||
|
||||
Acceptance (S2):
|
||||
- [ ] `cd web && npm run typecheck && npm test && npm run lint` pass; the new
|
||||
tests assert both `Skipped unsupported` headers, the mock values and the
|
||||
note text.
|
||||
- [ ] `npm run build` passes (`assert-css-layers.mjs` runs inside it).
|
||||
- [ ] `docs/tutorial/first-run.md` no longer says "three zeros", its JSON
|
||||
sample carries `skipped_unsupported`, and its `skipped_regex` sentence
|
||||
is unchanged.
|
||||
|
||||
### Orchestrator
|
||||
|
||||
Verify S1 acceptance before starting S2. After S2, run the full gate set
|
||||
(`zig build test`, `zig build test -Dintegration`, `test-aarch64` if qemu is
|
||||
present, `cd web && npm test`, `npm run assert-bundled`), then a live smoke
|
||||
against a scratch server with two real sources:
|
||||
|
||||
- `https://easylist.to/easylist/easylist.txt` — a browser-targeted list;
|
||||
expect a `skipped_unsupported` several times its `domains` (do not assert an
|
||||
exact number; assert the ratio and non-zero).
|
||||
- `https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts` — expect
|
||||
`skipped_unsupported` 0.
|
||||
|
||||
Verify the numbers appear in the `POST /api/blocklists/update` response, in
|
||||
`GET /api/blocklists`, and in both UI tables. Then restart the server and
|
||||
verify the counts survive into the status table without a refresh — that is
|
||||
ruling 3's rehydration working against the real database. Record deviations
|
||||
in `## Recorded (implementation)`.
|
||||
|
||||
## Module layout
|
||||
|
||||
New files: none. Deleted surface: none.
|
||||
|
||||
## File ownership
|
||||
|
||||
| File | Session |
|
||||
| --- | --- |
|
||||
| `src/storage/config_schema.zig` | S1 |
|
||||
| `src/storage/migrations.zig` | S1 |
|
||||
| `src/storage/repositories/sources_repo.zig` | S1 |
|
||||
| `src/filter/manager.zig` | S1 |
|
||||
| `src/filter/filter_integration_test.zig` | S1 |
|
||||
| `src/config/reconcile.zig` (test), `src/config/model.zig` (comment) | S1 |
|
||||
| `src/web/handlers/blocklists.zig`, `src/web/openapi.yaml` | S1 |
|
||||
| `web/src/lib/types.ts`, `web/src/lib/contractSamples.gen.ts` | S1 |
|
||||
| `PLAN.md` (§11.2 line, §3.8 sentence) | S1 |
|
||||
| `web/src/features/blocklists/BlocklistsPage.test.tsx` | S1 (mock fields), then S2 (assertions) — sequential, never concurrent |
|
||||
| `web/src/features/blocklists/BlocklistsPage.tsx`, `web/src/features/blocklists/SourceStatusSection.tsx` | S2 |
|
||||
| `docs/reference/configuration.md`, `docs/tutorial/first-run.md` | S2 |
|
||||
|
||||
## Acceptance (milestone complete)
|
||||
|
||||
- [ ] All session acceptance boxes above.
|
||||
- [ ] `config_schema.ddl_v1` and PLAN §11.2 are byte-identical, both carrying
|
||||
the new line; `migrations.steps` still holds exactly one step and
|
||||
`target_version` is still 1.
|
||||
- [ ] The live smoke: EasyList shows a large `skipped_unsupported` and
|
||||
StevenBlack shows 0, in the API and in both UI tables, and both values
|
||||
survive a server restart.
|
||||
- [ ] `nxdns export` output is byte-identical before and after a refresh that
|
||||
wrote the new column (runtime columns stay out of exports).
|
||||
- [ ] The regenerated contract samples typecheck against `web/src/lib/types.ts`
|
||||
— that is what the sample mechanism proves, and it covers server-to-
|
||||
TypeScript agreement only.
|
||||
- [ ] `src/web/openapi.yaml` is reviewed **by hand** against the two changed
|
||||
response shapes, and the reviewer says so in `## Recorded`. No automated
|
||||
guard covers this: `web_integration_test.zig:2089` checks that routes and
|
||||
methods exist and `:2575` counts operations, but nothing compares a
|
||||
component schema to a real response. An openapi.yaml that omits the new
|
||||
field will pass every gate in this repo.
|
||||
|
||||
## Anti-requirements
|
||||
|
||||
- No migration step, no `ddl_v2`, no runtime schema probing. The baseline is
|
||||
edited in place per §3.7; a pre-edit scratch database is deleted, not
|
||||
reconciled.
|
||||
- No merging of the two skip counters into one number, anywhere — not in the
|
||||
API, not in a UI total, not in prose. They are different facts.
|
||||
- No "this list targets browsers" heuristic: no threshold, badge, warning
|
||||
color or ratio computation in the UI. The number plus the note paragraph is
|
||||
the surface; a cutoff would be an invented policy.
|
||||
- No conditional rendering of the new column. It shows at 0 like every other
|
||||
counter column.
|
||||
- No change to `nxdns export` / `import` ZON: compile statistics are not
|
||||
configuration.
|
||||
- No per-line diagnostics, no sample of skipped lines in logs, API or UI —
|
||||
counters and health surfaces, not log spam (AGENTS.md).
|
||||
- No change to `rejectedWithoutEntries` or `failNoValidEntries` semantics.
|
||||
- No hand edits to `contractSamples.gen.ts`.
|
||||
- No new columns beyond the one. See ruling 7 for what that leaves open and
|
||||
why — the reason is a scope decision, not a claim that the other counters
|
||||
are already visible.
|
||||
Reference in New Issue
Block a user