homelab/stack
1
0
Fork 0
Code Issues 22 Pull Requests Actions Packages Projects 1 Releases Wiki Activity

fix(cli): don't drop unresolvable-docket rules; close extract-ocr's bib connection (refs #680)

Browse Source 
- fetch-pfs-comments upserts the rule item before the per-docket
  resolve loop, so a CMS id that never resolves to a reg.gov docket no
  longer loses the rule entirely (the upsert used to live only inside
  the per-docket loop, guarded by a successful resolve).
- --docket's help text and the command docstring now say precisely
  when the "no API calls" guarantee holds: only once a dockets row
  already exists for the CMS id. A CMS id with no row yet still costs
  one FR rule-metadata fetch plus one resolve_docket call.
- extract_ocr closes the read connection _bib_lookup_factory opens
  (via the same `.close` attribute contract extract() already honours
  in its own finally), instead of leaking it.

Adds coverage: the unresolvable-docket upsert (and rewires test_no_docket
off a MagicMock rule, now that the rule is upserted unconditionally),
--reattach's repair behavior at the CLI level, and the extract_ocr
connection close.
This commit is contained in:
kert
2026-09-11 17:31:57 -04:00
parent f61eae8656
commit d69d5fb3aa
4 changed files with 183 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
src/cli/bib.py
View File

@@ -220,7 +220,14 @@ def fetch_pfs_comments(
docket: str = typer.Option(
"",
"--docket",
help="Only pull this reg.gov docket (e.g. CMS-2026-2377); other dockets are skipped before any API call once known.",
help=(
"Only pull this reg.gov docket (e.g. CMS-2026-2377). Filtered "
"before any API call ONLY once the docket is known (a `dockets` "
"row already exists for its CMS id) — a CMS id with no `dockets` "
"row yet still costs one Federal Register rule-metadata fetch "
"plus one resolve_docket call, because the reg.gov docket id "
"isn't known until those calls resolve it."
),
),
force: bool = typer.Option(
False,
@@ -232,7 +239,10 @@ def fetch_pfs_comments(
on each docket from its stored watermark.
Sealed dockets (comment period closed + quiet period + an empty pull)
cost nothing: no rule-metadata fetch, no resolve call, no walk.
cost nothing: no rule-metadata fetch, no resolve call, no walk. The
same is true for a docket already known via `--docket` that resolves
to a different id. A CMS id with no `dockets` row at all is not free
even under `--docket`: see that option's help.
"""
from bib import connect
from bib.federalregister import pfs_rules, split_docket_ids
@@ -279,6 +289,12 @@ def fetch_pfs_comments(
rule.add_tag("module:pfs")
for cid in cms_ids:
rule.add_tag(f"cms-rule:{cid}")
# Upsert now, before any docket resolution: a CMS id whose
# docket never resolves (resolve_docket returns nothing) must
# not cost the rule item its only upsert. Each docket below
# re-upserts to add its reg-docket tag; upsert_status makes
# the extra call a no-op when nothing changed.
store.upsert(rule)
for cms_id in todo:
d = known[cms_id]

42
src/cli/comments.py
View File

@@ -239,22 +239,32 @@ def extract_ocr(
return
body_lookup = _bib_lookup_factory(True)
engine = RapidOcrEngine()
done = errors = 0
for i, comment_dir in enumerate(pending, 1):
try:
extract_comment(
comment_dir,
inline_body=body_lookup(comment_dir.name),
force=True,
ocr_engine=engine,
)
done += 1
except Exception as e: # noqa: BLE001
log.warning("ocr extract failed for %s: %s", comment_dir.name, e)
errors += 1
if i % 25 == 0:
typer.echo(f" {i}/{len(pending)} ok={done} errors={errors}", err=False)
try:
engine = RapidOcrEngine()
done = errors = 0
for i, comment_dir in enumerate(pending, 1):
try:
extract_comment(
comment_dir,
inline_body=body_lookup(comment_dir.name),
force=True,
ocr_engine=engine,
)
done += 1
except Exception as e: # noqa: BLE001
log.warning("ocr extract failed for %s: %s", comment_dir.name, e)
errors += 1
if i % 25 == 0:
typer.echo(
f" {i}/{len(pending)} ok={done} errors={errors}", err=False
)
finally:
# _bib_lookup_factory opens its own read connection to bib.sqlite
# when the store isn't :memory: (see _build_bib_helpers); close
# it here the same way extract() closes its own.
close = getattr(body_lookup, "close", None)
if close is not None:
close()
typer.echo(f" done: {done} re-extracted, {errors} errors")

47
tests/cli/test_bib_exercise.py
View File

@@ -287,6 +287,7 @@ class TestFetchPfsComments:
@patch("bib.translate.federal_register")
@patch("bib.regulations_gov.Client")
def test_no_docket(self, mc_client_cls, mc_translate, mc_split, mc_pfs, mc_connect):
from bib.item import Rule
from bib.store import Store
store = Store(":memory:", storage_dir="/tmp/nope")
@@ -299,7 +300,7 @@ class TestFetchPfsComments:
doc.html_url = "https://example.com"
mc_pfs.return_value = [doc]
rule = MagicMock()
rule = Rule(title="CY2024 PFS NPRM", url="https://example.com")
mc_translate.return_value = rule
api = MagicMock()
@@ -312,6 +313,50 @@ class TestFetchPfsComments:
assert result.exit_code == 0
assert "skip" in result.output
@patch("bib.connect")
@patch("bib.federalregister.pfs_rules")
@patch("bib.federalregister.split_docket_ids", return_value=["CMS-1676-P"])
@patch("bib.translate.federal_register")
@patch("bib.regulations_gov.Client")
def test_unresolvable_docket_still_upserts_rule(
self, mc_client_cls, mc_translate, mc_split, mc_pfs, mc_connect
):
"""refs #680: a rule whose only CMS id never resolves to a reg.gov
docket must still be upserted — the upsert used to live only
inside the per-docket loop, so it was skipped entirely here."""
from bib.item import Rule
from bib.store import Store
store = Store(":memory:", storage_dir="/tmp/nope")
mc_connect.return_value = store
doc = MagicMock()
doc.type = "Proposed Rule"
doc.publication_date = "2023-01-01"
doc.dockets = ["CMS-1676-P"]
doc.html_url = "https://example.com/unresolvable"
mc_pfs.return_value = [doc]
rule = Rule(title="CY2024 PFS NPRM", url="https://example.com/unresolvable")
mc_translate.return_value = rule
api = MagicMock()
api.__enter__ = MagicMock(return_value=api)
api.__exit__ = MagicMock(return_value=False)
mc_client_cls.return_value = api
api.resolve_docket.return_value = None
result = runner.invoke(app, ["fetch-pfs-comments"])
assert result.exit_code == 0
con = store._con() # noqa: SLF001
row = con.execute(
"SELECT title FROM items WHERE url = ?",
("https://example.com/unresolvable",),
).fetchone()
assert row is not None
assert row["title"] == "CY2024 PFS NPRM"
class TestIngestMail:
@patch("bib.connect")

93
tests/cli/test_comments.py
View File

@@ -48,6 +48,33 @@ def test_extract_ocr_empty_queue(tmp_path: Path):
assert "ocr queue: 0" in result.output
def test_extract_ocr_closes_bib_lookup_connection(tmp_path: Path, monkeypatch):
"""refs #680: _bib_lookup_factory opens its own read connection to
bib.sqlite (via a `.close` attribute on the returned callable —
the same contract `extract()` honours in its `finally`); extract_ocr
used to never call it, leaking the connection."""
cdir = tmp_path / "CMS-2024-0001" / "CMS-2024-0001-0001"
cdir.mkdir(parents=True)
(cdir / "combined.md").write_text(
"---\nattachments:\n- status: ocr_needed\n---\nbody\n"
)
closed = {"called": False}
def fake_lookup(comment_id: str) -> str:
return ""
fake_lookup.close = lambda: closed.__setitem__("called", True)
monkeypatch.setattr("cli.comments._bib_lookup_factory", lambda use_bib: fake_lookup)
monkeypatch.setattr("rex.comments.combine.extract_comment", lambda *a, **k: None)
monkeypatch.setattr("rex.comments.ocr.RapidOcrEngine", lambda: lambda p: "")
result = runner.invoke(app, ["extract-ocr", "--root", str(tmp_path)])
assert result.exit_code == 0, result.output
assert closed["called"] is True
def test_stats_after_extract(tmp_path: Path):
_seed(tmp_path)
runner.invoke(
@@ -127,3 +154,69 @@ def test_extract_attaches_combined_md_as_note(tmp_path: Path, monkeypatch):
runner.invoke(app, ["extract", "--root", str(tmp_path), "--workers", "1"])
assert len(notes()) == 1
assert md_attachments() == []
def test_extract_reattach_repairs_missing_note_for_current_dir(
tmp_path: Path, monkeypatch
):
"""refs #680: --reattach at the CLI level is the repair path — a dir
already "current" (combined.md up to date) is normally skipped
without touching bib at all, so a note lost from under it (e.g. a
bad Zotero sync) never comes back on a plain re-run. --reattach
re-fires the attach callback for every skipped dir."""
from bib import connect
from bib.item import Source
bib_db = tmp_path / "bib.sqlite"
monkeypatch.setattr("conf.path", lambda _: bib_db)
cdir = _seed(tmp_path)
comment_id = cdir.name
store = connect(str(bib_db))
item = Source(
title=f"Comment {comment_id}",
url=f"https://www.regulations.gov/comment/{comment_id}",
)
item_key = store.upsert(item)
store.close()
def note_count() -> int:
store = connect(str(bib_db))
n = (
store._con()
.execute(
"SELECT count(*) FROM notes n JOIN items i ON n.item_id = i.id "
"WHERE i.key = ?",
(item_key,),
)
.fetchone()[0]
)
store.close()
return n
result = runner.invoke(app, ["extract", "--root", str(tmp_path), "--workers", "1"])
assert result.exit_code == 0, result.output
assert note_count() == 1
# The note vanishes without combined.md changing at all — the dir
# stays "current" and a plain re-run must skip it untouched.
store = connect(str(bib_db))
store._con().execute(
"DELETE FROM notes WHERE item_id = (SELECT id FROM items WHERE key = ?)",
(item_key,),
)
store._con().commit()
store.close()
assert note_count() == 0
result = runner.invoke(app, ["extract", "--root", str(tmp_path), "--workers", "1"])
assert result.exit_code == 0, result.output
assert "skipped: 1" in result.output.lower()
assert note_count() == 0 # not repaired without --reattach
result = runner.invoke(
app, ["extract", "--root", str(tmp_path), "--workers", "1", "--reattach"]
)
assert result.exit_code == 0, result.output
assert note_count() == 1 # repaired