Fix Tier-1 regulation citations, ECCN as a Tier-2a shape, and section-rename orphans

[JSv4]

Three defects found while installing an export-control authority pack against a
live instance. Each failed silently, and two produced wrong answers rather
than missing ones.

They are separate commits and can be reviewed independently.

1. Tier-1 could not read a regulation citation

The statute forms the extractor was built around use bare integer section
numbers introduced by the word "Section". Regulations do neither.

surface form	before	after
Section 120.10 of the ITAR	(nothing)	itar:120.10
ITAR Section 120.41	itar:120 ← wrong document	itar:120.41
ITAR § 120.41	(nothing)	itar:120.41
22 C.F.R. § 126.1	(nothing)	itar:126.1
Part 122 of the ITAR	(nothing)	itar:122
Category XI of the United States Munitions List	(nothing)	usml:xi

Row 2 is the dangerous one: a citation to the definition of "specially
designed" resolved to the Part 120 overview.

This is not corpus-specific — every CFR-, state-admin-code- and
municipal-code-style corpus has these shapes.

All new patterns are gated on a registered authority alias, so Part 3 of the Agreement and Category IV of the Agreement still match nothing. The
Category divider list is deliberately narrow; widening it to
Title/Article/Chapter would fire against every other authority corpus for no
demonstrated gain.

2. ECCNs, as a Tier-2a shape

ECCN 3A611 carries no Section/Part/§ token and the section-number shape would
take 3 and stop at the A.

Added to grammars.py emitting a shape-level eccn: prefix, not a pack's
own key — the same test usreporter: and htsus: satisfy. An earlier revision
of this work put it in extractor.py and emitted ccl:<code> via a hardcoded
"ccl" alias lookup, which bakes one pack's naming convention into core. A
pack carrying the CCL maps eccn:3a611 to its own key with one equivalences
row.

Two tests in the Tier-1 suite assert that boundary holds, because the tempting
"just add it where the other patterns are" fix reintroduces the leak silently
and nothing else would fail.

3. Renaming a section stranded its document

bootstrap_authority_corpus found the existing document by canonical key, then
wrote through a helper that derives the corpus path from the title. A
section whose heading changed was written to a new path while the old document
stayed current — both carrying the same canonical_key. find_authority_target
orders by id and takes the first, so the superseded document won and the
key silently served stale text.

On a real rebuild this left all 21 USML categories serving
[STUB — replace with verbatim text] at IMPLEMENTING weight. The only signal
anywhere was 21 created, 0 updated in the install summary.

Also fixes an unrelated local-stack papercut found alongside it: the Django
setting for the warp-ingest API key defaults to "" while the service defaults
to abc123, so a stock --profile warp-ingest stack 401'd every PDF parse.

Testing

• 209 passing across test_enrichment_extractor_regulations,
  test_authority_section_rename, test_generic_grammars,
  test_enrichment_extractor, test_customs_trade_grammars,
  test_llm_citation_extractor
• 36 new regression tests; the Tier-1 and rename tests were each confirmed to
  fail against the unfixed code
• pre-commit run green after pre-commit clean, so the hook envs were
  rebuilt from scratch rather than resolved from cache

Measured end to end

On a clean install of a 4,679-section export-control domain pack: 1 of 12
probed citation surface forms resolved before, 12 of 12 after. The document
corpus then formed 157 reference edges, 155 resolved (98.7%) across 12
prefixes — the most-cited keys being exactly the dotted forms that previously
yielded nothing or the wrong key.

🤖 Generated with Claude Code

[JSv4]

Companion pack change: Open-Source-Legal/authority-packs#10, which adds the 638 eccn:<code> → ccl:<code> equivalence rows.

Worth stating explicitly for reviewers: the ECCN half of this PR is a breaking change for any install carrying a Commerce Control List corpus until that pack change lands. Before, core emitted ccl:3a611 directly; now it emits the shape key eccn:3a611, which resolves to nothing until the pack maps it. That is the intended direction — a pack's prefix should not be hardcoded in core — but the two need to land together.

