Skip to content

feat: model-quality campaign — per-stage model routing + closed-loop targeted repair#42

Open
miguelgfierro wants to merge 42 commits into
mainfrom
demo
Open

feat: model-quality campaign — per-stage model routing + closed-loop targeted repair#42
miguelgfierro wants to merge 42 commits into
mainfrom
demo

Conversation

@miguelgfierro

Copy link
Copy Markdown

Integration PR for the demo branch: two composable features that raise extraction quality by putting the right model on the right task and closing the verification loop. Verified together on this branch (325 unit tests green, incl. the cross-feature orchestrator harness).

1. Per-stage model routing — #40

What it does. Every LLM pipeline stage (splitter, classifier, extract, visual/content authenticity, judge, rules, transform, bbox matcher) can be pinned to its own model via a FLYDOCS_<STAGE>_MODEL env var. Any stage left unset falls back to the request's options.model, then the global FLYDOCS_MODEL. A pinned stage wins over options.model, so operator stage-tuning survives per-request overrides. env_template ships a recommended tiering: sonnet as the floor for every stage, anthropic:claude-opus-4-8 for the judge.

Benefit. Cost and quality stop being one global trade-off. Mechanical stages no longer have to pay for the strongest model, while verification — where a stronger, different model catches errors the extractor is systematically blind to — gets exactly that. pipeline.usage.by_model already splits cost per model id, so the spend per tier is visible on every response with no extra work.

Details: see the full description in #40.

2. Closed-loop targeted repair — #41

What it does. A new opt-in repair stage (options.stages.repair) between judge and judge_escalation. Fields that failed the judge re-check or a deterministic validator are re-extracted in ONE focused pass whose prompt quotes each previous value and the concrete reason it was rejected (judge evidence + validator errors). Repaired candidates are re-verified, and replace the originals only under a monotonic accept guard: validators pass, an explicit judge PASS when the judge stage is on, and never a null value. The pass runs on its own FLYDOCS_REPAIR_MODEL (opus recommended) and reports a pipeline.repair audit block.

Benefit. The pipeline already detects bad values twice (judge verdicts with written evidence, validator errors) but previously never acted on that evidence field by field — the only recovery was judge_escalation, a blind whole-document re-run that re-rolls the dice on fields that were right. Targeted repair fixes the specific mistakes at a fraction of that cost, can only improve the result (never degrade it), and because it runs before escalation, the expensive full re-run only fires when targeted repair was not enough.

Details: see the full description (including the adversarial-review findings and fixes) in #41.

How they compose

The intended production policy becomes: default model extracts → strong model judges → strong model repairs only what failed — the quality of a second full pass at a fraction of its cost, with a full audit trail (pipeline.repair, pipeline.escalation, pipeline.usage.by_model).

One reconciliation commit on this branch adapts #40's orchestrator test harness to the field_repairer constructor parameter introduced by #41.

miguelgfierro and others added 29 commits July 9, 2026 13:11
…ccept guard, never erase to null, compact array evidence
feat: per-stage model routing for the extraction pipeline
feat: closed-loop targeted repair of judge/validator-failing fields
@casc84ab

casc84ab commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review — mechanism is solid, the challenge is on the default policy and repair cost

Both features are well designed and unusually well tested (the edge cases that normally slip through here — null-candidate never erasing, judge-omitted candidate staying UNCERTAIN, compact array evidence, validator-only path — are all covered). The direction is right. My challenge is not on the code but on (a) the env_template default policy and (b) the real cost of repair on page-heavy documents.

I verified the funnel claim directly: judge_escalator._count_failures() reads the current extracted_groups, and repair mutates them in place with fresh PASS verdicts before escalation runs — so "the expensive full re-run only fires when targeted repair was not enough" is true, not just narrative.

Feature 1 — per-stage model routing

Clean implementation, and usage.by_model is exactly the observability we needed to govern spend per tier.

The shipped env_template only spends the new lever upward. It sets sonnet as the floor for every stage (including mechanical ones) and opus for judge — so as delivered it raises cost rather than optimizing it:

Lever the PR enables Used by env_template?
Cheaper model on splitter / classifier (mechanical) ❌ pinned to sonnet
Cheaper model on bbox_matcher (our single largest cost bucket) ❌ pinned to sonnet
Stronger model on judge ✅ opus, enabled

Concrete numbers from our last PROD window (Jul 7-8, 1594 pages): main pipeline was $28.32 all-sonnet (judge ~$11.65) and bbox_refine was $33.30 (the biggest bucket). Pinning judge to opus (~5× per token) takes the judge to ~$55-58 on its own — roughly doubling the main-pipeline bill. Meanwhile FLYDOCS_BBOX_MATCHER_MODEL governs the LlmValueMatcher in bbox_refine, which is value matching against a region (not extraction) and a natural haiku candidate — ~$20-25/window left on the table.

Suggested tiering: splitter / classifier / bbox_matcher → haiku; extract → sonnet; judge → opus only after measuring the detection lift via by_model. That nets a cost reduction.

Also worth documenting for backend consumers: "pinned wins over options.model" means a per-request options.model=cheap cannot lower a pinned-opus judge. Defensible, but surprising.

Feature 2 — closed-loop targeted repair

