diff --git a/PLAN.md b/PLAN.md index 7c428df..0e2ed71 100644 --- a/PLAN.md +++ b/PLAN.md @@ -98,7 +98,7 @@ Two SQLite files with opposite write profiles, isolated from each other: Blocklist domains are **not** stored in SQLite — they are a cache of re-downloadable remote artifacts, not config or state: -- Each source compiles to `/var/lib/nxdns/blocklists/.list`: normalized, one domain per line, small header (source URL, fetch time, counts, checksum). Wildcard/regex/exception-flavored lines: wildcards go to `.wild`, ABP exceptions (`@@||name^`) to `.allow`; regex lines are counted + skipped (counts in metadata → UI). The checksum covers the three bodies in that order, so a source with no exceptions keeps the digest it had when only two existed and no upgrade forces a refetch. +- Each source compiles to `/var/lib/nxdns/blocklists/.list`: normalized, one domain per line, small header (source URL, fetch time, counts, checksum). Wildcard/regex/exception-flavored lines: wildcards go to `.wild`, ABP exceptions (`@@||name^`) to `.allow`; regex lines and browser-syntax lines nxdns cannot translate into a DNS decision are counted + skipped, both counts in metadata → UI. The checksum covers the three bodies in that order, so a source with no exceptions keeps the digest it had when only two existed and no upgrade forces a refetch. - `config.db` keeps source **metadata only** (`blocklist_sources`). - Startup + post-update: parse files into the immutable in-memory matcher (RCU swap, §9.5). - Corruption recovery is per-file: checksum mismatch → re-download one list. @@ -419,6 +419,7 @@ CREATE TABLE blocklist_sources ( wildcard_count INTEGER NOT NULL DEFAULT 0, exception_count INTEGER NOT NULL DEFAULT 0, skipped_regex_count INTEGER NOT NULL DEFAULT 0, + skipped_unsupported_count INTEGER NOT NULL DEFAULT 0, checksum TEXT ); diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 7bf1a06..45b395e 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -287,6 +287,21 @@ first, so a downloaded list can reopen only a hole another downloaded list dug. Each source reports how many it carried as `exceptions`; there is no way to write one by hand, and no reason to want one — write an allow rule instead. +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`. + ### group_sources Which groups consult which blocklist sources. diff --git a/docs/tutorial/first-run.md b/docs/tutorial/first-run.md index 8a936be..780443a 100644 --- a/docs/tutorial/first-run.md +++ b/docs/tutorial/first-run.md @@ -220,15 +220,18 @@ curl -s -X POST http://127.0.0.1:8080/api/blocklists/update ``` ```json -{"sources":[{"id":1,"state":"ok","loaded":true,"last_attempt":1786629237,"last_success":1786629238,"url":"https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts","last_error":"","domains":97648,"wildcards":0,"exceptions":0,"skipped_regex":0}]} +{"sources":[{"id":1,"state":"ok","loaded":true,"last_attempt":1786629237,"last_success":1786629238,"url":"https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts","last_error":"","domains":97648,"wildcards":0,"exceptions":0,"skipped_regex":0,"skipped_unsupported":0}]} ``` -The three zeros are the parts of this list that a hosts file cannot have. +The four zeros describe this particular download, not the hosts format. `wildcards` counts entries covering a name and its subdomains, `exceptions` counts the `@@` lines an Adblock Plus list uses to lift a name another list blocks, and `skipped_regex` counts the regex lines nxdns declines to take from a -downloaded list. Only the Adblock Plus syntax writes any of them; a hosts file is -one name per line. +downloaded list. `skipped_unsupported` counts lines nxdns cannot translate into a +DNS decision, and a large value next to a small `domains` means the list targets +browsers rather than DNS. Only `exceptions` is Adblock-Plus-only: a hosts list +can carry regex lines, `*.`-prefixed wildcards, and bare sink addresses that +count as unsupported. This one carries none of them. The download is about 3 MB and takes a few seconds. Watch the first terminal until this appears: diff --git a/specs/milestone-24.md b/specs/milestone-24.md new file mode 100644 index 0000000..e207b9b --- /dev/null +++ b/specs/milestone-24.md @@ -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` (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. diff --git a/src/config/model.zig b/src/config/model.zig index c36d814..77a2fa9 100644 --- a/src/config/model.zig +++ b/src/config/model.zig @@ -11,8 +11,8 @@ //! Runtime columns are deliberately absent. `clients.first_seen`, //! `clients.last_seen`, `rules.created_at` and //! `blocklist_sources.{last_updated, domain_count, wildcard_count, -//! exception_count, skipped_regex_count, checksum}` are facts a running server -//! produces, not configuration. Including them would make two exports taken +//! exception_count, skipped_regex_count, skipped_unsupported_count, checksum}` +//! are facts a running server produces, not configuration. Including them would make two exports taken //! minutes apart differ, which would make the byte-stable round trip untestable //! against a live server. //! diff --git a/src/config/reconcile.zig b/src/config/reconcile.zig index fee373f..7fe5e3e 100644 --- a/src/config/reconcile.zig +++ b/src/config/reconcile.zig @@ -1102,6 +1102,7 @@ fn seedSourceStats(database: *db.Db, id: i64) !void { .wildcard_count = 21, .exception_count = 9, .skipped_regex_count = 7, + .skipped_unsupported_count = 33, .checksum = "a" ** 64, }); } @@ -1171,6 +1172,8 @@ test "a source keeps its id, its checksum and its counters across a reconcile" { try testing.expectEqual(@as(?i64, 1_700_000_000), row.last_updated); try testing.expectEqual(@as(i64, 4321), row.domain_count); try testing.expectEqual(@as(i64, 9), row.exception_count); + try testing.expectEqual(@as(i64, 7), row.skipped_regex_count); + try testing.expectEqual(@as(i64, 33), row.skipped_unsupported_count); try testing.expectEqualStrings("a" ** 64, row.checksum.?); } diff --git a/src/filter/filter_integration_test.zig b/src/filter/filter_integration_test.zig index e9dff36..24d9a91 100644 --- a/src/filter/filter_integration_test.zig +++ b/src/filter/filter_integration_test.zig @@ -121,6 +121,27 @@ const http_domains: i64 = 2; const http_wildcards: i64 = 1; const http_exceptions: i64 = 0; const http_regex: i64 = 1; +/// Zero, and asserted rather than assumed: a hosts list carries no line a DNS +/// sinkhole cannot translate, which is what makes the abp fixture below a +/// separate source instead of two more lines in this one. +const http_unsupported: i64 = 0; + +/// An ABP-format list: one element-hiding rule and one `$`-modifier rule that +/// nxdns counts and skips, beside two names it blocks. `detectFormat` assigns +/// one format to a whole source, so these lines cannot join `http_body` — a +/// single `##` there would re-parse every hosts line as ABP. +const abp_body = + "! small abp list\n" ++ + "##.ad-banner\n" ++ + "||ads.example^$third-party\n" ++ + "||blocked.example^\n" ++ + "tracker.example\n"; + +/// `||blocked.example^` covers its own apex, so it writes one `.list` entry +/// beside its `.wild` one; `tracker.example` writes the second. +const abp_domains: i64 = 2; +const abp_wildcards: i64 = 1; +const abp_unsupported: i64 = 2; /// Compiles `text` into `.list`, `.wild` and `.allow` under /// `dir`, exactly as the manager's compile stage does, and returns the @@ -393,7 +414,7 @@ const Env = struct { // fixtures: the loopback http server // --------------------------------------------------------------------------- -const Route = enum(u8) { body, redirect, not_found, oversize, chunked, stall }; +const Route = enum(u8) { body, redirect, not_found, oversize, chunked, stall, changed }; /// How long the `stall` route holds a reply open when nothing releases it. /// @@ -440,6 +461,11 @@ const oversize_length = "104857600"; const HttpFixture = struct { server: net.Server, body: []const u8, + /// What the `changed` route serves: the same list after its author edited + /// it. A test sets this before the serving task starts and reaches it by + /// switching the route, so the two bodies are read through the atomic that + /// selects them and never written beside a request in flight. + changed_body: []const u8, route: std.atomic.Value(u8), /// Connections accepted, whatever came over them. A test that claims a pass /// downloaded nothing reads this rather than the route counters: a refetch @@ -466,6 +492,7 @@ const HttpFixture = struct { return .{ .server = try local.listen(io, .{ .reuse_address = true }), .body = body, + .changed_body = "", .route = .init(@intFromEnum(Route.body)), .accepted = .init(0), .flushed_parts = .init(0), @@ -514,6 +541,7 @@ const HttpFixture = struct { fn respond(self: *HttpFixture, io: std.Io, request: *std.http.Server.Request) !void { switch (@as(Route, @enumFromInt(self.route.load(.acquire)))) { .body => try request.respond(self.body, .{ .keep_alive = false }), + .changed => try request.respond(self.changed_body, .{ .keep_alive = false }), .redirect => if (std.mem.eql(u8, request.head.target, redirect_path)) try request.respond(self.body, .{ .keep_alive = false }) else @@ -824,6 +852,7 @@ test "3: a damaged compiled file never replaces a serving snapshot with a worse .wildcard_count = 1, .exception_count = 0, .skipped_regex_count = 0, + .skipped_unsupported_count = 0, .checksum = &bodyChecksum(good_list, good_wild, ""), }); @@ -856,6 +885,7 @@ test "3: a damaged compiled file never replaces a serving snapshot with a worse .wildcard_count = 1, .exception_count = 0, .skipped_regex_count = 0, + .skipped_unsupported_count = 0, .checksum = &bodyChecksum(unsorted_list, good_wild, ""), }); @@ -913,6 +943,7 @@ test "4: a 200 response is fetched, compiled, recorded and served" { try testing.expectEqual(http_domains, row.domain_count); try testing.expectEqual(http_wildcards, row.wildcard_count); try testing.expectEqual(http_regex, row.skipped_regex_count); + try testing.expectEqual(http_unsupported, row.skipped_unsupported_count); try testing.expectEqual(@as(usize, 64), (row.checksum orelse return error.TestNoChecksum).len); try testing.expect(row.last_updated != null); @@ -1073,6 +1104,7 @@ test "8: refetching identical content skips the rewrite and still moves last_upd .wildcard_count = http_wildcards, .exception_count = http_exceptions, .skipped_regex_count = http_regex, + .skipped_unsupported_count = http_unsupported, .checksum = blk: { var rows = try listRows(&env.database); defer rows.deinit(); @@ -1143,6 +1175,7 @@ fn publishFixtureFiles(env: *Env, id: i64, list_body: []const u8, wild_body: []c .wildcard_count = 0, .exception_count = 0, .skipped_regex_count = 0, + .skipped_unsupported_count = 0, .checksum = &bodyChecksum(list_body, wild_body, ""), }); } @@ -1255,6 +1288,7 @@ test "10b: the scheduler sweeps orphans on its own, with no operator call" { .wildcard_count = 0, .exception_count = 0, .skipped_regex_count = 0, + .skipped_unsupported_count = 0, .checksum = &bodyChecksum(list_body, "", ""), }); @@ -2156,6 +2190,7 @@ test "20: a data directory written before exceptions existed loads with no check .wildcard_count = 1, .exception_count = 0, .skipped_regex_count = 0, + .skipped_unsupported_count = 0, .checksum = &legacy_checksum, }); @@ -2170,3 +2205,139 @@ test "20: a data directory written before exceptions existed loads with no check try testing.expect(decision.blocked); try testing.expect((try env.evaluate("x.ccc.example.com"))[0].blocked); } + +// --------------------------------------------------------------------------- +// 21–22: the unsupported count, from the compile to the database and back +// --------------------------------------------------------------------------- + +test "21: an abp list's unsupported lines are counted, persisted and read back" { + if (!build_options.integration) return error.SkipZigTest; + + const gpa = testing.allocator; + const env = try Env.create(gpa); + defer env.destroy(); + const io = env.io(); + + var fixture = try HttpFixture.init(io, abp_body); + defer fixture.deinit(io); + var group: std.Io.Group = .init; + defer group.cancel(io); + try group.concurrent(io, HttpFixture.serve, .{ &fixture, io }); + + var url_buf: [64]u8 = undefined; + const url = try fixture.url(&url_buf); + const id = try seedSource(&env.database, url); + + try testing.expect(try refreshOnce(env, url)); + try env.mgr.reload(io); + + var rows = try listRows(&env.database); + defer rows.deinit(); + const row = try rows.byUrl(url); + try testing.expectEqual(abp_domains, row.domain_count); + try testing.expectEqual(abp_wildcards, row.wildcard_count); + try testing.expectEqual(@as(i64, 0), row.skipped_regex_count); + try testing.expectEqual(abp_unsupported, row.skipped_unsupported_count); + + const status = try env.status(id); + try testing.expectEqual(manager.State.ok, status.state); + try testing.expectEqual(@as(u32, @intCast(abp_unsupported)), status.counts.skipped_unsupported); + + // What the number costs the operator: the `$`-modifier rule named a domain + // and blocked nothing, while the two lines nxdns could translate did block. + try testing.expect(!(try env.evaluate("ads.example"))[0].blocked); + try testing.expect((try env.evaluate("blocked.example"))[0].blocked); + try testing.expect((try env.evaluate("tracker.example"))[0].blocked); +} + +/// The same list before and after its author edited only lines nxdns skips. +/// The written entries are identical in both, so the two compiles produce one +/// checksum and the refresh takes the unchanged-checksum path. +const churn_before = + "! churn fixture\n" ++ + "##.ad-one\n" ++ + "||blocked.example^\n" ++ + "tracker.example\n"; + +const churn_after = + "! churn fixture\n" ++ + "##.ad-one\n" ++ + "##.ad-two\n" ++ + "/ads[0-9]+/\n" ++ + "||blocked.example^\n" ++ + "tracker.example\n"; + +test "22: a list that changed only its skipped lines still updates both skip counters" { + if (!build_options.integration) return error.SkipZigTest; + + const gpa = testing.allocator; + const env = try Env.create(gpa); + defer env.destroy(); + const io = env.io(); + + var fixture = try HttpFixture.init(io, churn_before); + fixture.changed_body = churn_after; + defer fixture.deinit(io); + var group: std.Io.Group = .init; + defer group.cancel(io); + try group.concurrent(io, HttpFixture.serve, .{ &fixture, io }); + + var url_buf: [64]u8 = undefined; + const url = try fixture.url(&url_buf); + const id = try seedSource(&env.database, url); + + try testing.expect(try refreshOnce(env, url)); + + var first_checksum: [64]u8 = undefined; + { + var rows = try listRows(&env.database); + defer rows.deinit(); + const row = try rows.byUrl(url); + try testing.expectEqual(@as(i64, 0), row.skipped_regex_count); + try testing.expectEqual(@as(i64, 1), row.skipped_unsupported_count); + @memcpy(&first_checksum, row.checksum orelse return error.TestNoChecksum); + } + + // The edited list. Two more skipped lines and not one written entry moved, + // so the refresh finds its stored checksum and rewrites nothing on disk. + fixture.setRoute(.changed); + try testing.expect(!try refreshOnce(env, url)); + + { + var rows = try listRows(&env.database); + defer rows.deinit(); + const row = try rows.byUrl(url); + try testing.expectEqualStrings(&first_checksum, row.checksum orelse return error.TestNoChecksum); + // The three written counts are the ones an unchanged checksum vouches + // for; the two skip counts are the ones it says nothing about. + try testing.expectEqual(abp_domains, row.domain_count); + try testing.expectEqual(abp_wildcards, row.wildcard_count); + try testing.expectEqual(@as(i64, 1), row.skipped_regex_count); + try testing.expectEqual(@as(i64, 2), row.skipped_unsupported_count); + } + + const live = try env.status(id); + try testing.expectEqual(@as(u32, 1), live.counts.skipped_regex); + try testing.expectEqual(@as(u32, 2), live.counts.skipped_unsupported); + + // The restart. A new manager over the same database and the same files + // carries nothing across in memory, so the status it publishes is what + // rehydration read out of the row — which is the only reason writing the + // fresh counts above matters. + env.mgr.deinit(io); + env.mgr = try manager.Manager.init( + gpa, + &env.database, + .{ .dir = env.tmp.dir }, + &env.f, + .{ .enabled = false }, + budget, + ); + try env.mgr.reload(io); + + const restored = try env.status(id); + try testing.expectEqual(manager.State.ok, restored.state); + try testing.expect(restored.loaded); + try testing.expectEqual(@as(u32, 1), restored.counts.skipped_regex); + try testing.expectEqual(@as(u32, 2), restored.counts.skipped_unsupported); +} diff --git a/src/filter/manager.zig b/src/filter/manager.zig index 3c52c87..212580f 100644 --- a/src/filter/manager.zig +++ b/src/filter/manager.zig @@ -822,12 +822,21 @@ pub const Manager = struct { if (std.mem.eql(u8, stored, &compiled.result.checksum) and self.diskBodiesMatch(io, dir, row.id, stored)) { + // The three written counts come from the row: the checksum + // covers the three bodies, so an unchanged checksum means an + // unchanged number of entries in each. The two skip counts do + // not: a skipped line lands in no body, so a list that changed + // only its regex or browser-syntax lines arrives here with a + // stale row and a fresh compile. Taking them from the row would + // show the operator one number and restore another after a + // restart. try sources_repo.updateSourceStats(self.database, row.id, .{ .last_updated = now, .domain_count = row.domain_count, .wildcard_count = row.wildcard_count, .exception_count = row.exception_count, - .skipped_regex_count = row.skipped_regex_count, + .skipped_regex_count = compiled.result.counts.skipped_regex, + .skipped_unsupported_count = compiled.result.counts.skipped_unsupported, .checksum = stored, }); status.succeed(now, compiled.result.counts); @@ -857,6 +866,7 @@ pub const Manager = struct { .wildcard_count = compiled.result.counts.wildcards, .exception_count = compiled.result.counts.exceptions, .skipped_regex_count = compiled.result.counts.skipped_regex, + .skipped_unsupported_count = compiled.result.counts.skipped_unsupported, .checksum = &compiled.result.checksum, }); status.succeed(now, compiled.result.counts); @@ -1537,7 +1547,7 @@ fn applyLoadOutcomes( entry.loaded = true; // Two states survive a successful load. `.ok`, because a // refresh in this process already filled the counters the - // compile produced and the three database columns are a subset + // compile produced and the five database columns are a subset // of them. And any refresh failure, because the files that just // loaded are exactly the ones the failed refresh could not // replace, so the operator must still see why. @@ -1547,6 +1557,7 @@ fn applyLoadOutcomes( .wildcards = countOf(row.wildcard_count), .exceptions = countOf(row.exception_count), .skipped_regex = countOf(row.skipped_regex_count), + .skipped_unsupported = countOf(row.skipped_unsupported_count), }); }, .failed => |reason| { @@ -1926,6 +1937,7 @@ test "a canceled compiled-file read cancels the reload instead of recording it" .domain_count = 1, .wildcard_count = 0, .skipped_regex_count = 0, + .skipped_unsupported_count = 0, .exception_count = 0, .checksum = &bodyChecksum(list_body, wild_body, allow_body), }); @@ -2049,6 +2061,7 @@ test "the log label names a source without printing what its url carries" { .domain_count = 0, .wildcard_count = 0, .skipped_regex_count = 0, + .skipped_unsupported_count = 0, .checksum = null, }; const printed = try std.fmt.bufPrint(&buf, "blocklist {f}: download failed: {s}", .{ @@ -2223,6 +2236,7 @@ fn testRow(id: i64, enabled: bool) sources_repo.SourceRow { .wildcard_count = 4, .exception_count = 2, .skipped_regex_count = 1, + .skipped_unsupported_count = 5, .checksum = "0" ** 64, }; } @@ -2333,6 +2347,9 @@ test "a load of a source this process never refreshed takes the row counters" { try testing.expectEqual(@as(u32, 9), statuses[0].counts.domains); try testing.expectEqual(@as(u32, 4), statuses[0].counts.wildcards); try testing.expectEqual(@as(u32, 1), statuses[0].counts.skipped_regex); + // Rehydration: a restart reads this from the row and nowhere else, because + // no path reparses a compiled file's header. + try testing.expectEqual(@as(u32, 5), statuses[0].counts.skipped_unsupported); } test "a status borrows nothing, so a copy outlives the table it came from" { diff --git a/src/storage/config_schema.zig b/src/storage/config_schema.zig index eec9207..b00584d 100644 --- a/src/storage/config_schema.zig +++ b/src/storage/config_schema.zig @@ -61,6 +61,7 @@ pub const ddl_v1: [:0]const u8 = \\ wildcard_count INTEGER NOT NULL DEFAULT 0, \\ exception_count INTEGER NOT NULL DEFAULT 0, \\ skipped_regex_count INTEGER NOT NULL DEFAULT 0, + \\ skipped_unsupported_count INTEGER NOT NULL DEFAULT 0, \\ checksum TEXT \\); \\ diff --git a/src/storage/migrations.zig b/src/storage/migrations.zig index 514d70f..d472e95 100644 --- a/src/storage/migrations.zig +++ b/src/storage/migrations.zig @@ -271,6 +271,7 @@ test "a fresh database reaches the baseline with every v1 column and rule kind" try testing.expectEqual(@as(u32, 1), target_version); try testing.expect(try columnExists(&database, "upstreams", "tls_name")); try testing.expect(try columnExists(&database, "blocklist_sources", "exception_count")); + try testing.expect(try columnExists(&database, "blocklist_sources", "skipped_unsupported_count")); try database.exec( \\INSERT INTO rules (group_id, pattern, kind, action, created_at) diff --git a/src/storage/repositories/sources_repo.zig b/src/storage/repositories/sources_repo.zig index a102e2d..992b3b8 100644 --- a/src/storage/repositories/sources_repo.zig +++ b/src/storage/repositories/sources_repo.zig @@ -1,9 +1,10 @@ //! `blocklist_sources`. //! //! Only the four configuration columns are read and written. `last_updated`, -//! `domain_count`, `wildcard_count`, `exception_count`, `skipped_regex_count` -//! and `checksum` are facts a running server produces; an insert leaves them at -//! their column defaults so two exports taken minutes apart stay identical. +//! `domain_count`, `wildcard_count`, `exception_count`, `skipped_regex_count`, +//! `skipped_unsupported_count` and `checksum` are facts a running server +//! produces; an insert leaves them at their column defaults so two exports taken +//! minutes apart stay identical. //! //! The import path is list / insert / deleteAll / count; the runtime columns and //! the REST surface follow it, both keyed by row id. @@ -93,6 +94,12 @@ pub const SourceRow = struct { /// builds `SourceRow` values from the refresh columns alone. exception_count: i64 = 0, skipped_regex_count: i64, + /// Lines the compiler read and could not translate into a DNS decision: + /// cosmetic element hiding, `$`-modifier rules (save the tolerated + /// `$important` exception suffix, which lands in `exception_count`), scheme + /// anchors. Counted and not written, like `skipped_regex_count` and unlike + /// the three counts above. + skipped_unsupported_count: i64, checksum: ?[]const u8, }; @@ -102,6 +109,7 @@ pub const SourceStats = struct { wildcard_count: i64, exception_count: i64, skipped_regex_count: i64, + skipped_unsupported_count: i64, /// Lowercase hex sha256 over the `.list` body, then the `.wild` body, then /// the `.allow` body. The allow body is hashed last so an empty one leaves /// the digest of a two-body compile unchanged, which is what keeps a @@ -112,7 +120,7 @@ pub const SourceStats = struct { const row_columns_sql = \\SELECT id, url, name, enabled, last_updated, \\ domain_count, wildcard_count, skipped_regex_count, checksum, - \\ is_suggested, exception_count + \\ is_suggested, exception_count, skipped_unsupported_count \\ FROM blocklist_sources ; @@ -143,6 +151,7 @@ fn readSourceRow(stmt: *db.Stmt, gpa: Allocator) db.Error!SourceRow { .wildcard_count = stmt.columnInt(6), .exception_count = stmt.columnInt(10), .skipped_regex_count = stmt.columnInt(7), + .skipped_unsupported_count = stmt.columnInt(11), .checksum = checksum, }; } @@ -159,7 +168,8 @@ pub fn freeSourceRows(gpa: Allocator, items: []const SourceRow) void { const update_stats_sql = \\UPDATE blocklist_sources \\ SET last_updated = ?2, domain_count = ?3, wildcard_count = ?4, - \\ skipped_regex_count = ?5, checksum = ?6, exception_count = ?7 + \\ skipped_regex_count = ?5, checksum = ?6, exception_count = ?7, + \\ skipped_unsupported_count = ?8 \\ WHERE id = ?1 ; @@ -175,6 +185,7 @@ pub fn updateSourceStats(database: *db.Db, id: i64, stats: SourceStats) db.Error try stmt.bindInt(5, stats.skipped_regex_count); try stmt.bindText(6, stats.checksum); try stmt.bindInt(7, stats.exception_count); + try stmt.bindInt(8, stats.skipped_unsupported_count); try stmt.exec(); } @@ -315,8 +326,11 @@ test "insertBlocklistSource leaves the runtime columns at their defaults" { try testing.expectEqual( @as(i64, 0), try database.queryInt( - "SELECT sum(domain_count + wildcard_count + exception_count + skipped_regex_count)" ++ - " FROM blocklist_sources", + // Every runtime counter, summed only because this asserts they are + // all 0. No production query may add the two skip counters to the + // three written ones: a skipped line was never written. + "SELECT sum(domain_count + wildcard_count + exception_count + skipped_regex_count" ++ + " + skipped_unsupported_count) FROM blocklist_sources", ), ); } @@ -370,6 +384,7 @@ test "listSourceRows returns row ids and the runtime columns in url order" { try testing.expectEqual(@as(i64, 0), row.wildcard_count); try testing.expectEqual(@as(i64, 0), row.exception_count); try testing.expectEqual(@as(i64, 0), row.skipped_regex_count); + try testing.expectEqual(@as(i64, 0), row.skipped_unsupported_count); } } @@ -389,6 +404,7 @@ test "updateSourceStats writes the runtime columns of one source only" { .wildcard_count = 21, .exception_count = 9, .skipped_regex_count = 7, + .skipped_unsupported_count = 15, .checksum = "a" ** 64, }); @@ -406,6 +422,7 @@ test "updateSourceStats writes the runtime columns of one source only" { try testing.expectEqual(@as(i64, 21), updated.wildcard_count); try testing.expectEqual(@as(i64, 9), updated.exception_count); try testing.expectEqual(@as(i64, 7), updated.skipped_regex_count); + try testing.expectEqual(@as(i64, 15), updated.skipped_unsupported_count); try testing.expectEqualStrings("a" ** 64, updated.checksum.?); // The two untouched rows kept their defaults. @@ -423,6 +440,7 @@ fn listSourceRowsUnderFailure(gpa: Allocator) !void { .wildcard_count = 3, .exception_count = 5, .skipped_regex_count = 4, + .skipped_unsupported_count = 6, .checksum = "b" ** 64, }); @@ -489,6 +507,7 @@ test "updateSource leaves the runtime columns where the refresh path left them" .wildcard_count = 3, .exception_count = 2, .skipped_regex_count = 1, + .skipped_unsupported_count = 4, .checksum = "c" ** 64, }); diff --git a/src/web/handlers/blocklists.zig b/src/web/handlers/blocklists.zig index 8717ee4..9c63db8 100644 --- a/src/web/handlers/blocklists.zig +++ b/src/web/handlers/blocklists.zig @@ -61,6 +61,7 @@ pub const StatusView = struct { wildcards: u32, exceptions: u32, skipped_regex: u32, + skipped_unsupported: u32, pub fn from(status: *const manager_mod.SourceStatus) StatusView { return .{ @@ -75,6 +76,7 @@ pub const StatusView = struct { .wildcards = status.counts.wildcards, .exceptions = status.counts.exceptions, .skipped_regex = status.counts.skipped_regex, + .skipped_unsupported = status.counts.skipped_unsupported, }; } }; @@ -321,6 +323,7 @@ test "editing a blocklist keeps the counters the refresh wrote" { .wildcard_count = 3, .exception_count = 2, .skipped_regex_count = 1, + .skipped_unsupported_count = 8, .checksum = "abc", }); @@ -335,6 +338,8 @@ test "editing a blocklist keeps the counters the refresh wrote" { try testing.expectEqualStrings("renamed", row.name); try testing.expect(!row.enabled); try testing.expectEqual(@as(i64, 42), row.domain_count); + try testing.expectEqual(@as(i64, 1), row.skipped_regex_count); + try testing.expectEqual(@as(i64, 8), row.skipped_unsupported_count); try testing.expectEqual(@as(usize, 2), bench.reloads); } @@ -383,7 +388,13 @@ test "a status becomes the flat shape the API answers with" { const message = "connection refused"; @memcpy(status.last_error[0..message.len], message); status.last_error_len = message.len; - status.counts = .{ .domains = 10, .wildcards = 2, .exceptions = 4, .skipped_regex = 1 }; + status.counts = .{ + .domains = 10, + .wildcards = 2, + .exceptions = 4, + .skipped_regex = 1, + .skipped_unsupported = 6, + }; const view: StatusView = .from(&status); try testing.expectEqual(@as(i64, 7), view.id); @@ -393,4 +404,6 @@ test "a status becomes the flat shape the API answers with" { try testing.expectEqualStrings(message, view.last_error); try testing.expectEqual(@as(u32, 10), view.domains); try testing.expectEqual(@as(u32, 4), view.exceptions); + try testing.expectEqual(@as(u32, 1), view.skipped_regex); + try testing.expectEqual(@as(u32, 6), view.skipped_unsupported); } diff --git a/src/web/openapi.yaml b/src/web/openapi.yaml index c07a9bd..75a7acd 100644 --- a/src/web/openapi.yaml +++ b/src/web/openapi.yaml @@ -1876,7 +1876,7 @@ components: Blocklist: type: object - required: [id, url, name, enabled, is_suggested, last_updated, domain_count, wildcard_count, exception_count, skipped_regex_count, checksum] + required: [id, url, name, enabled, is_suggested, last_updated, domain_count, wildcard_count, exception_count, skipped_regex_count, skipped_unsupported_count, checksum] properties: id: { type: integer } url: { type: string } @@ -1890,6 +1890,7 @@ components: wildcard_count: { type: integer } exception_count: { type: integer } skipped_regex_count: { type: integer } + skipped_unsupported_count: { type: integer } checksum: type: string nullable: true @@ -1919,7 +1920,7 @@ components: SourceStatus: type: object - required: [id, state, loaded, last_attempt, last_success, url, last_error, domains, wildcards, exceptions, skipped_regex] + required: [id, state, loaded, last_attempt, last_success, url, last_error, domains, wildcards, exceptions, skipped_regex, skipped_unsupported] properties: id: { type: integer } state: @@ -1936,6 +1937,7 @@ components: wildcards: { type: integer } exceptions: { type: integer } skipped_regex: { type: integer } + skipped_unsupported: { type: integer } Rule: type: object diff --git a/web/src/features/blocklists/BlocklistsPage.test.tsx b/web/src/features/blocklists/BlocklistsPage.test.tsx index 281ac8b..1307a30 100644 --- a/web/src/features/blocklists/BlocklistsPage.test.tsx +++ b/web/src/features/blocklists/BlocklistsPage.test.tsx @@ -19,6 +19,7 @@ const BLOCKLISTS = { wildcard_count: 10, exception_count: 7, skipped_regex_count: 3, + skipped_unsupported_count: 21, checksum: "abc", }, { @@ -32,6 +33,7 @@ const BLOCKLISTS = { wildcard_count: 0, exception_count: 0, skipped_regex_count: 0, + skipped_unsupported_count: 0, checksum: null, }, ], @@ -102,6 +104,7 @@ const SNAPSHOT = { wildcards: 12, exceptions: 9, skipped_regex: 4, + skipped_unsupported: 17, }, ], }; @@ -117,7 +120,13 @@ test("renders the source table and the status empty state", async () => { expect(screen.getByText("10")).toBeTruthy(); expect(screen.getByText("7")).toBeTruthy(); expect(screen.getByText("3")).toBeTruthy(); + expect(screen.getByText("21")).toBeTruthy(); expect(screen.getByText("never")).toBeTruthy(); + expect(screen.getByRole("columnheader", { name: "Skipped regex" })).toBeTruthy(); + expect(screen.getByRole("columnheader", { name: "Skipped unsupported" })).toBeTruthy(); + expect( + screen.getByText(/Skipped unsupported lines are syntax nxdns cannot translate into a DNS decision/), + ).toBeTruthy(); const enabledToggle = screen.getByLabelText("StevenBlack enabled") as HTMLInputElement; expect(enabledToggle.checked).toBe(true); @@ -153,6 +162,7 @@ test("update now disables the button, then replaces the status section from the wildcards: 12, exceptions: 9, skipped_regex: 4, + skipped_unsupported: 17, }, { id: 2, @@ -166,6 +176,7 @@ test("update now disables the button, then replaces the status section from the wildcards: 0, exceptions: 0, skipped_regex: 0, + skipped_unsupported: 0, }, ], }; @@ -180,6 +191,8 @@ test("update now disables the button, then replaces the status section from the expect(screen.getByText("12")).toBeTruthy(); expect(screen.getByText("9")).toBeTruthy(); expect(screen.getByText("4")).toBeTruthy(); + expect(screen.getByText("17")).toBeTruthy(); + expect(screen.getAllByRole("columnheader", { name: "Skipped unsupported" })).toHaveLength(2); expect(screen.queryByText(/run .Update now. to fetch status/)).toBeNull(); // The store notifies one flush before the mutation's success state lands. await screen.findByText(/Update completed/); diff --git a/web/src/features/blocklists/BlocklistsPage.tsx b/web/src/features/blocklists/BlocklistsPage.tsx index 9792a65..b3f9f77 100644 --- a/web/src/features/blocklists/BlocklistsPage.tsx +++ b/web/src/features/blocklists/BlocklistsPage.tsx @@ -42,6 +42,10 @@ const styles = stylex.create({ marginTop: "1rem", color: colors.textMuted, }, + note: { + marginTop: "0.5rem", + color: colors.textMuted, + }, table: { width: "100%", minWidth: "max-content", @@ -155,6 +159,7 @@ export default function BlocklistsPage() { Wildcards Exceptions Skipped regex + Skipped unsupported Last updated Actions @@ -188,6 +193,9 @@ export default function BlocklistsPage() { {b.wildcard_count} {b.exception_count} {b.skipped_regex_count} + + {b.skipped_unsupported_count} + {b.last_updated === null ? "never" : formatTime(b.last_updated)} @@ -221,6 +229,13 @@ export default function BlocklistsPage() { ))} +

+ 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. +

)} diff --git a/web/src/features/blocklists/SourceStatusSection.tsx b/web/src/features/blocklists/SourceStatusSection.tsx index db3399a..8ebdbad 100644 --- a/web/src/features/blocklists/SourceStatusSection.tsx +++ b/web/src/features/blocklists/SourceStatusSection.tsx @@ -89,6 +89,7 @@ export default function SourceStatusSection({ sources, namesById }: SourceStatus Wildcards Exceptions Skipped regex + Skipped unsupported Last error @@ -112,6 +113,9 @@ export default function SourceStatusSection({ sources, namesById }: SourceStatus {source.wildcards} {source.exceptions} {source.skipped_regex} + + {source.skipped_unsupported} + {source.last_error === "" ? ( — diff --git a/web/src/lib/contractSamples.gen.ts b/web/src/lib/contractSamples.gen.ts index 8b8867e..9dc0822 100644 --- a/web/src/lib/contractSamples.gen.ts +++ b/web/src/lib/contractSamples.gen.ts @@ -93,6 +93,7 @@ export const sample_list_blocklists: { blocklists: Blocklist[] } = { last_updated: null, name: "ads", skipped_regex_count: 0, + skipped_unsupported_count: 0, url: "https://lists.example/ads.txt", wildcard_count: 0, }, @@ -118,6 +119,7 @@ export const sample_update_blocklists_now: { sources: SourceStatus[] } = { last_success: 0, loaded: false, skipped_regex: 0, + skipped_unsupported: 0, state: "never_fetched", url: "https://lists.example/ads.txt", wildcards: 0, diff --git a/web/src/lib/types.ts b/web/src/lib/types.ts index 2db1295..21d44ae 100644 --- a/web/src/lib/types.ts +++ b/web/src/lib/types.ts @@ -162,6 +162,7 @@ export interface Blocklist { wildcard_count: number; exception_count: number; skipped_regex_count: number; + skipped_unsupported_count: number; checksum: string | null; } @@ -192,6 +193,7 @@ export interface SourceStatus { wildcards: number; exceptions: number; skipped_regex: number; + skipped_unsupported: number; } export type RuleKind = "exact" | "wildcard" | "regex";