Skip to content

feat(pl): Polish legislation corpus — schema, parser, register harvester - #2281

Open
overthelex wants to merge 9 commits into
mainfrom
feat/pl-corpus-harvest
Open

feat(pl): Polish legislation corpus — schema, parser, register harvester#2281
overthelex wants to merge 9 commits into
mainfrom
feat/pl-corpus-harvest

Conversation

@overthelex

@overthelex overthelex commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Foundation for the Polish corpus: schema, article parser, register harvester and the snapshot-chain builder. Draft — the text harvest, the court harvesters, the audit and the legacy repair are not in this PR.

Why the shape is what it is

Poland publishes no point-in-time legislation service. The only texts that exist are the one published on promulgation and one per obwieszczenie w sprawie ogłoszenia jednolitego tekstu — and the consolidated text is served under the obwieszczenie's ELI, not the base act's:

  • DU/1974/141/text.html is the 1974 Kodeks pracy (8 hits for socjalistyczn)
  • DU/2020/1320/text.html is the 2020 consolidation (0 hits, 12 for monitoring)

So the schema stores published snapshots plus the full amendment edge set and refuses to interpolate. pl_article_as_of() returns the nearest published text and the amendments effective since it; there is no argument that suppresses the second half, because a caller who saw only the text would reasonably believe it was the law on that date, and for Poland that is usually false.

Verified end-to-end against a throwaway Postgres with the real Kodeks pracy chain (11 snapshots):

query result
('DU/1974/141','1','2019-06-01') exact, DU/2019/1040, 0 amendments
('DU/1974/141','1','2020-06-01') stale, same snapshot, 3 amendments listed
('DU/1974/141','1','1950-01-01') pre_enactment
('DU/1974/141','1','1980-01-01') stale, returns the genuine 1974 wording

Snapshot 1 of that chain carries 71 amendments over 17 years with no consolidation published — the number a "current text only" schema would hide.

valid_to is derived from the next snapshot's exact_on, not from the source's expirationDate: that field marks when the obwieszczenie was superseded, so consecutive texts overlap by 28–55 days on three of the ten KP snapshots, and overlapping intervals cannot answer "which text on date D". source_expiration is stored beside it so the disagreement stays auditable.

Article extraction

Guided by /struct rather than by matching headings — every unit in the HTML carries id="{struct node id}". That replaces the Ukrainian monotonicity heuristic with an identity, which is strictly stronger: DU/2020/1320 has 497 unit_arti anchors for a 494-article code, the extras being articles quoted inside the obwieszczenie's own passages. No threshold catches that.

Fixture tests assert exact counts against the live API: 305, 494, 1088, 1295, 363.

Three defects the tests caught, each of which would have shipped silently

  • Charset. Documents declare it twice in one attribute (charset=UTF-8; charset=UTF-8) and lxml falls back to latin-1. Struct ids contain Polish letters (bran_piąty-chpt_I-arti_114), so mis-decoded ids stop matching struct — 134 of 305 articles lost on Kodeks pracy while every anchor count still looked correct.
  • Footnotes render inline as <a class="gloss-link"><sup>3)</sup><span>…prose…</span></a>. Left in, they put 21,332 characters of editorial note into the provisions of DU/2020/1320 (1.6% of the body), and the marker after an article number turned Art. 47 into Art. 47^6 — firing the label check on formatting.
  • Numbering has a third level (arti_18_3_a = art. 18³ᵃ, 24 of 494 articles) and a range form (arti_266-280, a repealed span with an empty body — a fact, not a missing value).

Measured, not assumed

  • DU: 97,681 acts; the 105 year listings sum exactly to the declared actsCount. 39,110 serve HTML (2012–2024 ≈ 100%, 2025–2026 0% — a publication lag, so the sync must re-poll HTML-less acts).
  • Monitor Polski: 0 of 66,532 acts have HTML. Register-and-graph only; no text pipeline.
  • /references is redundant — the act detail inlines the same edges, byte-identical on DU/1964/93 across all five categories. Saves 164,213 requests.
  • Konstytucja RP DU/1997/483 has no machine-readable text (no HTML, /struct 404, zero-byte text.html, PDF only). Carried as a negative landmark, verdict 903, rather than an assertion that cannot hold.
  • One non-monotonic article is a defect in the published source: DU/1964/93 labels the article at position 536 Art. 538. in both struct and the DOM while its text is the real 536. Pinned rather than smoothed over.

Note on the existing Polish data

pl_court_decisions already holds 2,864,093 rows / 105 GB from three snapshot sources. It is stale (hf-pl-nsa stops 2025-02-26), its ids are built from parquet row positions so a re-import duplicates rather than updates, and it loaded with ON CONFLICT DO NOTHING so bad text cannot be repaired. This PR adds judgment_id and text_status to make the repair possible; the repair itself is a later PR.

Migrations

184 and 185 apply cleanly and are idempotent (verified by running each twice). 186 must not go through the migration runner — it wraps each file in one transaction and CREATE INDEX CONCURRENTLY cannot run in one, the same reason scripts/nl/179b_* sits outside.

🤖 Generated with Claude Code


Summary by cubic

