P47 follow-ups: deferred minors from the seal-and-skip reviews #680

New Issue

Closed

opened 2026-09-08 19:29:24 +00:00 by kert · 2 comments

kert commented

2026-09-08 19:29:24 +00:00

Owner

Copy Link

Deferred minors from the P47 seal-and-skip reviews (branch p47-seal-skip, spec docs/superpowers/specs/2026-09-08-comment-pipeline-seal-and-skip-design.md). None affect data safety; all are polish or small costs.

llm/index.py: items whose load() yields no text never get a fingerprint stamped, so they are re-loaded (one stat + one SQL read) on every run.
bib/store.py::_init_schema: the duplicate-attachments GROUP BY scan runs on every Store() open even after the unique index exists — short-circuit on sqlite_master.
cli/comments.py::dockets: docket_counts runs two url LIKE scans per docket (SQLite LIKE can't use idx_items_url); fine for an operator command, could be one grouped query.
cli/bib.py::fetch-pfs-comments --docket: still costs one FR fetch + one resolve_docket call for CMS ids with no dockets row (documented in help; only true "no API calls" once known).
cli/bib.py::fetch-pfs-comments: rule items whose docket cannot be resolved are no longer upserted to bib (the upsert moved inside the per-docket loop) — decide whether that is wanted.
bib/store.py::upsert_status: extra_json is exempt from the empty-keeps-stored merge; a Rule re-upserted from a thinner payload can still lose extra fields — consider a per-key merge of the decoded dict.
bib/dockets.py::fingerprint_files catches only FileNotFoundError; cli/comments.py::extract_ocr opens a read connection it never closes; regulations_gov.py sealed-guard block duplicated across both backfills; llm/source.py section markers misplaced, _load shadows row, per-item _attachment_paths query for rule/corpus fingerprints; Zotero snapshot never contains WAL-only rows (pre-existing).
Tests: explicit-title dedupe hit in attach_file; --reattach at the CLI level; dedupe conflict path with a missing kept file.

Refs #615.

Deferred minors from the P47 seal-and-skip reviews (branch `p47-seal-skip`, spec `docs/superpowers/specs/2026-09-08-comment-pipeline-seal-and-skip-design.md`). None affect data safety; all are polish or small costs. - [ ] `llm/index.py`: items whose `load()` yields no text never get a fingerprint stamped, so they are re-loaded (one stat + one SQL read) on every run. - [ ] `bib/store.py::_init_schema`: the duplicate-attachments GROUP BY scan runs on every `Store()` open even after the unique index exists — short-circuit on `sqlite_master`. - [ ] `cli/comments.py::dockets`: `docket_counts` runs two `url LIKE` scans per docket (SQLite `LIKE` can't use `idx_items_url`); fine for an operator command, could be one grouped query. - [ ] `cli/bib.py::fetch-pfs-comments --docket`: still costs one FR fetch + one `resolve_docket` call for CMS ids with no `dockets` row (documented in help; only true "no API calls" once known). - [ ] `cli/bib.py::fetch-pfs-comments`: rule items whose docket cannot be resolved are no longer upserted to bib (the upsert moved inside the per-docket loop) — decide whether that is wanted. - [ ] `bib/store.py::upsert_status`: `extra_json` is exempt from the empty-keeps-stored merge; a Rule re-upserted from a thinner payload can still lose extra fields — consider a per-key merge of the decoded dict. - [ ] `bib/dockets.py::fingerprint_files` catches only `FileNotFoundError`; `cli/comments.py::extract_ocr` opens a read connection it never closes; `regulations_gov.py` sealed-guard block duplicated across both backfills; `llm/source.py` section markers misplaced, `_load` shadows `row`, per-item `_attachment_paths` query for rule/corpus fingerprints; Zotero snapshot never contains WAL-only rows (pre-existing). - [ ] Tests: explicit-title dedupe hit in `attach_file`; `--reattach` at the CLI level; dedupe conflict path with a missing kept file. Refs #615.

kert commented

2026-09-08 19:57:34 +00:00

Author

Owner

Copy Link

Residuals parked from the final fix-wave re-review (all minor):

no test pins the <target>: N sealed docket(s) already complete — not listed line in cli/llm.py
an item that had text and later loads blank keeps its old vectors: the blank stamp writes chunk_count=0 without _delete_old_chunks, then it is fingerprint-skipped forever (pre-existing orphan made permanent; near-impossible for comments)
walk_docket now runs docket_counts (two COUNT … LIKE scans) on every clean walk, not only pre-seal — compute lazily after should_seal
manual stack comments seal can seal a docket with zero stored comments (auto path is guarded)
ZoteroPdfIndex.snapshot() has no production caller any more (lazy path only); keep for tests or drop

Residuals parked from the final fix-wave re-review (all minor): - [ ] no test pins the `<target>: N sealed docket(s) already complete — not listed` line in `cli/llm.py` - [ ] an item that had text and later loads blank keeps its old vectors: the blank stamp writes chunk_count=0 without `_delete_old_chunks`, then it is fingerprint-skipped forever (pre-existing orphan made permanent; near-impossible for comments) - [ ] `walk_docket` now runs `docket_counts` (two `COUNT … LIKE` scans) on every clean walk, not only pre-seal — compute lazily after `should_seal` - [ ] manual `stack comments seal` can seal a docket with zero stored comments (auto path is guarded) - [ ] `ZoteroPdfIndex.snapshot()` has no production caller any more (lazy path only); keep for tests or drop