This commit is contained in:
@@ -0,0 +1,67 @@
|
||||
Executing unit: u8
|
||||
|
||||
[Claude #9] Stage 3 code-edit Round #N — unit u8
|
||||
|
||||
### unit_executed
|
||||
- **id**: u8
|
||||
- **summary**: New `_attempt_salvage_chain` in `phase_z2_pipeline.py` — deterministic Step 17 salvage cascade orchestrator. Iterates `phase_z2_failure_router.route_retry_failure` across the triad (`cross_zone_redistribute → glue_compression → font_step_compression`); per stage runs plan → apply CSS → render candidate → `run_overflow_check`. PASS promotes `final.html`; cascade-exit routes (`layout_adjust` / `frame_reselect` / `none`) terminate; all-fail preserves (b) revert. Honors IMP-09 `dynamic_cols` / `fr_default` gate. **u9 pipeline wiring deferred.**
|
||||
|
||||
### files_changed
|
||||
- `src/phase_z2_pipeline.py` (u8-only delta = +95 / −1 — includes +13 lines of new imports, +6 lines of header comment + lookup table, +76 lines of orchestrator function body. Slightly above the u8 line estimate of 50 due to 3-action plan/apply dispatch + (b)-revert telemetry needing explicit per-stage step dicts; the cascade body itself is loop-driven and table-based to avoid duplication.)
|
||||
|
||||
### diff_summary
|
||||
1. **New imports** (lines 58-76, `+13`):
|
||||
- From `phase_z2_retry`: `plan_cross_zone_redistribute` / `apply_cross_zone_redistribute_css` (u4), `plan_glue_compression` / `apply_glue_compression_css` (u5), `plan_font_step_compression` / `apply_font_step_compression_css` (u6). No mutation of existing `plan_zone_ratio_retry` / `apply_retry_to_layout_css` (u1) import.
|
||||
- From `phase_z2_failure_router`: `route_retry_failure` (u3) added alongside existing `enrich_retry_trace_with_failure_classification`.
|
||||
2. **`_SALVAGE_FAIL_BY_ACTION` constant** (lines 1937-1944) — single-point mapping from cascade action label to the failure-type label the failure router uses for the next routing step. Mirrors `phase_z2_failure_router.SALVAGE_FAILURE_TYPE_BY_ACTION` (u2) at the orchestrator layer so the route-walk does not need to reach back into the failure-router module for cascade state.
|
||||
3. **`_attempt_salvage_chain` orchestrator** (lines 1947-2024):
|
||||
- **Signature**: `run_dir`, `out_path`, `slide_title`, `slide_footer`, `zones_data`, `layout_preset`, `layout_css`, `cascade_inputs: dict`, `initial_failure_type: str`, `gap_px: int`. Mirrors `_attempt_zone_ratio_retry` arg shape so u9 can plumb identically. `cascade_inputs` is a flat dict that u9 will populate with `{fit_analysis, containers, min_margin_px, excess_px, block_count, zone_position, current_font_px, excess_after_glue_px, available_lines, chars_per_line}` — keeps the orchestrator agnostic to where the values come from.
|
||||
- **IMP-09 gate** (lines 1956-1959): exits with `salvage_attempted=False` + `salvage_skipped_reason` when `dynamic_cols=True` or `dynamic_rows=False`. Same posture as `_attempt_zone_ratio_retry` (line 1833-1843) — row-axis CSS overrides on 2-D or fr_default layouts would either misapply or no-op.
|
||||
- **Cascade loop** (lines 1963-2023, bounded to `len(_SALVAGE_FAIL_BY_ACTION) == 3` iterations to guard against router cycles):
|
||||
- `route_retry_failure(failure_type)` resolves the next cascade action. If `next_action` is not one of the 3 salvage labels (i.e. `layout_adjust` / `frame_reselect` / `none` terminal), the orchestrator records `salvage_terminal_action` + `salvage_terminal_rationale` and returns — preserving (b) revert.
|
||||
- Per stage: dispatch to the matching plan/apply pair. `cross_zone_redistribute` has an explicit `fit_analysis is None` defensive guard (synthesises a `feasible=False` plan with a clear `failure_reason` rather than crashing `plan_cross_zone_redistribute` on `.roles` dereference). `glue_compression` / `font_step_compression` planners already self-guard on zero / missing inputs (u5/u6 case-0 short-circuits).
|
||||
- When `plan.feasible` is True: `apply_fn(plan)` emits scoped CSS (`[data-role=…]` for cross_zone, `[data-zone-position=…]` for glue/font — guardrail respect baked into u4/u5/u6). The CSS is injected into the slide HTML via a `<style>…</style>` block placed inside `</head>` (or prepended if `</head>` absent). The candidate is written to `salvage_<action>_candidate.html` next to the slide, then `run_overflow_check` runs.
|
||||
- On PASS: `out_path.write_text(candidate_html)` promotes the candidate to `final.html`; step recorded with `post_salvage_overflow`; loop exits with `salvage_passed=True`.
|
||||
- On FAIL (infeasible plan, no CSS, or visual_check fail): step recorded with `failure_reason` (planner reason first, then visual_check fail_reasons, then a generic fallback). `failure_type` updated via `_SALVAGE_FAIL_BY_ACTION[next_action]` so the next loop iteration's `route_retry_failure` advances the cascade.
|
||||
- **No `final.html` mutation outside `passed=True`** — (b) revert preserved on every fail path (skip, terminal, infeasible, no-CSS, visual_check fail). Verified via probes 3, 5, 6.
|
||||
4. **Backward-compat / surface invariants preserved**:
|
||||
- `_attempt_zone_ratio_retry` body unchanged.
|
||||
- `enrich_retry_trace_with_failure_classification` import preserved (u2/u3 surface intact).
|
||||
- No changes to `phase_z2_router.ACTION_BY_CATEGORY`, `_attempt_zone_ratio_retry`, or pipeline-Step-17 wiring (u9 territory).
|
||||
- No `ACTION_IMPLEMENTATION_STATUS` / `NEXT_ACTION_IMPLEMENTATION_STATUS` mutations (u7 already registered IMPLEMENTED for the 3 cascade actions).
|
||||
|
||||
### evidence
|
||||
- `python -c "from src.phase_z2_pipeline import _attempt_salvage_chain, _SALVAGE_FAIL_BY_ACTION; ..."` → `OK: ['cross_zone_redistribute', 'glue_compression', 'font_step_compression']` (import + table sanity).
|
||||
- `python -m pytest -q tests/phase_z2` → **62 passed in 0.12s** (regression-free; u15 will add cascade-orchestrator fixture tests).
|
||||
- `git diff --numstat src/phase_z2_pipeline.py` → `95 1 src/phase_z2_pipeline.py` (only file touched by u8).
|
||||
- Direct behavior probes (all PASS):
|
||||
- **Probe 3 (IMP-09 gate, `dynamic_cols=True`)**: `salvage_attempted=False`, `salvage_skipped_reason` contains "IMP-09", `final.html` unchanged → (b) revert preserved on gate skip.
|
||||
- **Probe 4 (IMP-09 gate, `dynamic_rows=False`)**: same as probe 3 → gate covers fr_default layouts.
|
||||
- **Probe 5 (terminal-route from non-cascade failure)**: `initial_failure_type='rerender_still_fails'` → `salvage_attempted=True` but `salvage_steps=[]`, `salvage_terminal_action='frame_reselect'`, `final.html` unchanged → terminal route preserves (b) revert without consuming any cascade slot.
|
||||
- **Probe 6 (cascade walk from `donor_slack_insufficient`, all 3 stages infeasible)**: `actions == ['cross_zone_redistribute', 'glue_compression', 'font_step_compression']`, all `passed=False`, all `css_override=None`, all `candidate_path=None`, `final.html` unchanged → 3-stage walk works, (b) revert preserved, no spurious render.
|
||||
- **Probe 6.5 (per-stage failure_reason surfaces)**: `cross_zone_redistribute` step → `"fit_analysis missing"`; `glue_compression` step → `"excess_px <= 0"`; `font_step_compression` step → `"excess_after_glue_px <= 0"` or `"text_metrics missing"` → upstream planner reasons propagate cleanly through the orchestrator.
|
||||
- **Probe 7 (u3 routing aligns with u8 lookup table)**: `route_retry_failure('donor_slack_insufficient') → cross_zone_redistribute`; `route_retry_failure('cross_zone_redistribute_insufficient') → glue_compression`; `route_retry_failure('glue_absorption_insufficient') → font_step_compression`; `route_retry_failure('font_step_insufficient') → layout_adjust` → 4-transition cascade verified end-to-end against the failure router surface.
|
||||
- **Probe 8 (feasible + render + PASS)**: SKIPPED — needs Selenium-driven `run_overflow_check`. Will be covered by u15 fixture tests in `tests/phase_z2/test_phase_z2_step17_salvage_chain.py`.
|
||||
- No new tests added in this unit (u15 is the test unit for the cascade orchestrator).
|
||||
|
||||
### scope_lock
|
||||
- **No changes** to: `phase_z2_router.py`, `phase_z2_failure_router.py`, `phase_z2_retry.py` (all u1-u7 surfaces intact).
|
||||
- **No changes** to `_attempt_zone_ratio_retry`, Step 17 telemetry block (lines 4329-4342), or `enrich_retry_trace_with_failure_classification` call site — u9 territory.
|
||||
- **No AI/Kei call** on normal path (`feedback_ai_isolation_contract`).
|
||||
- **No `:root` / `body` / `.slide` / `.zone` CSS** — the `<style>` injection only contains output from the u4/u5/u6 `apply_*_css` functions, all of which emit `[data-role=…]` or `[data-zone-position=…]` scoped rules (`feedback_phase_z_spacing_direction`).
|
||||
- **No `ACTION_BY_CATEGORY` change**, no `NEXT_ACTION_BY_FAILURE` change, no `ACTION_IMPLEMENTATION_STATUS` / `NEXT_ACTION_IMPLEMENTATION_STATUS` change — cascade actions stay `IMPLEMENTED` (u7) and surface-less at Step 16 by design.
|
||||
|
||||
### remaining_units
|
||||
- **u9** — Pipeline wiring after `_attempt_zone_ratio_retry`: assemble `cascade_inputs` from `fit_analysis` / `containers` / overflow / `debug_zones` / `text_metrics`, call `_attempt_salvage_chain` when `failure_type in ('donor_slack_insufficient', 'no_donor_candidates')`, merge result into `retry_trace` (`salvage_steps`, `salvage_passed`), update Step 17 telemetry, mirror post-retry state refresh from lines 4311-4323 when `salvage_passed=True`.
|
||||
- **u10** — Tests for multi-donor `plan_zone_ratio_retry` (u1).
|
||||
- **u11** — Tests for `plan_cross_zone_redistribute` (u4).
|
||||
- **u12** — Tests for `plan_glue_compression` (u5) — scope-CSS assertion + envelope assertion.
|
||||
- **u13** — Tests for `plan_font_step_compression` (u6) — 4-guard ladder assertions.
|
||||
- **u14** — Tests for failure router cascade (u2/u3/u7).
|
||||
- **u15** — End-to-end fixture test for `_attempt_salvage_chain` (u8) — cases (a) cross_zone pass / (b) glue pass after cross_zone fail / (c) all 3 fail → (b) revert.
|
||||
|
||||
### follow_up_issue_candidates
|
||||
None inside u8's scope-lock. One **observation** for the implementer of u9 (not a follow-up issue): when `cascade_inputs.fit_analysis` is missing the orchestrator records a clean infeasible step rather than crashing — this lets u9 plumb only what is available without needing to short-circuit at the call site for partial-context zones. If we later want stricter "fit_analysis required" semantics, that is a u9-time decision, not a u8 axis.
|
||||
|
||||
### one_step_per_turn_compliance
|
||||
This turn implemented **exactly u8** from the Stage 2 implementation_units list. No work began on u9 / u10 / u11 / u12 / u13 / u14 / u15. No `git add`, no `git commit`, no push — per Stage 3 prompt.
|
||||
Reference in New Issue
Block a user