Polish legislation corpus end to end: schema, ELI register harvester, snapshot-chain builder, one-pass text harvest, and a struct‑guided article parser; includes a runbook and final corpus state in scripts/pl/README.md. Poland has no point‑in‑time service, so we store published snapshots plus the amendment graph; pl_article_as_of() returns the nearest published text and amendments since it.

  • Schema and behavior: pl_acts, pl_act_references, pl_act_snapshots, pl_act_articles, pl_act_units, pl_snapshot_texts; view pl_act_timeline; function pl_article_as_of(...). Consolidations attach to the base act; exact_on from legal status/announcement; valid_to derives from the next exact_on (not overlapping expirationDate). Adds judgment_id and text_status to pl_court_decisions for dedupe/repair.
  • Harvesters and chain: register harvester enumerates year listings, then act details into pl_acts and pl_act_references (skips /references, saving 164,213 calls); snapshot chain built in pure SQL (scripts/pl/build_pl_snapshots.sql), re‑runnable.
  • Text and parser: one pass per snapshot fetches /struct and text.html, stages raw files, parses into articles, then writes pl_snapshot_texts so the anti‑join worklist stays safe. HTML‑less snapshots write verdict 903 without a fetch (all MP, many older DU); verdict 901 stores whole‑document text when struct is absent. Parser walks /struct ids, fixes charset double‑declare, strips inline footnotes, supports third‑level numbering and ranges, and repairs unescaped quotes, raw control characters, and trailing backslashes in /struct JSON.

Rollout and migration

  • Apply mcp_backend/src/migrations/190_pl_legislation.sql and 191_pl_law_texts.sql via the migration runner.
  • Do not run scripts/pl/192_pl_indexes_concurrently.sql through the runner; apply it manually and ensure no INVALID indexes remain.
  • Optional: switch Polish FTS to 'polish' as noted in 192_pl_indexes_concurrently.sql.
  • Seed and build:
    • python3 scripts/pl/harvest_eli_register.py --listings
    • python3 scripts/pl/harvest_eli_register.py --details
    • psql -f scripts/pl/build_pl_snapshots.sql
    • python3 scripts/pl/harvest_eli_texts.py

Written for commit 5915603. Summary will update on new commits.

Review in cubic

vovkes and others added 9 commits August 14, 2026 12:18
Poland publishes no point-in-time legislation service. The only texts that
exist are the one published on promulgation and one per "obwieszczenie w
sprawie ogloszenia jednolitego tekstu" - and, verified against the API, the
consolidated text is served under the OBWIESZCZENIE's ELI, not the base act's:
DU/1974/141/text.html is the 1974 Kodeks pracy, DU/2020/1320/text.html is the
2020 consolidation. So 184/185 store published snapshots plus the full
amendment edge set and refuse to interpolate between them. pl_article_as_of()
returns the nearest published text AND the amendments effective since it, with
no argument that suppresses the second half.

Article extraction is guided by /struct rather than by matching headings,
because every unit in the HTML carries id="{struct node id}". That replaces the
Ukrainian monotonicity heuristic with an identity - extract exactly what struct
declares - which is strictly stronger: DU/2020/1320 has 497 <div unit_arti>
anchors for a 494-article code, the extras being articles quoted inside the
obwieszczenie's own passages.

Three defects the fixture tests caught, each of which would have shipped
silently:

- The documents declare charset twice in one attribute
  ("text/html; charset=UTF-8; charset=UTF-8") and lxml falls back to latin-1.
  Struct ids contain Polish letters (bran_piaty-chpt_I-arti_114), so mis-decoded
  ids stop matching struct and the articles are dropped - 134 of 305 lost on
  Kodeks pracy while every anchor count still looked correct.
- Footnotes render as <a class="gloss-link"><sup>3)</sup><span>...prose...</span></a>
  inline. Left in place they put 21,332 characters of editorial note into the
  provisions of DU/2020/1320 (1.6% of the body), and the <sup>3)</sup> marker
  sitting after an article number turned "Art. 47" into "Art. 47^6".
- Polish numbering has a third level (arti_18_3_a = art. 18 sup 3 letter a,
  24 of 494 articles in the Kodeks pracy) and a range form (arti_266-280).

Baseline pinned live: DU declares 97,681 acts and the 105 year listings sum to
97,681, of which 39,110 serve HTML; MP has 66,532 acts and 0 with HTML, so it
is a register-and-graph corpus with no text pipeline. Konstytucja RP DU/1997/483
has no HTML, no struct and a zero-byte text.html, and is carried as a negative
landmark (verdict 903) rather than an assertion that cannot hold.

Fixtures assert exact article counts against the live API: 305, 494, 1088,
1295, 363. The single non-monotonic article is a defect in the published
source - DU/1964/93 labels the article at position 536 "Art. 538." in both
struct and the DOM while its text is the real 536 - and is pinned rather than
smoothed over.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stage 1 enumerates the register from 197 year listings, then fills in the
per-act fields the listing omits and the reference graph, worklist-as-anti-join
so a killed run just leaves fewer rows for the next one. /references is not
called: the act detail inlines the same edges, verified byte-identical on
DU/1964/93 across all five of its categories, so calling it would double the
pass for nothing.

Stage 2 derives the snapshot chain in SQL. Verified end-to-end against a
throwaway Postgres with the real Kodeks pracy: 11 snapshots, 1 ogloszony plus
10 tekst jednolity, and exact_on_src records that DU/1998/94 falls back to
announcementDate while the other ten carry legalStatusDate.