Right technique, and the monotonic accept guard is careful work. Three things to address before a broad rollout:

  1. The "focused" pass does not reduce input, which is the dominant cost. extract_repair is called with document_bytes=task.slice_bytes — the full segment slice, all pages. The focus is on the schema subset, not on the pages. For multimodal docs the input (pages) dominates — that's exactly why per-page bbox_refine was the largest bucket. Repairing 2 fields in a 50-page scanned legajo re-sends all 50 pages, so "a fraction of the cost" holds vs escalation but is overstated for page-heavy docs. ExtractedField.page is available and slice_pdf/PageRange already exist — slicing to the failing fields' pages would make repair cheap for real.

  2. No cap on repair scope → worst case is more expensive than before. If most fields fail, the subset ≈ the whole document and repair degenerates into a full re-extract on opus plus a full judge re-check on opus, and escalation can still fire afterward. For an all-fail doc the new path costs more than the old judge→escalation path. A threshold ("if >X% of fields failed, skip targeted repair and go straight to escalation") removes that pathological case; targeted repair only pays off for a minority of failing fields.

  3. Monotonicity is weaker for array/table fields. Arrays are repaired whole: if one row's value fails, the entire array field is re-extracted and, if it passes validation, replaces the original array — including rows that were previously correct. The accept guard is at the array-field level, not the row level, so a previously-correct row can change and slip through if the change still passes validators. "Can only improve, never degrade" is not strictly true for tables — which is our bastanteo apoderados/socios case. Not a code bug, but a QA item: diff correct rows before/after on table fields.

Minor: flag_for_review PASS fields get sent to repair (often ambiguous-but-correct → cost leak, though the guard prevents degradation); repair_timeout_s=300 exceeds FLYDOCS_SYNC_TIMEOUT_S=60 so a sync caller enabling repair blows the sync budget; given the recent 429/OOM history I'd bound repair concurrency on rollout and watch by_model.

Suggested sequencing

Ship routing first (low risk, immediate observability/cost benefit) with the corrected tiering, then repair (opt-in, larger surface) with page-slicing and a scope threshold. Neither change touches the PR's architecture.

Flagged-but-PASS fields are often ambiguous-but-correct; repairing them
is spend the accept guard usually discards. FLYDOCS_REPAIR_INCLUDE_FLAGGED
(default true, preserving current behaviour) lets cost-sensitive
deployments limit repair to judge FAIL + validator errors.
Without a cap, an all-fail task degenerates into a full re-extract plus
a full judge re-check on the repair model, with escalation still able
to fire afterwards -- strictly costlier than judge->escalation alone.
Above the cap (default 0.5) repair now steps aside for that task and
leaves the FAIL verdicts for judge_escalation; RepairInfo.tasks_skipped
records it.
Repair fanned out one unbounded gather over all failing tasks;
FLYDOCS_REPAIR_TASK_CONCURRENCY (default 4) now caps in-flight repair
passes, mirroring bbox_refine_doc_concurrency for provider rate limits.
Arrays are re-extracted and accepted whole, so a previously-correct row
can be replaced by a new verifier-passing value -- monotonicity is
field-level only. rows_changed records how many rows differ per
repaired array so table repairs can be diffed in QA; the module
docstring no longer overstates the guarantee.
The focused repair trimmed the schema subset but re-sent every page of
the segment slice -- and pages, not fields, dominate multimodal input
cost. When every failing field carries page provenance and the media is
PDF, the repair (extract + judge re-check) now runs on a sub-slice
spanning the failing pages plus one page of margin, and accepted
candidates' slice-relative pages are remapped to segment coordinates.
Falls back to the full slice for non-PDF media, unknown pages (nulled
values lose theirs), non-shrinking spans, or slicing errors.
The repair pass eats into the hard FLYDOCS_SYNC_TIMEOUT_S wall (the
pipeline is cancelled at the ceiling with a deterministic 408), so
enabling stages.repair on the sync channel raises 408 likelihood.
RequestValidator.validate() gains a sync flag, set by the sync
controller and the /extract:validate dry-run; the async submit path is
unchanged.
…v_template

Splitter, classifier and the bbox value matcher are mechanical work and
run fine on haiku; extraction/reasoning stay on sonnet and the judge on
opus, so the per-stage lever cuts cost instead of only raising it. Also
state that a pinned stage wins over options.model, correct the repair
predicate wording (flag_for_review included), and document the three
new repair settings plus the sync-cap note.
…cing notes

payload-reference and deployment now state that a pinned
FLYDOCS_<STAGE>_MODEL wins over the request's options.model (previously
only a code comment). pipeline.md documents the repair page sub-slice,
the failing-fraction scope cap, the concurrency bound, the
include-flagged knob and the sync-latency warning.
@miguelgfierro

Copy link
Copy Markdown
Author

Thanks — every point actioned in #43 (feat/pr42-review-fixes → demo), which will flow into this PR once merged. No architecture changes.

Routing: env_template now nets a cost reduction — mechanical stages (splitter/classifier/bbox_matcher) → haiku, extract → sonnet, judge → opus documented as opt-in after measuring lift via by_model. Stage-pin-wins-over-options.model documented.

Repair: (2.1) page-slices to the failing fields' pages via ExtractedField.page so a 2-field fix in a 50-page legajo no longer re-sends 50 pages; (2.2) repair_max_failing_fraction skips targeted repair and goes straight to escalation past a threshold, killing the >X%-fail pathological case; (2.3) RepairInfo.rows_changed surfaces whole-array row changes (bastanteo apoderados/socios QA case). Plus repair_include_flagged (keep ambiguous PASS fields out), repair_sync_latency warning (300s repair vs 60s sync budget), and repair_task_concurrency for the 429/OOM history.

336 unit tests green, ruff clean.

@miguelgfierro miguelgfierro marked this pull request as ready for review July 9, 2026 17:02
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.

2 participants