docs(plans): close wave 3, record three retargets and two new tickets - #201
Conversation
Wave 3 built one ticket and sent three back. A premise check was added to the dispatch harness for this wave and caught all three on its first run. - GH-152 A1: the ticket blamed the rowizer. Measurement shows the page-wide text-strategy find_tables at reconstruct.py:141 merges the tables first and suppresses the fallback, so wiring only the rowizer is a measured end-to-end no-op. A1 becomes a detector; A2 is recut to consume it at both rungs. - GH-150 B2: the defect still reproduces (both PDFs copied, hashed, measured through the installed package), but the fix is a merge inside born_digital.py, which B2 does not own. B2 becomes fixtures plus a strict xfail pin; new TICKET-C1 owns the fix and is blocked on PR #200 for that file. - GH-147 B1: the metric is invalidated by GH-147 A2's own fix. A2 sets native_text = raw_text.strip() on refused pages, so word recall there is ~1.0 by construction and the 20/40 figure cannot survive a correct fix. Retargeted onto refusal rate plus a structural witness. Also records: ownership amendments (structure_check.py released, manifest.py added, born_digital.py contended between B1/C1/A2); that waves 4 and 5 were authored the same day and should be assumed stale until checked; and three process notes -- measure at the caller, a green suite is not a guard, and the corpus spans two locations of which neither is complete alone.
|
Warning Review limit reached
Next review available in: 104 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR |
PR Summary by QodoClose Wave 3 docs; retarget three tickets and add C1 + updated A2
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 4 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="docs/plans/extraction-defects/STATUS.md">
<violation number="1" location="docs/plans/extraction-defects/STATUS.md:264">
P2: This ownership amendment conflicts with the file-ownership schedule above, which still marks `structure_check.py` as wave-1-only. Update the ownership and wave tables to keep one authoritative dispatch mapping.</violation>
<violation number="2" location="docs/plans/extraction-defects/STATUS.md:265">
P2: This adds a new owned file without adding it to the ownership/wave schedule. Add `src/socr/core/manifest.py` to the file-ownership table with its serialized wave placement so concurrency rules stay enforceable.</violation>
<violation number="3" location="docs/plans/extraction-defects/STATUS.md:295">
P2: This new wave-4 sequencing rule contradicts the current Wave 4 table. Update the wave table and write sets to include GH-150 C1 and the `born_digital.py` contention so dispatch instructions are consistent.</violation>
</file>
<file name="docs/plans/gh152-side-by-side-tables/TICKETS.md">
<violation number="1" location="docs/plans/gh152-side-by-side-tables/TICKETS.md:62">
P2: `TICKET-A2` is now in wave 4, but `TICKET-B1` still depends on A2 in wave 3, so the plan cannot be dispatched in order. Move one ticket so dependency and wave order agree.</violation>
</file>
<file name="docs/plans/gh147-landscape-pages/TICKETS.md">
<violation number="1" location="docs/plans/gh147-landscape-pages/TICKETS.md:88">
P3: The acceptance-revert step won't revert A2. `git stash push -- <path>` only discards uncommitted working-tree changes, but A2's refusal fix is already merged (this ticket says `merged 13033a3`, and the code lives at `born_digital.py:915-931`), so the stash is empty and the rotated after-arm is never run against a reverted A2 — the "must FAIL" verification is vacuous. Use `git revert 13033a3` (or temporarily `git checkout 13033a3^ -- src/socr/core/born_digital.py`) to actually exercise the pre-fix state, or rewrite the step to describe reverting the merge rather than stashing.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| ### Ownership amendments | ||
|
|
||
| - `src/socr/tables/structure_check.py` — released from wave-1-only; GH-151 B1 extends it. | ||
| - `src/socr/core/manifest.py` — **added**, claimed by GH-151 B1. A flag the manifest does not |
There was a problem hiding this comment.
P2: This adds a new owned file without adding it to the ownership/wave schedule. Add src/socr/core/manifest.py to the file-ownership table with its serialized wave placement so concurrency rules stay enforceable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/plans/extraction-defects/STATUS.md, line 265:
<comment>This adds a new owned file without adding it to the ownership/wave schedule. Add `src/socr/core/manifest.py` to the file-ownership table with its serialized wave placement so concurrency rules stay enforceable.</comment>
<file context>
@@ -224,15 +224,75 @@ load-bearing rather than vacuous.
+### Ownership amendments
+
+- `src/socr/tables/structure_check.py` — released from wave-1-only; GH-151 B1 extends it.
+- `src/socr/core/manifest.py` — **added**, claimed by GH-151 B1. A flag the manifest does not
+ read re-stamps `audit_passed=True` (the PP-7-R1 bug shape), which would make the gate inert.
+- `src/socr/core/born_digital.py` — contended. GH-151 B1 holds it (PR #200); **GH-150 C1 needs
</file context>
| measurement: GH-152 A1 (`tables/reconstruct.py`), GH-150 B2 (`tests/` + `fixtures/` + `logs/`), | ||
| GH-147 B1 (`tests/` + `fixtures/` + `logs/`). Write sets are disjoint. | ||
|
|
||
| **Then wave 4, gated on PR #200 merging:** GH-150 C1 and GH-152 A2 both want |
There was a problem hiding this comment.
P2: This new wave-4 sequencing rule contradicts the current Wave 4 table. Update the wave table and write sets to include GH-150 C1 and the born_digital.py contention so dispatch instructions are consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/plans/extraction-defects/STATUS.md, line 295:
<comment>This new wave-4 sequencing rule contradicts the current Wave 4 table. Update the wave table and write sets to include GH-150 C1 and the `born_digital.py` contention so dispatch instructions are consistent.</comment>
<file context>
@@ -224,15 +224,75 @@ load-bearing rather than vacuous.
+measurement: GH-152 A1 (`tables/reconstruct.py`), GH-150 B2 (`tests/` + `fixtures/` + `logs/`),
+GH-147 B1 (`tests/` + `fixtures/` + `logs/`). Write sets are disjoint.
+
+**Then wave 4, gated on PR #200 merging:** GH-150 C1 and GH-152 A2 both want
+`born_digital.py`, so they serialize behind #200 and behind each other.
</file context>
|
|
||
| ### Ownership amendments | ||
|
|
||
| - `src/socr/tables/structure_check.py` — released from wave-1-only; GH-151 B1 extends it. |
There was a problem hiding this comment.
P2: This ownership amendment conflicts with the file-ownership schedule above, which still marks structure_check.py as wave-1-only. Update the ownership and wave tables to keep one authoritative dispatch mapping.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/plans/extraction-defects/STATUS.md, line 264:
<comment>This ownership amendment conflicts with the file-ownership schedule above, which still marks `structure_check.py` as wave-1-only. Update the ownership and wave tables to keep one authoritative dispatch mapping.</comment>
<file context>
@@ -224,15 +224,75 @@ load-bearing rather than vacuous.
+
+### Ownership amendments
+
+- `src/socr/tables/structure_check.py` — released from wave-1-only; GH-151 B1 extends it.
+- `src/socr/core/manifest.py` — **added**, claimed by GH-151 B1. A flag the manifest does not
+ read re-stamps `audit_passed=True` (the PP-7-R1 bug shape), which would make the gate inert.
</file context>
| this way.) | ||
|
|
||
| ### TICKET-A2 — rowize each band independently, in reading order · TODO · depends-on: A1 · wave 2 | ||
| ### TICKET-A2 — consume the helper at BOTH merging rungs · TODO · depends-on: A1 · wave 4 |
There was a problem hiding this comment.
P2: TICKET-A2 is now in wave 4, but TICKET-B1 still depends on A2 in wave 3, so the plan cannot be dispatched in order. Move one ticket so dependency and wave order agree.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/plans/gh152-side-by-side-tables/TICKETS.md, line 62:
<comment>`TICKET-A2` is now in wave 4, but `TICKET-B1` still depends on A2 in wave 3, so the plan cannot be dispatched in order. Move one ticket so dependency and wave order agree.</comment>
<file context>
@@ -14,20 +14,72 @@ until both have landed.
+this way.)
-### TICKET-A2 — rowize each band independently, in reading order · TODO · depends-on: A1 · wave 2
+### TICKET-A2 — consume the helper at BOTH merging rungs · TODO · depends-on: A1 · wave 4
**Problem:** Two tables need two grids, emitted left-to-right then top-to-bottom.
-**Do:** Rowize each band separately and order the resulting regions by (band, y0).
</file context>
| **Done when:** a dated log records the refused-page count and rate, the opened-PDF count, the | ||
| per-file manifest, the named zero-byte exclusions, and the below-80% distribution labelled as | ||
| detector output. **Acceptance is not a green suite:** revert A2 with | ||
| `git stash push -- src/socr/core/born_digital.py` and the rotated after-arm (no separator / |
There was a problem hiding this comment.
P3: The acceptance-revert step won't revert A2. git stash push -- <path> only discards uncommitted working-tree changes, but A2's refusal fix is already merged (this ticket says merged 13033a3, and the code lives at born_digital.py:915-931), so the stash is empty and the rotated after-arm is never run against a reverted A2 — the "must FAIL" verification is vacuous. Use git revert 13033a3 (or temporarily git checkout 13033a3^ -- src/socr/core/born_digital.py) to actually exercise the pre-fix state, or rewrite the step to describe reverting the merge rather than stashing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/plans/gh147-landscape-pages/TICKETS.md, line 88:
<comment>The acceptance-revert step won't revert A2. `git stash push -- <path>` only discards uncommitted working-tree changes, but A2's refusal fix is already merged (this ticket says `merged 13033a3`, and the code lives at `born_digital.py:915-931`), so the stash is empty and the rotated after-arm is never run against a reverted A2 — the "must FAIL" verification is vacuous. Use `git revert 13033a3` (or temporarily `git checkout 13033a3^ -- src/socr/core/born_digital.py`) to actually exercise the pre-fix state, or rewrite the step to describe reverting the merge rather than stashing.</comment>
<file context>
@@ -30,10 +30,71 @@ Prose on such pages should still be retained.
+**Done when:** a dated log records the refused-page count and rate, the opened-PDF count, the
+per-file manifest, the named zero-byte exclusions, and the below-80% distribution labelled as
+detector output. **Acceptance is not a green suite:** revert A2 with
+`git stash push -- src/socr/core/born_digital.py` and the rotated after-arm (no separator /
+`native_table_lane_refused` / no shipped grid) must **FAIL**, while the upright control passes
+in both states.
</file context>
r-uben
left a comment
r-uben
left a comment
There was a problem hiding this comment.
Independent review of head 528b3b5 (docs-only). Retargets check out against current main; line citations are exact (reconstruct.py:141, born_digital.py:915-931 / :1169-1192 / :1201, snap at :1348-1354). GH-152 A1 is detector-only; GH-150 B2 is fixtures+strict xfail with C1 owning the born_digital.py merge; GH-147 B1 correctly drops the word-recall / 20/40 criterion after A2 (13033a3 / #193). Process notes (measure at extract_structured/process(); tests must fail on revert) match the Wave 2 #192 false alarm and #200’s review round.
Non-blocking, but please fix before dispatchers treat this board as live:
-
#200 is still open (
mergeable_state: unstable). Calling GH-151 B1 BUILT and Wave 3 CLOSED is true as “PR exists”, not as “accepted on main”. C1 must stay blocked until #200 merges — that part is right. #200 already edits this sameSTATUS.mdownership table (different hunks); C1 is not in that table on either branch. -
Live tables vs amendments. File-ownership + Wave 3/4 grids still describe the original wave. Amendments/Next action add C1 and 3b in prose only. Wave 4 “C1 and A2 both want
born_digital.py” overstates A2: A2’s Files makeborn_digital.pyconditional on LTR remaining inDone when, and the recutDone whendoes not require LTR emission. Pick one before staffing wave 4. -
Small factual nits. “Ticket detail … is unchanged” is now false. GH-147 B1 “
reconstruct.pymoves under this ticket” should say GH-152. Header stillLast updated: 2026-08-12. GH-147 folderSTATUS.md20/40 left stale (you flagged it).
CI is green. I would not block merge on (3); I would not dispatch 3b/4 from the unpatched tables.
…cking lesson (#210) Two things #201 could not know when it was written: - fake-native B1 (#209) is a fourth claimant on born_digital.py, behind #200, GH-150 C1 and GH-152 A2. None of it parallelises; settling #200's direction is what unsticks all four. Named as the single bottleneck. - the fake-native plan was opened as an interrupt claiming #200 could not be decided until it landed. Measurement put the population at 2.4% and the TR-3 overlap at 6/68 - it blocked nothing. Recorded as a process note: the measurement was cheap and right to run, the blocking claim attached to it was not.
1 participant
Summary
Wave 3 built one ticket and sent three back. This records why, rewrites the three retargeted tickets against their measured scopes, adds the two tickets the rulings called for, and closes the wave on the coordinator board.
Scope: four plan documents. No code.
The headline is not the code
A premise check was added to the dispatch harness for this wave — each ticket's stated problem had to be re-verified against current
mainbefore any design was accepted, with aretargetverdict available to the ruling stage. It fired on three of four tickets on its first run.Why each was retargeted
GH-152 A1 — the ticket blamed the rowizer's y-only clustering. Measurement showed the page-wide text-strategy
find_tablesatreconstruct.py:141merges the two tables first and then suppresses the fallback that would have coped (born_digital.py:1169-1192short-circuits on a non-emptytable_regions). Wiring only the rowizer is a measured end-to-end no-op on the aligned fixture. A1 becomes a detector wired into nothing; A2 is recut to consume it at both merging rungs, and to fix the rowizer snap-radius label drop atreconstruct.py:1348-1354as a second, distinct defect.GH-150 B2 — the defect still reproduces: both PDFs were copied to
/tmp, hashed (Heston 449,243 B /2ff4173d…; Drechsler 711,917 B /d9eba00c…) and measured through the installed package, and both target pages still carry a pipe separator with no image reference. But the production fix is a merge of chart placeholders with non-emptyfind_tables()regions, insidesrc/socr/core/born_digital.py— a file B2 does not own. B2 becomes fixtures, provider-free tests and a strict xfail pin; new TICKET-C1 owns the fix and is blocked on #200 for that file. Emptyingfind_tables()is a labelled simulation, not the after-state.GH-147 B1 — the sharpest one. The ticket measures landscape damage by below-80% word recall, but GH-147 A2 (merged
13033a3) never callsextract_structuredon a refused page: it setsnative_text = raw_text.strip()(born_digital.py:915-931). Word recall there is therefore ~1.0 by construction, and the 20/40 figure cannot survive a correct fix. The old criterion would have failed the ticket for succeeding. Retargeted onto refusal rate undernative_table_lane_refusedplus a structural witness (paired rotated/upright fixtures with ground truth), with acceptance proved by reverting A2 and requiring the after-arm to fail.Ownership amendments
structure_check.py— released from wave-1-only; bug(tables): a table can ship at 100% word recall with its structure destroyed — recall is not a sufficient gate #151 B1 extends it.manifest.py— added, claimed by bug(tables): a table can ship at 100% word recall with its structure destroyed — recall is not a sufficient gate #151 B1. A flag the manifest does not read re-stampsaudit_passed=True(the PP-7-R1 shape) and would make the gate inert.born_digital.py— contended three ways. bug(tables): a table can ship at 100% word recall with its structure destroyed — recall is not a sufficient gate #151 B1 holds it (feat(151): gate native table ship on structural grid defects (GH-151 B1) #200); bug(figures): figures are extracted as tables — the two worst pages in the corpus are charts #150 C1 needs it next; bug(tables): two side-by-side tables are merged into one region and flattened #152 A2 may need it if reading order stays in itsDone when, sinceborn_digital.py:1201re-sorts byy0alone. These serialize.Assumption worth stating plainly
Three of four is not bad luck. This plan folder was authored against a codebase that has since moved twice. Waves 4 and 5 were written the same day and should be assumed stale until checked. A retarget is the expected output of a premise check, not a failure — and the check belongs before staffing, not after.
Process notes recorded on the board
extract_structuredorprocess().--native-onlyruling and the suite still passed. The follow-up round then found the replacement test could not distinguish the correct fix from a simpler wrong one. Each new test must be shown to fail when its production line is reverted.Test plan
Documentation only; no code paths touched. Four files changed.
Summary by cubic
Closes wave 3 planning: documents one build and three retargets, adds two follow-up tickets, and updates ownership and process guidance; no code changes. This prevents dispatching against stale premises and sets serialization for upcoming work in
born_digital.py.Reviewer notes
reconstruct.py.xfail; new TICKET-C1 owns the merge of chart placeholders with non-empty tables inborn_digital.pyand is blocked on PR feat(151): gate native table ship on structural grid defects (GH-151 B1) #200.structure_check.pyreleased;manifest.pyadded with an audit-stamping risk note;born_digital.pycontended and serialized behind PR feat(151): gate native table ship on structural grid defects (GH-151 B1) #200.BornDigitalDetector().extract_structuredorprocess()); require tests to fail on revert; copy corpus PDFs to/tmpand avoid Google Drive.Rollout
born_digital.py.Written for commit 528b3b5. Summary will update on new commits.