The chain shows why valid_to is derived rather than taken from the source. The
API's expirationDate is when the OBWIESZCZENIE was superseded, so consecutive
consolidated texts overlap - DU/2019/1040 expires 2020-07-30 while DU/2020/1320
is already exact from 2020-06-18, and three of the ten snapshots overlap by
28-55 days. Overlapping intervals cannot answer "which text on date D".
source_expiration is stored beside the derived value so the disagreement stays
auditable.

pl_article_as_of verified on all four confidence states against loaded text:
  ('DU/1974/141','1','2019-06-01') -> exact,  DU/2019/1040, 0 amendments
  ('DU/1974/141','1','2020-06-01') -> stale,  same snapshot, 3 amendments listed
  ('DU/1974/141','1','1950-01-01') -> pre_enactment
  ('DU/1974/141','1','1980-01-01') -> stale, returning the genuine 1974 wording
Snapshot 1 of the chain carries 71 amendments over 17 years with no
consolidation published - the number a "current text only" schema would hide.

Three fixes found by running it rather than reading it:
- psql autocommits each statement, so TEMP ... ON COMMIT DROP was dropped before
  the COPY on the next line could see it; the upsert now runs in an explicit
  transaction, which also makes each batch atomic.
- over ssh the remote command is one string handed to a remote shell, so
  "-c COPY t (a, b) FROM STDIN" has to be shlex-quoted before joining.
- esc() no longer maps empty string to NULL. A repealed article legitimately has
  no body - the Kodeks pracy carries "Art. 266-280." as one unit covering a
  repealed span - and NULL both loses the difference between "repealed" and
  "extraction failed" and violates NOT NULL.

plprod deliberately does not reuse prod_writer.copy_into: its _escape_copy maps
newline to a space, which is right for metadata and wrong for statute text,
where flattening an article destroys the paragraph structure that makes
"art. 415 § 1" addressable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…g it

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
184 and 185 were already taken. Prod's schema_migrations has entries through
189, and the tree carries 184_v2_ua_opendata_tool_pricing.sql and
185_npa_corpus_tool_pricing.sql. The earlier survey that reported 182 as the
highest was reading a stale branch.

migrate.ts keys schema_migrations on the filename and orders lexicographically,
so a collision does not skip a file - it makes the order between two same-
numbered files depend on their alphabetical tail, which is how 153/154 already
became ambiguous.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
harvest_eli_texts.py fetches struct and text.html for a snapshot in ONE pass
over one worklist, because the parser needs both documents simultaneously and
splitting them would double the resume bookkeeping for nothing. Raw documents
are staged on disk before parsing, so changing an extraction rule is a reparse
rather than a refetch.

Snapshots the source never published as HTML get verdict 903 written in SQL
without a fetch - every Monitor Polski act and 58,571 DU acts including
Konstytucja RP. 903 is not a failure; it is what separates a gap in the source
from a gap in our harvest, and conflating the two is how a corpus reports high
coverage and is wrong.

Text rows are written after their articles, not before: they are what the
anti-join keys on, so the other order would let a crash in between leave a
snapshot marked done with no articles.

Register state on prod: the listings pass loaded 164,213 acts in 271 s, and both
publishers matched their declared actsCount exactly (DU 97,681 with 39,110
serving HTML; MP 66,532 with none). The details pass is running at 10.8 acts/s
with no failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ISAP does not escape ASCII double quotes inside JSON string values. Polish
typography opens a quotation with the low quote and the source frequently closes
it with a straight ", so a title like

    "title" : "...przedsiebiorstwa panstwowego ,,Polskie Koleje Panstwowe"1)",

terminates the JSON string early and makes the whole 76 KB payload unparseable.
DU/2024/561 is such an act, and json.loads losing it cost all 140 of its
articles - an entire consolidated statute discarded over one quotation mark.

repair_struct_json walks the payload and escapes any quote inside a string that
is not followed by a structural delimiter, then parses. Verified on DU/2024/561:
2 top-level parts, 140 arti nodes recovered. It still raises if the payload is
genuinely broken, so an unrepairable one remains an honest failure rather than a
silent empty result.

Verdict 901 now also stores the whole document text. Without a struct there are
no articles, but holding an act unsegmented beats losing it, and the verdict
records which of the two the row is.

Found by looking at the first five failures out of 6,600 snapshots instead of
letting a 0.08% rate accumulate: they turned out to be four distinct classes,
of which this was the only one destroying data.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The quote repair recovered 2 of the 14 unparseable payloads. The other 12 fail
on a second, independent defect in the same serialiser: long table captions are
wrapped across physical lines and the newline is emitted raw inside the JSON
string, which JSON forbids. DU/2020/2075 is 1.6 KB and dies at char 1375 on a
caption for a Ministry of Finance pay table.

Both rules are guarded on in_string, because outside a string a newline is legal
whitespace and a quote is structure.

Re-running the first fix taught the more useful lesson: it recovered DU/2024/561
in full (140 articles) and left twelve acts holding whole-document text with no
articles. That is the fallback working as intended - an act held unsegmented
rather than lost - but it also meant a single verdict was covering two unrelated
source bugs, and only looking at the smallest remaining payload separated them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third defect in the same serialiser. A title ending "Zalacznik - WZOR\" leaves
\" looking like an escaped quote, so the string never closes. If the character
after that quote is a structural delimiter, the backslash is literal content and
the quote is the real terminator (DU/2018/428).