The other two commits (Tier-1 regulation shapes, section-rename orphans) are independent of the pack and safe on their own.

[claude]

Review

Solid, well-scoped fix. Three independent, well-tested defects with clear regression coverage and honest commit messages about what was tried and rejected (ccl: in the extractor, widening the Category divider list). A few notes below, none blocking.

Findings

1. Title doesn't get re-synced on a heading-only rename (opencontractserver/enrichment/authorities.py, bootstrap() around L554-577 / _stamp_key L1108-1130).
   The version-up fix only calls _stamp_key(..., title=sec.heading) on the 'text differs' path (near L590). The 'text unchanged' branch (L554-577, the current == sec.text case) calls self._stamp_key(existing.id, sec, aliases) without title=, so Document.title never updates there. _section_metadata() doesn't include the heading, so if only the heading changes and body text stays byte-identical, explicit_metadata_changed/default_metadata_missing are both False and the run hits skipped += 1 — the document keeps serving its old title indefinitely, silently. It's much lower severity than the bug fixed here (content stays correct, no forked document, find_authority_target still resolves fine) — but since the whole premise of this PR is 'sections get renamed between installs,' a text-preserving heading fix (e.g. a typo correction) is a realistic case not covered by test_authority_section_rename.py's three tests (all three change body text alongside/instead of the heading). Worth either threading title=sec.heading through the restamp call too, or a comment noting it's accepted as out of scope.

