Files
stack/docs/superpowers/specs/2026-09-08-comment-pipeline-seal-and-skip-design.md

196 lines
19 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# comment pipeline — docket seals and cheap skips: never redo what is known complete
Tracker: new milestone (P47) with one issue per component; refs #615 (re-farm), #662 (mirror backfill), #253 (OCR stub, out of scope).
## Goal
A default run of any stage of the regulations.gov comment pipeline — `bib fetch-pfs-comments`, `bib backfill-comments`, `comments extract`, `llm index` — must do no work for a docket that is known complete, and must decide per item whether there is work *before* touching the network, the filesystem, or a PDF. `--force` is the only way to redo known-complete work, and it applies to one run.
Success criteria, measured on the live data (2026-09-08: 12 dockets, 201,874 comments, 28,493 PDFs / 13.3 GB, 224,351 bib items):
- `stack llm index --collection all` with nothing changed finishes in under a minute, opens zero PDFs, and does not copy `zotero.sqlite`.
- `stack bib fetch-pfs-comments` with every docket sealed makes at most 4 API calls (the Federal Register rule search) and writes zero rows.
- `stack comments extract` with nothing changed reads zero `combined.md` files.
- Re-running any stage twice produces byte-identical `bib.sqlite` content (no `updated_at` / `access_date` churn, no tag rewrites, no duplicate attachment rows).
## Current state (measured 2026-09-08)
What a default run redoes today, per stage. File refs are to the tree at 832bfd5.
| Stage | Skips | Redoes | Cost |
|---|---|---|---|
| fetch (`cli/bib.py:189-298`, `bib/regulations_gov.py:198-260`) | nothing | rediscovers every rule, one FR GET per rule, one rate-limited reg.gov call per CMS id (even with `--docket`, which filters after the resolve call), re-walks every docket from page 1 (the `lastModifiedDate` cursor is intra-call only), re-upserts every comment | ~170 API calls per large docket; `Store.upsert` (`bib/store.py:207-245`) stamps `access_date`, bumps `updated_at`, deletes and rewrites every tag row (`store.py:429-453`); `attach_file` (`store.py:502-533`) mints a duplicate row and copies the file again — 33,379 duplicated `(item_id, filename)` groups covering 66,917 of 78,017 rows in the live store |
| backfill API (`regulations_gov.py:378-544`) | `enriched:ok` / `enriched:gone` | nothing significant | one anti-join scan — already correct |
| backfill mirror (`regulations_gov.py:657-776`) | `enriched:ok` | lists the whole S3 prefix for comments and attachments before filtering | 4080 LIST requests per docket |
| extract (`rex/comments/walker.py:79-118`, `combine.py:42-132`) | dirs with a `combined.md` (stat only) | reads + YAML-parses every skipped `combined.md` for sibling derivation and fires the attach callback; loads every comment abstract from bib at startup (`cli/comments.py:52-63`); never detects a stale extraction | 28,228 files / 757 MB parsed per run |
| index comments (`llm/index.py:90-143`, `llm/source.py:104-132`, `llm/pages.py:61-83`) | sha256(text) match in `index_state` | chunks and opens every PDF page (`enrich_pdf_pages`) *before* the hash check; one `_year_of` query per comment; reads every `combined.md`; `store.get` per comment without one | full PyMuPDF decode of 13.3 GB per no-op run |
| index corpus (`source.py:265-357`, `cli/llm.py:24-27`) | same | hydrates all 224,351 items (3 queries each) to discard 201,874 comments; re-extracts every attachment; copies the 1.95 GB `zotero.sqlite` unconditionally | ~670k queries + 1.95 GB copy per run |
The one signal that would let a stage skip a whole docket — the reg.gov `commentEndDate` — is fetched every run (`cli/bib.py:170`, `:272`) and used only as a filter predicate. No docket-level state exists anywhere.
## Decisions
1. **Two-level skip.** A docket-level *seal* removes whole dockets from every walk; a per-item *fingerprint* (cheap facts only) is checked before any load. Content hashes stay as the third line.
2. **Seal = auto after close + quiet period.** A docket seals when its comment close date is ≥ `seal_quiet_days` (default 30) in the past and the most recent completed pull after that date found zero new comments. Manual `seal` / `unseal` exist. Sealing happens at the end of a fetch walk because fetch is the only stage that learns about new comments.
3. **Docket state lives in a `dockets` table in `bib.sqlite`**, not `bib_meta` JSON, not tags, not `.state/` (cache-only by convention). Index-side docket completion lives in the llm Postgres DB next to `index_state`.
4. **Fix every default-path redo for open dockets too** (fetch watermark, no-op upsert, idempotent attachments, extract skip cost, index ordering, bulk queries, lazy Zotero snapshot).
5. **Attachment duplicates are cleaned up now** — guard plus a one-time migration — because the guard alone leaves 66k dead rows that every later stage pays for.
6. **`--force` semantics unchanged and uniform**: bypasses seal, fingerprint, hash, and docket-completion for that run; never unseals.
## Architecture
```
dockets (bib.sqlite) index_state(+fingerprint), index_docket_state (llm pg)
id, close date, watermark, sealed_at (item_key, collection) → fingerprint, content_hash
│ │
fetch ──► sealed? ──skip──┐ backfill ──► sealed? ──skip──┐ extract ──► sealed? ──skip──┐ index ──► sealed & complete? ──skip──┐
│ no │ │ no │ │ no │ │ no │
▼ │ ▼ │ ▼ │ ▼ │
walk from watermark │ pending set from tags │ fingerprint per dir │ fingerprint per DocRef │
upsert → unchanged? no-op│ (mirror: list S3, open only) │ differs? extract │ differs? load → hash differs? embed │
walk clean → advance │ │ │ clean over sealed docket → completion │
watermark, maybe seal ───┘ └──────────────────────────────┴──────────────────────────────────────────┘
```
## Components
### `bib/schema.sql` + `bib/store.py` — `dockets` table and store API
```sql
CREATE TABLE IF NOT EXISTS dockets (
id TEXT PRIMARY KEY, -- reg.gov docket id, e.g. CMS-2026-2377
rule_cms_id TEXT, -- CMS-1848-P
fr_document_id TEXT, -- reg.gov document id carrying the comments
fr_object_id TEXT, -- attributes.objectId (what /comments filters on)
comment_end_date TEXT, -- YYYY-MM-DD from document attributes
pull_watermark TEXT, -- max lastModifiedDate seen on a completed walk
last_pull_at TEXT,
last_pull_new INTEGER, -- comments created by that walk
sealed_at TEXT,
seal_reason TEXT, -- 'auto' | 'manual'
counts_json TEXT -- {"comments":n,"enriched":n,"extracted":n} at seal
);
CREATE UNIQUE INDEX IF NOT EXISTS attachments_item_filename ON attachments(item_id, filename);
```
The unique index is created by `Store.connect` only when no duplicates remain (see migration); fresh databases get it from the schema.
Store methods (all thin SQL, unit-tested against an in-memory store):
- `docket_get(id) -> Docket | None`, `docket_upsert(Docket)`, `dockets() -> list[Docket]`, `sealed_docket_ids() -> set[str]`.
- `docket_seal(id, reason, counts)`, `docket_unseal(id)`.
- `docket_of_item(key) -> str | None` reads the `reg-docket:` tag.
- `upsert(item) -> key` keeps its signature; new `upsert_status(item) -> tuple[key, Literal["created","updated","unchanged"]]` compares the normalized row (all columns except `access_date`, `updated_at`, `created_at`) and the tag/collection sets, and returns `unchanged` without writing anything. `stamp_access` runs only on create/update.
- `attach_file(key, path, title)` returns the existing attachment key when `(item_id, filename)` already exists; no copy, no insert.
`Docket` is a frozen dataclass in new `bib/dockets.py`, together with the pure `should_seal(docket, today, quiet_days) -> bool`.
### `bib/dockets.py` (new, pure)
- `should_seal`: `comment_end_date` present, `today >= end + quiet_days`, `last_pull_at >= end + quiet_days`, `last_pull_new == 0`, not already sealed.
- `fingerprint_files(paths) -> str`: sha256 over sorted `(name, size, mtime_ns)` — shared by extract and index.
### `bib/regulations_gov.py` — watermark walk, sealed skips
- `iter_comments(object_id, since: str | None = None)`: initial cursor = `since`. Boundary rows re-yield (`>=` filter); the no-op upsert absorbs them.
- `walk_docket(store, api, docket: Docket, *, attachments, limit) -> WalkResult(new, updated, unchanged, watermark, clean)` extracted from the CLI loop so both fetch commands share it. Advances `pull_watermark` / `last_pull_at` / `last_pull_new` only when `clean` (no HTTPStatusError break, no exception). Calls `should_seal` and `docket_seal(..., reason="auto")` when true.
- `backfill_comments(..., docket=...)` and `backfill_from_mirror(...)`: return early with a log line when the docket is sealed and `force` is false. Mirror listing is unchanged for open dockets.
### `cli/bib.py` — fetch commands
`fetch-pfs-comments` per proposed rule: split CMS ids → for each, `store.docket_get`. Sealed → `echo " <docket>: sealed <date>, skipping"` and `continue` *before* the `federal_register()` rule fetch and the `resolve_docket` call. Unknown → `resolve_docket` + `find_documents_in_docket` once, insert the `Docket` row with `comment_end_date`, `fr_document_id`, `fr_object_id`. Known and open → `walk_docket` from the watermark using the stored object id (no resolve, no document listing). `--docket` filters *before* any API call (the current help text becomes true). `fetch-docket-comments` takes the same path for one docket. Both accept `--force` (walk from page 1, ignore seal, never unseal).
### `dev/scripts/dedupe_attachments.py` (one-time migration)
Groups `attachments` by `(item_id, filename)`; keeps the row with the smallest `rowid`; deletes the others' rows and their files under the attachment store when no other row references the same path. Prints a report (groups, rows removed, bytes freed, and the list of removed keys that carry a Zotero mapping, for a separate Zotero-side pass — Zotero itself is not touched). `--dry-run` default; `--apply` executes inside one transaction. After a clean apply, `Store.connect` creates the unique index on next open.
### `rex/comments/walker.py` + `combine.py` + `cli/comments.py` — extract
- `walk_and_extract(root, ..., sealed: set[str], force, reattach)`: docket dirs in `sealed` are skipped at the loop head (`walker.py:95`) unless `force`.
- Per dir, "up to date" = `combined.md` exists **and** its frontmatter `sources` fingerprint equals `fingerprint_files` over the dir's current attachment/body files. `extract_comment` writes `sources` into the frontmatter.
- Skipped dirs no longer go through `derive_siblings_from_combined` or `on_extracted`; `--reattach` restores the old behaviour for repair runs.
- `_build_bib_helpers` loads abstracts in batches keyed by the todo list instead of the whole table.
- New commands: `stack comments dockets` (table: id, rule, close date, comments/enriched/extracted, watermark, sealed), `stack comments seal <id> [--reason]`, `stack comments unseal <id>`, `stack comments dockets --discover` (one-time populate of the 12 known dockets from `reg-docket:` tags, resolving close dates with one API call each).
### `llm/source.py` — lazy `DocRef`
```python
@dataclass(frozen=True)
class DocRef:
key: str
collection: str
docket: str | None
fingerprint: str
load: Callable[[], Doc] # builds the Doc (reads combined.md / attachments / fr_anchors)
```
- `iter_comment_refs(store, root, docket=None, sealed_complete: set[str])`: one SQL query per docket for `(key, comment_id, year, updated_at, has_combined)`; skips dockets in `sealed_complete`; fingerprint = `fingerprint_files(combined.md + attachment files)` + `updated_at`. `load` does today's `iter_comment_docs` body for one item.
- `iter_corpus_refs`: keys selected in SQL (`tag = corpus tag AND NOT source:regulations-gov`), no hydration; fingerprint = `updated_at` + attachment file stats. `load` hydrates and extracts attachments.
- `iter_rule_refs`: fingerprint = `fr_anchor_docs.sha256`.
- `ZoteroPdfIndex` becomes lazy: constructed with the source path; `snapshot()` runs on first `lookup()` miss of a local PDF, and only copies when the source mtime is newer than the existing `.state/llm/zotero.sqlite`.
### `llm/migrate.py` + `llm/index.py` — fingerprint-first loop
- Migration: `ALTER TABLE index_state ADD COLUMN IF NOT EXISTS fingerprint TEXT`; `CREATE TABLE IF NOT EXISTS index_docket_state (collection TEXT, docket TEXT, sealed_at TIMESTAMPTZ, indexed_at TIMESTAMPTZ, PRIMARY KEY (collection, docket))`.
- `index_refs(refs, collection, *, force, sealed: dict[str, sealed_at])`, per ref:
1. `not force and state[key].fingerprint == ref.fingerprint` → skip (no load).
2. `doc = ref.load()`; `h = content_hash(doc.text)`; `not force and state[key].content_hash == h` → update fingerprint only, skip.
3. `chunks = enrich_pdf_pages(doc, chunk_doc(doc))` → embed → write chunks + state row (fingerprint, hash, chunk_count).
- After a run over a docket whose seal is set and which ended with zero errors, upsert `index_docket_state(collection, docket, sealed_at)`. On the next run `iter_*_refs` receives `sealed_complete = {docket for rows where sealed_at == dockets.sealed_at}`; an unseal/reseal changes `sealed_at` and invalidates the row.
- `stats` output adds `fingerprint_skipped`, `hash_skipped`, `docket_skipped`.
### `cli/llm.py`
`index` builds the ref iterators, passes `store.sealed_docket_ids()` joined with `index_docket_state`, honours `--docket` for comments, `--key` for rules, and `--force`. The Zotero snapshot line moves into the lazy index. `--collection all` runs the three in sequence as today.
### Config
`stack.toml`: `[comments] seal_quiet_days = 30`. No new env.
### `dev/scripts/refarm_cms_2026_2377.sh`
Collapses to the plain chain (mirror backfill → extract → index → API fetch → API backfill → extract → index) with no special ordering justification: every step is now cheap when there is nothing to do. The 2026-09-15 crontab line stays.
## Data flow for one incremental run (open docket)
1. `fetch-pfs-comments`: FR search (≤4 calls) → 11 dockets sealed, skipped with no calls → CMS-2026-2377 open: walk from watermark `2026-09-08T12:00:00` → 40 new, 3 updated, 12 unchanged boundary rows → watermark advanced, `last_pull_new=40`, `should_seal` false (close date 2026-09-14 not past).
2. `backfill --mirror --docket CMS-2026-2377`: not sealed → list S3 → 40 pending → enrich, attach (idempotent).
3. `comments extract --docket CMS-2026-2377`: 40 dirs lack `combined.md`, 2 have a changed fingerprint → 42 extracted; the rest are skipped without a read.
4. `llm index --collection comments`: 11 dockets in `index_docket_state` → not iterated; CMS-2026-2377: 18,691 refs, 42 fingerprints differ → 42 loaded, hashed, PDF-paged, embedded; 18,649 skipped without a load.
## Error handling
- A walk that breaks on an HTTP error leaves the watermark untouched; the next run resumes from the old watermark (re-yields are absorbed by the no-op upsert).
- Sealing never happens on a run that had errors (`clean=False`).
- A fingerprint mismatch caused by a touch (mtime only) costs one load and hash, then settles.
- `index_docket_state` is written only after a zero-error run; any embed failure leaves the docket iterable.
- The dedupe migration is dry-run by default and transactional on `--apply`; it refuses to run if any duplicate group's files differ in size (reports them instead).
- A sealed docket named by `--docket` without `--force` prints a one-line notice and exits 0.
## Testing
Pure units first (TDD): `should_seal` (boundary days, missing close date, unclean pull), `fingerprint_files` (order-independent, mtime-sensitive), `upsert_status` (created/updated/unchanged, tag-set change → updated, access stamp only on change), `attach_file` idempotency, `iter_comments(since=)` passes the filter and re-yields the boundary row, `walk_docket` watermark advance only when clean, `index_refs` ordering (a ref whose fingerprint matches never has `load` called; hash match updates fingerprint without embedding; PDFs opened only on the embed path — inverts `tests/llm/test_index.py::test_enriches_chunks_before_add`).
CLI: extend `tests/cli/test_bib_exercise.py` (`TestFetchPfsComments`: sealed docket makes no reg.gov call; `--docket` filters before resolve), `tests/bib/test_regulations_gov_mirror.py::test_idempotent_rerun` (sealed → no S3 listing), `tests/rex/comments/test_walker.py` (skipped dirs not read, `--reattach`), `tests/cli/test_comments.py` (`dockets`, `seal`, `unseal`), `tests/cli/test_llm_exercise.py` (corpus run without Zotero copy when no fallback needed), `tests/llm/test_migrate.py` (new column/table idempotent).
Migration: fixture DB with duplicate groups → dry-run report → apply → unique index creatable → second apply is a no-op.
Acceptance on live data (recorded in the tracker): the four success criteria above, timed.
## Rollout
1. Deploy code; `Store.connect` creates `dockets`; llm migration adds the column and table on container start.
2. `stack comments dockets --discover` populates 12 rows (12 API calls, once).
3. `dev/scripts/dedupe_attachments.py --dry-run`, review, `--apply`; reopen the store → unique index created.
4. `stack comments seal <id> --reason manual` for the 11 historical dockets (CMS-2017-0092 … CMS-2025-0304); CMS-2026-2377 stays open and auto-seals after 2026-10-14 once a pull finds nothing new.
5. One last full walk: `stack llm index --collection all` stamps fingerprints for all 196k items (stat-only, no PDF decode, no embedding) and writes `index_docket_state` for the 11 sealed dockets. Then a second run must meet the sub-minute criterion.
6. `stack comments extract` once to stamp `sources` fingerprints into existing `combined.md` frontmatter (read-only pass for unchanged dirs; rewrites frontmatter only).
7. Re-farm script simplified; crontab unchanged.
## Out of scope
- OCR for image-only attachments (#253); `extract-ocr` stays a stub.
- Zotero-side removal of already-synced duplicate attachments (listed by the migration report; separate pass like #665).
- Reducing mirror S3 listing cost for open dockets.
- Chunking, embedding model, or retrieval changes.