Recovers the last three unparseable payloads. The five remaining 902s are not a
parser problem: /struct genuinely 404s and text.html is an ~800-byte header stub
with no body, so no machine-readable text was ever published for them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@overthelex
overthelex marked this pull request as ready for review August 18, 2026 22:03
@overthelex

Copy link
Copy Markdown
Owner Author

Corpus is complete and verified on prod — marking ready for review

Final state

acts 164,213 (DU 97,681 / MP 66,532), 100% detail coverage
reference edges 732,983
snapshots 164,206 over 154,843 laws; 4,223 with more than one
articles 673,735
struct units 5,539,599

Verdicts: 200 = 39,036 · 903 = 125,100 (source published no HTML: all of MP plus most pre-2012 DU) · 904 = 62 · 902 = 5 · 900 = 1 · 901 = 1 · 905 = 1. Seven unresolved out of 39,106 HTML snapshots (0.018%), each explained below.

Landmarks, all exact against the live DB: KC 1088 · KC t.j. 2023 1295 · KP 305 · KP t.j. 2020 494 · KK 363 · KPK 682 · KPC 1153 · KPA 196. Negative landmark Konstytucja RP DU/1997/483 = verdict 903, 0 articles.

Integrity, all zero: snapshots without a text row · orphaned text rows · orphaned articles · non-dense seq · laws with more than one open interval.

pl_article_as_of on the live corpus:

KP art.1 @2019-06-01 -> exact          DU/2019/1040  0 amendments
KP art.1 @2020-06-01 -> stale          DU/2019/1040  3 amendments
KC art.415 @2015-01-01 -> stale        DU/2014/121   1 amendment
KPC art.1 @2024-01-01 -> stale         DU/2023/1550  6 amendments
KP art.1 @1950-01-01 -> pre_enactment

Art. 1 of the labour code at 1980 returns "…służy umacnianiu socjalistycznych stosunków pracy", at 2024 "…prawa i obowiązki pracowników i pracodawców" — the snapshots are genuinely different texts, not one text repeated.

Three source defects found by re-running the failures

The first text pass left 83 failures. Chasing them down turned one verdict into three distinct bugs in ISAP's /struct JSON serialiser, each of which alone makes a whole payload unparseable:

  1. Unescaped ASCII double quotes — Polish typography opens with and the source closes with a straight ". DU/2024/561: 76 KB and 140 articles behind one quotation mark.
  2. Raw control characters — long table captions wrapped across physical lines emit the newline raw inside the string (DU/2020/2075).
  3. Unescaped trailing backslash — a title ending Załącznik - WZÓR\ makes \" look like an escaped quote so the string never closes (DU/2018/428).

repair_struct_json() handles all three, guarded on in_string because outside a string those characters are legal structure. 901 went 14 → 1.

The five remaining 902s are not a parser problem: /struct genuinely 404s and text.html is an ~800-byte header stub with no body, so no machine-readable text was ever published for those acts.

Note for the reviewer

Migrations 190/191 were applied to prod manually on 2026-08-14 so the harvest could run. They are idempotent (verified by running each twice), so the CI runner re-applying them on merge is harmless. 192 is deliberately not run — it contains a UNIQUE INDEX CONCURRENTLY on the 105 GB pl_court_decisions, which should wait until that corpus is repaired, since it would otherwise index rows the repair is about to collapse.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

26 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/pl/build_pl_snapshots.sql">

<violation number="1" location="scripts/pl/build_pl_snapshots.sql:35">
P2: On rerun, acts missing from an aggregate retain stale nonzero counters, and `snapshot_count` can remain wrong after an act stops being a base snapshot. Reset all three derived columns to zero before applying the aggregate updates.</violation>

<violation number="2" location="scripts/pl/build_pl_snapshots.sql:46">
P1: When Stage 2 is rerun after HTML becomes available or an act becomes classified as a consolidation, this delete leaves stale parsed-text rows keyed by the same `snapshot_eli`. Reconcile or invalidate the dependent text, article, and unit rows when rebuilding snapshots so the harvester can fetch or reparse them.</violation>
</file>

<file name="scripts/pl/harvest_eli_texts.py">

<violation number="1" location="scripts/pl/harvest_eli_texts.py:70">
P3: The progress log always reports `notext=0`, hiding empty and no-HTML outcomes while they are counted as failures or omitted. Increment `no_text` for those verdicts or remove the unused metric.</violation>

<violation number="2" location="scripts/pl/harvest_eli_texts.py:142">
P2: `--reparse` is documented as "parse from the on-disk cache only, no network", but the flag has no effect. `handle(row, reparse)` never reads its `reparse` parameter and always calls `fetch(..., use_cache=True)`, which is the same behavior as a normal run. Worse, in `--reparse` mode a snapshot whose cache files are missing still triggers a live API request, contradicting the "no network" guarantee; and the flag cannot be used to force a refresh either. Pass the flag into the fetch/cache logic so a reparse truly never contacts the API (skipping or erroring on absent cache files), or remove it.</violation>

<violation number="3" location="scripts/pl/harvest_eli_texts.py:159">
P2: When `/struct` returns 599 or 900, this branch still writes 902. Preserve the actual status and map only 404 to `P.NO_STRUCT`, otherwise transient and empty responses are misclassified as missing structure.</violation>