2. _compile_law_re's optional 'the' is a side effect (opencontractserver/enrichment/extractor.py L193-197).
   The suffix-style pattern changed from requiring literal of\s+the\s+ to of\s+(?:the\s+)?. None of the new tests exercise 'Section N of AUTH' without 'the' (all the PR's motivating examples — dotted sections, §, Part — keep 'of the'), so this reads like an incidental broadening picked up when the token/number pieces were factored into shared constants, rather than a deliberately-tested behavior change. It's plausibly desirable (citations to acts often omit 'the,' e.g. 'Section 4(a)(5) of Securities Act of 1933'), but as-is it's an unreviewed widening of an existing, already-shipped pattern rather than the new regulation-only surface the PR is scoped to. A one-line test either way would make the intent explicit.

Things verified and liked

• The _SECTION_NUMBER/_ECCN_RE 'don't swallow a sentence-final period' behavior is correct: (?:\.\d+)* and (?:\.[a-z0-9.]+)? both require a trailing digit/char after the dot, so trailing full stops are preserved (hand-traced; matches test_sentence_final_period_is_not_swallowed).
• Named-group reuse across the _compile_division_re alternation is handled correctly (sec/sec2, auth/auth2) — Python's re would otherwise raise on redefinition; easy to get wrong and it wasn't.
• The ECCN_PREFIX / Tier-2a placement decision is well-justified and actively guarded by ECCNStaysOutOfTier1Tests — good defense against the 'just add it next to the other patterns' regression the PR description calls out.
• The rename fix's core mechanism checks out: routing the write through existing.title so create_or_update_text_document derives the same corpus path (version-up) rather than a new one, then renaming only the freshly created version via _stamp_key, keeps find_authority_target's 'first document with this key wins' query safe by construction (only one live document per key going forward) rather than trying to fix the ordering/lookup side instead.
• local.yml's WARP_INGEST_API_KEY default now matches the pattern production.yml already used (L161) — consistent fix, not a new pattern invented for this PR.
• Good test hygiene: the PR notes the Tier-1/rename tests were confirmed to fail against the pre-fix code, and the ECCN boundary tests exist specifically to catch a known tempting-but-wrong fix.

Minor

• _DIVISION_NUMBER = r"[IVXLCivxlc]+" doesn't validate actual Roman-numeral well-formedness (e.g. 'IIII' or 'VIVI' would match the character class), and there's no trailing word boundary. Given it's gated behind both a divider word (Category) and a registered authority alias, a real false positive is very unlikely — flagging only because it's cheap to note.

No security or performance concerns — the new patterns are all linear-time (no nested unbounded quantifiers), and the DB-touching change (authorities.py) stays within the existing permission/visibility surface (CorpusDocumentService.get_corpus_documents) without introducing new inline Tier-0 access.

[claude]

Review

Overall this is a well-scoped, well-tested fix — three independent, clearly-motivated defects, each with regression tests that were confirmed to fail against the unfixed code. The Tier-1 vs Tier-2a boundary reasoning for ECCNs (shape-level eccn: prefix vs. a pack's own key) is exactly right and the guarding test (ECCNStaysOutOfTier1Tests) is a good call — it protects an architectural invariant that's easy to accidentally erode later.

One correctness bug found, plus a couple of minor nits.

Bug: ECCN paragraph-suffix regex swallows a trailing sentence period

opencontractserver/enrichment/grammars.py (_ECCN_RE, ~line 182-185):

_ECCN_RE = re.compile(
    r"\bECCNs?\s+(?P<code>\d[A-E]\d{3}(?:\.[a-z0-9.]+)?)",
    re.IGNORECASE,
)

The paragraph-suffix group (?:\.[a-z0-9.]+)? includes . inside the repeated character class, not just as the leading separator. When a paragraph-suffixed ECCN sits at the end of a sentence — e.g. "...controlled under ECCN 9A610.a." — the trailing full stop gets absorbed into code because . is itself a valid member of [a-z0-9.]+. Verified directly against the compiled pattern:

$ printf 'under ECCN 9A610.a.\n' | grep -oP '\bECCNs?\s+\d[A-E]\d{3}(?:\.[a-z0-9.]+)?'
ECCN 9A610.a.

vs. the non-sentence-final case, which is correct:

$ printf 'controlled by ECCN 9A610.a\n' | grep -oP '\bECCNs?\s+\d[A-E]\d{3}(?:\.[a-z0-9.]+)?'
ECCN 9A610.a

So "This item is controlled under ECCN 9A610.a." produces canonical_key = "eccn:9a610.a." (trailing dot), which will never match any pack's eccn:9a610.a equivalences row — a silent resolution failure of exactly the kind this PR is otherwise explicitly hunting down. _SECTION_NUMBER in extractor.py gets this right for the analogous statute/regulation case by requiring a digit after every dot ((?:\.\d+)*, with the comment "requires digits after each dot, so a sentence-final period is never swallowed") — the ECCN grammar doesn't apply the same discipline because the suffix is alphanumeric rather than pure digits.

None of the new tests in test_generic_grammars.py::ECCNGrammarTests exercise a paragraph-suffixed ECCN followed immediately by a sentence period (test_paragraph_suffix_preserved uses "controlled by ECCN 9A610.a" with no trailing punctuation at all), so this slipped past the otherwise-thorough test suite.

Suggested fix: require the char after each internal dot to include at least one non-dot alphanumeric before allowing another dot, e.g. something like (?:\.[a-z0-9]+)* (repeated single-level groups, mirroring _SECTION_NUMBER's structure) instead of one (?:\.[a-z0-9.]+)+ blob — plus a regression test with a paragraph suffix at the end of a sentence.

Minor nits (non-blocking)

• extractor.py::_divisions duplicates the Candidate(...) construction that _law_candidate already encapsulates, just against a different pair of resolved (sec, auth_text) values pulled from the two alternation branches. Since _law_candidate currently takes the re.Match directly and reads fixed group names (sec/auth), it can't be reused as-is for the sec/sec2/auth/auth2 shape — but splitting it into _law_candidate(m) (thin) + a shared _build_law_candidate(m, sec, auth_text) would let _divisions reuse the latter and remove the duplication.
• authorities.py: title=(existing.title if existing is not None else None) or sec.heading treats a blank ("") existing.title the same as None/first-install, falling back to the new heading. Since Document.title is null=True, blank=True, this is theoretically reachable and would reintroduce the exact forking bug this PR fixes (path derived from the new heading) for a document that happens to have a blank title — though in practice this is deliberate (an empty title would otherwise crash create_or_update_text_document's if not title or not title.strip(): raise ValueError(...) guard), and authority section headings are never blank in real packs, so this is very unlikely to bite. Just flagging for awareness, not requesting a change.

Nice touches worth calling out

• Gating all new Tier-1 patterns on the registered-alias alternation (so "Part 3 of the Agreement" / "Category IV of the Agreement" still match nothing) is the right way to add precision-sensitive patterns without opening the floodgates for unrelated corpora.
• test_authority_section_rename.py::test_renamed_section_resolves_to_the_new_text targets the actual observable symptom (stale text being served) rather than just structural document counts — that's the test that would have caught the real-world bug.
• The local.yml fix for the WARP_INGEST_API_KEY default mismatch is a nice, easy-to-miss papercut to catch incidentally.

[codecov]

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
opencontractserver/enrichment/extractor.py	94.11%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[claude]

Review

Read through all four commits (Tier-1 regulation citations, ECCN Tier-2a grammar, section-rename version-up, warp-ingest local.yml default) plus the new/changed tests. Overall this is a well-scoped, well-documented fix — the commit messages and inline comments do a good job explaining why each pattern/boundary exists, and the regression tests are specific enough to catch the "tempting wrong fix" the comments warn about (e.g. ECCNStaysOutOfTier1Tests, test_part_requires_a_registered_authority).

I traced the new regexes (_SECTION_NUMBER, _SECTION_TOKEN, _compile_division_re, the ECCN pattern in grammars.py) by hand against the new test cases and they check out — dotted section numbers, §/§§ with/without a following space, the of the → of (the)? relaxation, and the Roman-numeral Category alternation all behave as documented, and the alias-gating correctly keeps Part 3 of the Purchase Agreement / Category IV of the Agreement from matching.

Possible gap: heading-only renames (unchanged body text) still leave a stale Document.title

AuthorityCorpusBootstrapper.bootstrap (opencontractserver/enrichment/authorities.py:551-606) has two branches after finding existing by key:

• Text differs → routes through create_or_update_text_document at existing.title (old path) and then calls self._stamp_key(out["document_id"], sec, aliases, title=sec.heading) — this is the fixed path, and it's well covered by test_authority_section_rename.py.
• Text is unchanged (current == sec.text, lines 562-577) → only restamps custom_meta via self._stamp_key(existing.id, sec, aliases) — no title= argument is passed, and _section_metadata (line 1052) never includes the heading in custom_meta, so explicit_metadata_changed/default_metadata_missing can't detect a heading change either. If a source pack corrects a heading/title without changing the body text, that document's Document.title never gets updated and stays stale indefinitely on every future re-run — it isn't a new-vs-old-document forking bug (still one document, still the current content for the key), but it does look like exactly the class of "silent, only visible as title != heading" issue this PR is otherwise closing. Given the PR's stated goal was fixing the rename path in general, it might be worth deciding whether this branch should also thread title=sec.heading through to _stamp_key (and/or fold heading into _section_metadata so it participates in the change-detection), and adding a test for "heading changes, text does not."

Minor nit

opencontractserver/enrichment/authorities.py:595 — title=(existing.title if existing is not None else None) or sec.heading could be simplified to title=(existing.title if existing else sec.heading) (the existing is not None check is redundant with the truthy or, since we're already inside if existing is not None:... actually this line is outside that branch, so existing can be None here — but the or sec.heading still handles that). Purely stylistic, not a correctness issue.

Other things checked, no issues found

• local.yml / production.yml / config/settings/base.py: confirmed WARP_INGEST_API_KEY defaults now genuinely match ("" in Django settings vs. abc123 in the service) — the fix closes the gap described.
• _eccns grammar: anchored correctly on the literal ECCN/ECCNs, keys are lowercased, and it stays out of the Tier-1 registry extractor as documented (ECCNStaysOutOfTier1Tests) — good boundary enforcement between shape-level and pack-level prefixes.
• Candidate/_cand field usage in the new _divisions and _eccns helpers is consistent with sibling extractors (registry-tier candidates leave jurisdiction/authority_type as None, grammar-tier candidates set them).
• Test coverage is thorough for the paths that are covered (36 new tests cited in the PR description, plus the two "boundary" tests guarding against the tempting-but-wrong fixes called out in the commit messages).