<violation number="4" location="scripts/pl/harvest_eli_texts.py:174">
P2: `901` is defined for malformed or non-act HTML, but this branch uses it for malformed `/struct` JSON. Add a distinct struct-parse verdict and store that status here so audits can distinguish HTML failures from structure failures.</violation>

<violation number="5" location="scripts/pl/harvest_eli_texts.py:187">
P2: When parsing yields no articles, `full` is the only stored text, but this line leaves `text_hash` NULL. Hash the stored fallback text as well so those snapshots participate in cross-snapshot deduplication.</violation>

<violation number="6" location="scripts/pl/harvest_eli_texts.py:249">
P1: When a 903 row is marked while `has_html` is false, a later metadata refresh to true leaves that row excluded by this predicate. Include existing 903 rows when `s.has_html` becomes true so publication-lag snapshots are harvested.</violation>

<violation number="7" location="scripts/pl/harvest_eli_texts.py:266">
P1: If the process stops after this `COPY`, article rows are committed while the anti-join text rows remain absent. The next run then hits duplicate primary keys instead of resuming; load the three tables atomically or make replay idempotent.</violation>
</file>

<file name="scripts/pl/harvest_eli_register.py">

<violation number="1" location="scripts/pl/harvest_eli_register.py:48">
P2: When an act has `previousTitle` metadata, this loader silently drops it because `ACT_COLS` and `act_row()` omit `previous_titles`. Map and normalize that source field so the register preserves the schema’s prior-title data.</violation>

<violation number="2" location="scripts/pl/harvest_eli_register.py:183">
P3: `counts['failed'] += 1` runs inside `handle_detail`, which executes in `ThreadPoolExecutor` workers, so multiple threads mutate the shared `counts` dict concurrently. The read-modify-write is not atomic and can undercount failures, hiding real failure counts from the progress logs. Guard the counter with the same `_lock` used by `throttle()`, or return the failure status and accumulate in the main thread.</violation>

<violation number="3" location="scripts/pl/harvest_eli_register.py:241">
P1: If the reference write fails after the act write succeeds, `detail_fetched_at` prevents the anti-join from retrying that act, permanently losing its graph edges. Persist references before marking acts fetched, or make both writes one transaction.</violation>
</file>

<file name="scripts/pl/pl_article_parser.py">

<violation number="1" location="scripts/pl/pl_article_parser.py:346">
P1: In `_text_with_offsets`, `spans` record offsets measured in the `raw` string, but they are dropped whenever `_clean(raw)` changes the length - which happens for essentially every real article: block tags emit `\n` between paragraphs, and `_clean` strip()s the trailing newline (and collapses whitespace), so `len(raw) != len(cleaned)` unless the article is a single line. As a result `spans={}` for almost all articles, so `char_from`/`char_to` on `pl_act_units` are never populated and the documented sub-article addressing feature (migration 191: `"art. 415 § 1" resolves to a (char_from, char_to) slice of pl_act_articles.text`) yields no offsets. The offsets are built against `raw` while the stored `article.text` is `cleaned`, so even in the rare equal-length case the mapping is not guaranteed. Compute offsets against the cleaned text stream instead of discarding them on a length change.</violation>

<violation number="2" location="scripts/pl/pl_article_parser.py:485">
P2: When a struct repeats an article ID, this dictionary keeps only the last occurrence, so the first occurrence’s unit rows resolve to the wrong `article_ord` (and repeated child spans collide). Track occurrence-specific article ordinals and spans instead of keying these mappings only by `struct_id`.</violation>
</file>

<file name="scripts/pl/plprod.py">

<violation number="1" location="scripts/pl/plprod.py:96">
P1: When the text harvest is interrupted after the article/unit COPY or a later COPY fails, the anti-join leaves the snapshot pending but those rows remain committed. The next run retries them and hits the primary keys; load all three tables transactionally or make the COPY idempotent.</violation>

<violation number="2" location="scripts/pl/plprod.py:162">
P2: `rows_of()` corrupts values containing a literal backslash followed by `n` or `t` because it decodes `\\n`/`\\t` before `\\`. Parse COPY escapes left-to-right so literal backslashes in returned strings survive.</violation>

<violation number="3" location="scripts/pl/plprod.py:168">
P3: `scalar()` is defined in the new plprod.py but never called anywhere in the repo (only its own `def` matches a repo-wide search), and its `splitlines()[2]` parsing depends on psql's column-header output layout. Since it's unused and brittle, drop it until a caller exists, or switch it to `rows_of`/`psql` which already parse reliably.</violation>
</file>

<file name="scripts/pl/192_pl_indexes_concurrently.sql">

<violation number="1" location="scripts/pl/192_pl_indexes_concurrently.sql:1">
P2: When any concurrent index build fails, the documented `psql -f` invocation continues with the remaining statements because this file does not enable `ON_ERROR_STOP`. The operator can therefore leave the idempotency/index set partially applied; add `\set ON_ERROR_STOP on` at the top or pass `-v ON_ERROR_STOP=1` in the documented command.</violation>
</file>

<file name="scripts/pl/test_pl_article_parser.py">

<violation number="1" location="scripts/pl/test_pl_article_parser.py:65">
P3: The negative-landmark setup is dead code. fetch_all() downloads DU/1997/483/.detail.json, but no assertion ever reads it: the landmark check calls P.parse(None, b"") directly, which unconditionally returns NO_STRUCT as soon as the parser sees struct is None. The fetched file is never consumed, and the test never exercises the actual landmark behavior (struct 404 / zero-byte html) that the comment says it validates. The convenience: either have the test load the fetched detail/HTML and assert NO_STRUCT through the real data path, or drop the fetch.</violation>
</file>

<file name="scripts/pl/pin_baseline.py">

<violation number="1" location="scripts/pl/pin_baseline.py:70">
P2: Concurrent year fetches can pass `throttle()` together, so a fast run bursts requests and does not enforce the intended source-rate margin. Protect the shared timestamp and enforce the intended aggregate interval before issuing requests.</violation>

<violation number="2" location="scripts/pl/pin_baseline.py:244">
P2: Running the documented `--landmarks-only` check over an existing full output replaces the pinned baseline with one containing no publisher counts, while a matching landmark run exits successfully. Write landmarks-only results to a separate file or refuse to overwrite an existing full baseline.</violation>
</file>

<file name="scripts/pl/README.md">

<violation number="1" location="scripts/pl/README.md:32">
P3: The claim that migrations are 190/191 because "184/185 were already taken" is inaccurate: migrations 186 through 189 also exist in mcp_backend/src/migrations (184_v2_ua_opendata... through 189_edrnpa_number_norm_index). The numbering landed at 190/191 because every prior number through 189 was taken. Update the explanation so it does not mislead about which slots were occupied.</violation>

<violation number="2" location="scripts/pl/README.md:44">
P2: The Run sequence omits `harvest_eli_texts.py`, the stage 3+4 text/struct harvest that is the whole purpose of the parser documented here. Following Run, a reader reaches `build_pl_snapshots.sql` then the fixture test and never populates `pl_act_articles`/`pl_snapshot_texts`, so the pipeline cannot be reproduced end to end. Add the text-harvest commands after snapshots are built.</violation>
</file>

<file name="mcp_backend/src/migrations/191_pl_law_texts.sql">

<violation number="1" location="mcp_backend/src/migrations/191_pl_law_texts.sql:51">
P2: If the harvester stops after these rows but before the marker row, the anti-join retries them and the primary key aborts the batch. Make the three writes atomic or make retries idempotent.</violation>

<violation number="2" location="mcp_backend/src/migrations/191_pl_law_texts.sql:208">
P2: When the requested article does not exist in a successfully extracted snapshot, this branch falsely reports that the source has no text. Add a distinct article-not-found result after checking the snapshot status.</violation>

<violation number="3" location="mcp_backend/src/migrations/191_pl_law_texts.sql:217">
P1: When a snapshot has verdict 904 or 905, this join still serves its rows and can label them exact. Restrict the join to snapshots whose `pl_snapshot_texts.http_status = 200` before exposing legal text.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

WHERE c.src_eli = a.eli;

-- 3. The snapshot chain.
DELETE FROM pl_act_snapshots;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When Stage 2 is rerun after HTML becomes available or an act becomes classified as a consolidation, this delete leaves stale parsed-text rows keyed by the same snapshot_eli. Reconcile or invalidate the dependent text, article, and unit rows when rebuilding snapshots so the harvester can fetch or reparse them.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/pl/build_pl_snapshots.sql, line 46:

<comment>When Stage 2 is rerun after HTML becomes available or an act becomes classified as a consolidation, this delete leaves stale parsed-text rows keyed by the same `snapshot_eli`. Reconcile or invalidate the dependent text, article, and unit rows when rebuilding snapshots so the harvester can fetch or reparse them.</comment>

<file context>
@@ -0,0 +1,152 @@
+ WHERE c.src_eli = a.eli;
+
+-- 3. The snapshot chain.
+DELETE FROM pl_act_snapshots;
+
+WITH raw AS (
</file context>

# proportionally only when nothing moved; otherwise drop the spans for this
# article. A wrong offset silently returns the wrong provision, which is
# worse than returning none, so the honest failure is to have no span.
if len(raw) != len(cleaned):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: In _text_with_offsets, spans record offsets measured in the raw string, but they are dropped whenever _clean(raw) changes the length - which happens for essentially every real article: block tags emit \n between paragraphs, and _clean strip()s the trailing newline (and collapses whitespace), so len(raw) != len(cleaned) unless the article is a single line. As a result spans={} for almost all articles, so char_from/char_to on pl_act_units are never populated and the documented sub-article addressing feature (migration 191: "art. 415 § 1" resolves to a (char_from, char_to) slice of pl_act_articles.text) yields no offsets. The offsets are built against raw while the stored article.text is cleaned, so even in the rare equal-length case the mapping is not guaranteed. Compute offsets against the cleaned text stream instead of discarding them on a length change.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/pl/pl_article_parser.py, line 346:

<comment>In `_text_with_offsets`, `spans` record offsets measured in the `raw` string, but they are dropped whenever `_clean(raw)` changes the length - which happens for essentially every real article: block tags emit `\n` between paragraphs, and `_clean` strip()s the trailing newline (and collapses whitespace), so `len(raw) != len(cleaned)` unless the article is a single line. As a result `spans={}` for almost all articles, so `char_from`/`char_to` on `pl_act_units` are never populated and the documented sub-article addressing feature (migration 191: `"art. 415 § 1" resolves to a (char_from, char_to) slice of pl_act_articles.text`) yields no offsets. The offsets are built against `raw` while the stored `article.text` is `cleaned`, so even in the rare equal-length case the mapping is not guaranteed. Compute offsets against the cleaned text stream instead of discarding them on a length change.</comment>

<file context>
@@ -0,0 +1,538 @@
+    # proportionally only when nothing moved; otherwise drop the spans for this
+    # article. A wrong offset silently returns the wrong provision, which is
+    # worse than returning none, so the honest failure is to have no span.
+    if len(raw) != len(cleaned):
+        spans = {}
+    return cleaned, spans
</file context>

refs += rrows

if acts:
plprod.upsert_rows("pl_acts", ACT_COLS, acts, ["eli"], prefer_new=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: If the reference write fails after the act write succeeds, detail_fetched_at prevents the anti-join from retrying that act, permanently losing its graph edges. Persist references before marking acts fetched, or make both writes one transaction.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/pl/harvest_eli_register.py, line 241:

<comment>If the reference write fails after the act write succeeds, `detail_fetched_at` prevents the anti-join from retrying that act, permanently losing its graph edges. Persist references before marking acts fetched, or make both writes one transaction.</comment>

<file context>
@@ -0,0 +1,282 @@
+                    refs += rrows
+
+        if acts:
+            plprod.upsert_rows("pl_acts", ACT_COLS, acts, ["eli"], prefer_new=True)
+            counts["acts"] += len(acts)
+        if refs:
</file context>

Comment thread scripts/pl/plprod.py
for i in range(0, len(rows), batch):
chunk = rows[i:i + batch]
data = "".join("\t".join(esc(c) for c in r) + "\n" for r in chunk)
r = subprocess.run(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When the text harvest is interrupted after the article/unit COPY or a later COPY fails, the anti-join leaves the snapshot pending but those rows remain committed. The next run retries them and hits the primary keys; load all three tables transactionally or make the COPY idempotent.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/pl/plprod.py, line 96:

<comment>When the text harvest is interrupted after the article/unit COPY or a later COPY fails, the anti-join leaves the snapshot pending but those rows remain committed. The next run retries them and hits the primary keys; load all three tables transactionally or make the COPY idempotent.</comment>

<file context>
@@ -0,0 +1,169 @@
+    for i in range(0, len(rows), batch):
+        chunk = rows[i:i + batch]
+        data = "".join("\t".join(esc(c) for c in r) + "\n" for r in chunk)
+        r = subprocess.run(
+            _argv(["-c", f"COPY {table} ({col_sql}) FROM STDIN WITH (FORMAT text)"]),
+            input=data, capture_output=True, text=True, timeout=PSQL_TIMEOUT)
</file context>

Comment on lines +217 to +219
LEFT JOIN pl_act_articles art
ON art.snapshot_eli = s.snapshot_eli
AND art.art_no = p_art_no

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When a snapshot has verdict 904 or 905, this join still serves its rows and can label them exact. Restrict the join to snapshots whose pl_snapshot_texts.http_status = 200 before exposing legal text.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcp_backend/src/migrations/191_pl_law_texts.sql, line 217:

<comment>When a snapshot has verdict 904 or 905, this join still serves its rows and can label them exact. Restrict the join to snapshots whose `pl_snapshot_texts.http_status = 200` before exposing legal text.</comment>

<file context>
@@ -0,0 +1,238 @@
+    coalesce((SELECT j FROM am), '[]'::jsonb),
+    (SELECT snapshot_eli FROM nx), (SELECT exact_on FROM nx)
+FROM s
+LEFT JOIN pl_act_articles art
+       ON art.snapshot_eli = s.snapshot_eli
+      AND art.art_no = p_art_no
</file context>
Suggested change
LEFT JOIN pl_act_articles art
ON art.snapshot_eli = s.snapshot_eli
AND art.art_no = p_art_no
LEFT JOIN pl_act_articles art
ON art.snapshot_eli = s.snapshot_eli
AND art.art_no = p_art_no
AND EXISTS (
SELECT 1
FROM pl_snapshot_texts t
WHERE t.snapshot_eli = s.snapshot_eli
AND t.http_status = 200
)

d, code = fetch_json(f"acts/{pub}/{year}?limit=5000")
if d is None:
print(f" {pub}/{year}: HTTP {code}, skipped", file=sys.stderr)
counts["failed"] += 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: counts['failed'] += 1 runs inside handle_detail, which executes in ThreadPoolExecutor workers, so multiple threads mutate the shared counts dict concurrently. The read-modify-write is not atomic and can undercount failures, hiding real failure counts from the progress logs. Guard the counter with the same _lock used by throttle(), or return the failure status and accumulate in the main thread.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/pl/harvest_eli_register.py, line 183:

<comment>`counts['failed'] += 1` runs inside `handle_detail`, which executes in `ThreadPoolExecutor` workers, so multiple threads mutate the shared `counts` dict concurrently. The read-modify-write is not atomic and can undercount failures, hiding real failure counts from the progress logs. Guard the counter with the same `_lock` used by `throttle()`, or return the failure status and accumulate in the main thread.</comment>

<file context>
@@ -0,0 +1,282 @@
+            d, code = fetch_json(f"acts/{pub}/{year}?limit=5000")
+            if d is None:
+                print(f"  {pub}/{year}: HTTP {code}, skipped", file=sys.stderr)
+                counts["failed"] += 1
+                continue
+            items = d.get("items") or []
</file context>

Comment thread scripts/pl/plprod.py
yield tuple(cells)


def scalar(sql):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: scalar() is defined in the new plprod.py but never called anywhere in the repo (only its own def matches a repo-wide search), and its splitlines()[2] parsing depends on psql's column-header output layout. Since it's unused and brittle, drop it until a caller exists, or switch it to rows_of/psql which already parse reliably.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/pl/plprod.py, line 168:

<comment>`scalar()` is defined in the new plprod.py but never called anywhere in the repo (only its own `def` matches a repo-wide search), and its `splitlines()[2]` parsing depends on psql's column-header output layout. Since it's unused and brittle, drop it until a caller exists, or switch it to `rows_of`/`psql` which already parse reliably.</comment>

<file context>
@@ -0,0 +1,169 @@
+        yield tuple(cells)
+
+
+def scalar(sql):
+    return psql(f"SELECT ({sql})").strip().splitlines()[2].strip()
</file context>

"-o", out], check=True)
print(f" fetched {out} ({os.path.getsize(out)} bytes)")
# The negative landmark: no HTML at all, so only the detail is stored.
out = fname("DU/1997/483", ".detail.json")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The negative-landmark setup is dead code. fetch_all() downloads DU/1997/483/.detail.json, but no assertion ever reads it: the landmark check calls P.parse(None, b"") directly, which unconditionally returns NO_STRUCT as soon as the parser sees struct is None. The fetched file is never consumed, and the test never exercises the actual landmark behavior (struct 404 / zero-byte html) that the comment says it validates. The convenience: either have the test load the fetched detail/HTML and assert NO_STRUCT through the real data path, or drop the fetch.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/pl/test_pl_article_parser.py, line 65:

<comment>The negative-landmark setup is dead code. fetch_all() downloads DU/1997/483/.detail.json, but no assertion ever reads it: the landmark check calls P.parse(None, b"") directly, which unconditionally returns NO_STRUCT as soon as the parser sees struct is None. The fetched file is never consumed, and the test never exercises the actual landmark behavior (struct 404 / zero-byte html) that the comment says it validates. The convenience: either have the test load the fetched detail/HTML and assert NO_STRUCT through the real data path, or drop the fetch.</comment>

<file context>
@@ -0,0 +1,184 @@
+                            "-o", out], check=True)
+            print(f"  fetched {out} ({os.path.getsize(out)} bytes)")
+    # The negative landmark: no HTML at all, so only the detail is stored.
+    out = fname("DU/1997/483", ".detail.json")
+    subprocess.run(["curl", "-s", "-m", "60", f"{API}/acts/DU/1997/483", "-o", out],
+                   check=True)
</file context>

Comment thread scripts/pl/README.md

Parser: `pl_article_parser.py`, tests `test_pl_article_parser.py` (all passing).
Schema: `mcp_backend/src/migrations/190_pl_legislation.sql`, `191_pl_law_texts.sql`
(applied to prod 2026-08-14; 184/185 were already taken, hence 190/191),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The claim that migrations are 190/191 because "184/185 were already taken" is inaccurate: migrations 186 through 189 also exist in mcp_backend/src/migrations (184_v2_ua_opendata... through 189_edrnpa_number_norm_index). The numbering landed at 190/191 because every prior number through 189 was taken. Update the explanation so it does not mislead about which slots were occupied.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/pl/README.md, line 32:

<comment>The claim that migrations are 190/191 because "184/185 were already taken" is inaccurate: migrations 186 through 189 also exist in mcp_backend/src/migrations (184_v2_ua_opendata... through 189_edrnpa_number_norm_index). The numbering landed at 190/191 because every prior number through 189 was taken. Update the explanation so it does not mislead about which slots were occupied.</comment>

<file context>
@@ -0,0 +1,133 @@
+
+Parser: `pl_article_parser.py`, tests `test_pl_article_parser.py` (all passing).
+Schema: `mcp_backend/src/migrations/190_pl_legislation.sql`, `191_pl_law_texts.sql`
+(applied to prod 2026-08-14; 184/185 were already taken, hence 190/191),
+plus `192_pl_indexes_concurrently.sql` **which the migration runner must not run**
+(it wraps each file in one transaction and `CREATE INDEX CONCURRENTLY` cannot run
</file context>
Suggested change
(applied to prod 2026-08-14; 184/185 were already taken, hence 190/191),
(applied to prod 2026-08-14; 184-189 were already taken, hence 190/191),


_lock = Lock()
_last = [0.0]
counts = {"ok": 0, "no_text": 0, "failed": 0, "articles": 0}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The progress log always reports notext=0, hiding empty and no-HTML outcomes while they are counted as failures or omitted. Increment no_text for those verdicts or remove the unused metric.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/pl/harvest_eli_texts.py, line 70:

<comment>The progress log always reports `notext=0`, hiding empty and no-HTML outcomes while they are counted as failures or omitted. Increment `no_text` for those verdicts or remove the unused metric.</comment>

<file context>
@@ -0,0 +1,282 @@
+
+_lock = Lock()
+_last = [0.0]
+counts = {"ok": 0, "no_text": 0, "failed": 0, "articles": 0}
+started = time.time()
+
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant