diff --git a/Front/client/src/pages/Home.tsx b/Front/client/src/pages/Home.tsx index 5b564ae..2579c97 100644 --- a/Front/client/src/pages/Home.tsx +++ b/Front/client/src/pages/Home.tsx @@ -170,6 +170,16 @@ export default function Home() { const key = deriveUserOverridesKey(p.uploadedFile.name); void saveUserOverrides(key, { zone_geometries: null }); } + // IMP-55 (#93) u12 — persist the marker reset to disk so a stale + // `manual_section_assignment: true` from a prior drag (written via + // u6's co-PUT) cannot survive the layout apply. The in-memory reset + // on line 192 protects the current session, but a page reload would + // re-seed from disk via u3's restore branch and re-arm the u7 gate. + // Unconditional — apply always resets, independent of hadPriorGeoms. + if (p.uploadedFile) { + const key = deriveUserOverridesKey(p.uploadedFile.name); + void saveUserOverrides(key, { manual_section_assignment: false }); + } return { ...p, userSelection: { @@ -179,6 +189,17 @@ export default function Home() { layout_preset: layoutId, zone_sections: carriedZoneSections, zone_geometries: {}, + // IMP-55 (#93) u5 — reset the bool intent marker to `false` on + // layout apply. `carriedZoneSections` above is auto-carry (old + // zone.section_ids → new layout positions), NOT user drag-drop + // intent. Without this explicit reset the spread of + // `...p.userSelection.overrides` would carry a prior-drag `true` + // into the new layout, causing handleGenerate (u7) to forward + // auto-carried assignments as user overrides and re-trigger the + // PARTIAL_COVERAGE regression. The marker flips back to `true` + // only when the user actually drag-drops a section in the new + // layout (u6 handleSectionDrop). + manual_section_assignment: false, }, selectedZoneId: null, selectedRegionId: null, @@ -193,10 +214,27 @@ export default function Home() { // pending 모드 취소 → 평소 (final.html iframe) 모드 복귀. const handleCancelPendingLayout = useCallback(() => { setPendingLayout(null); - setState((p) => ({ - ...p, - userSelection: createInitialUserSelection(p.slidePlan), - })); + setState((p) => { + // IMP-55 (#93) u12 — persist marker=false to disk on cancel. In-memory + // the u3 seed via createInitialUserSelection already pins false (u5 + // contract), but if a prior drag-drop wrote `true` to disk via u6's + // co-PUT, that value would survive a reopen and re-arm the u7 + // forwarding gate on the next page load. Symmetric with the apply + // path's disk PUT above. + if (p.uploadedFile) { + const key = deriveUserOverridesKey(p.uploadedFile.name); + void saveUserOverrides(key, { manual_section_assignment: false }); + } + return { + ...p, + // IMP-55 (#93) u5 — cancel discards all pending overrides via + // `createInitialUserSelection`, whose u3 seed pins + // `manual_section_assignment: false`. In-memory reset is implicit + // via the seed; u12 adds the disk-side PUT above to keep persisted + // state consistent so a reopen does not re-arm the marker. + userSelection: createInitialUserSelection(p.slidePlan), + }; + }); setHasPendingChanges(false); }, []); @@ -354,29 +392,45 @@ export default function Home() { } } - // 2026-05-22 — IMP-08 B-3 원래 동작 (sameAsDefault with effectiveSlidePlan) 복귀. - // 시연 안정성 우선. section swap 은 별 path (수동 drag detection) 로 풀어야 함. - // 임시 over-aggressive fix 가 default flow 깨뜨려 PARTIAL_COVERAGE 발생했음. - const userZoneSections = state.userSelection.overrides.zone_sections; - if (userZoneSections) { - const defaultByZone = new Map(); - sourcePlan.zones.forEach((z) => { - defaultByZone.set(z.zone_id, z.section_ids); - }); - const zoneSectionsDiff: Record = {}; - for (const [zoneId, sids] of Object.entries(userZoneSections)) { - if (!Array.isArray(sids)) continue; - const cleaned = sids.filter((s) => typeof s === "string" && s.trim()); - const defaults = defaultByZone.get(zoneId) ?? []; - const sameAsDefault = - cleaned.length === defaults.length && - cleaned.every((sid, i) => sid === defaults[i]); - if (!sameAsDefault) { - zoneSectionsDiff[zoneId] = cleaned; + // IMP-55 (#93) u7 — Replace the IMP-08 B-3 self-compare with the bool + // `manual_section_assignment` intent marker gate. The prior code built + // `defaultByZone` from `sourcePlan.zones` and compared against the + // user's `overrides.zone_sections`, but `sourcePlan === effectiveSlidePlan` + // (Home.tsx:305) and `effectiveSlidePlan.zones === pendingZones` + // (Home.tsx:649), which is itself derived from + // `state.userSelection.overrides.zone_sections` via slidePlanUtils.ts. + // The comparison was degenerate (user input vs itself), so real drag-drop + // swaps were classified `sameAsDefault` and silently dropped from + // `overrides.zoneSections` — the exact regression IMP-55 fixes. + // - true → forward `zone_sections` filtered to zone_ids that exist in + // `sourcePlan.zones` (cross-layout safety so foreign zone keys from a + // stale persisted layout never reach backend `--override-section- + // assignment`). u6 is the SOLE setter of true (real drag-drop). + // - false → skip. Backend determines assignment from its own default + // policy. u3 seeds false on first load, u5 resets false on layout + // apply auto-carry, u12 persists false so a stale disk `true` cannot + // survive a reopen-after-apply window. + // No `sameAsDefault` heuristic — the marker is the source of intent. + const manualMarker = + state.userSelection.overrides.manual_section_assignment; + if (manualMarker === true) { + const userZoneSections = state.userSelection.overrides.zone_sections; + if (userZoneSections) { + const validZoneIds = new Set( + sourcePlan.zones.map((z) => z.zone_id), + ); + const zoneSectionsForward: Record = {}; + for (const [zoneId, sids] of Object.entries(userZoneSections)) { + if (!validZoneIds.has(zoneId)) continue; + if (!Array.isArray(sids)) continue; + const cleaned = sids.filter( + (s) => typeof s === "string" && s.trim(), + ); + zoneSectionsForward[zoneId] = cleaned; + } + if (Object.keys(zoneSectionsForward).length > 0) { + overrides.zoneSections = zoneSectionsForward; } - } - if (Object.keys(zoneSectionsDiff).length > 0) { - overrides.zoneSections = zoneSectionsDiff; } } } @@ -478,7 +532,21 @@ export default function Home() { const handleSectionDrop = useCallback((sectionId: string, zoneId: string) => { setState((p) => { const newSelection = moveSectionToZone(p.userSelection, sectionId, zoneId); - const finalSelection = selectZone(newSelection, zoneId); // 이동된 존 자동 선택 + const zoneSelected = selectZone(newSelection, zoneId); // 이동된 존 자동 선택 + // IMP-55 (#93) u6 — flip the bool intent marker to `true` on real + // user drag-drop. Inverse of the u5 reset (layout apply/cancel + // auto-carry → false). handleGenerate (u7) gates `overrides.zoneSections` + // forwarding on this marker, so an unflipped drop would never reach + // the backend (the IMP-55 self-compare regression). The marker is + // flipped BEFORE persistence so the in-memory selection and the + // co-PUT body stay in sync atomically. + const finalSelection = { + ...zoneSelected, + overrides: { + ...zoneSelected.overrides, + manual_section_assignment: true, + }, + }; // IMP-52 u7 — persist the post-drop zone_sections snapshot. The on-disk // schema axis (`zone_sections`) shares the in-memory shape (zone_id → // section_ids), so we forward the full mutated value; the u4 PUT path @@ -486,10 +554,15 @@ export default function Home() { // p.uploadedFile gate skips persistence before any MDX is loaded — // the demo-mode initial render path would otherwise PUT to the empty // key. saveUserOverrides is debounced (300ms) and per-key coalesced. + // IMP-55 (#93) u6 — co-PUT `manual_section_assignment: true` in the + // SAME body so the disk file never has the post-drop zone_sections + // without the marker (would otherwise look like an unmotivated + // IMP-52 zone_sections write to the u9 backend fallback). if (p.uploadedFile) { const key = deriveUserOverridesKey(p.uploadedFile.name); void saveUserOverrides(key, { zone_sections: finalSelection.overrides.zone_sections, + manual_section_assignment: true, }); } return { ...p, userSelection: finalSelection }; diff --git a/Front/client/src/services/userOverridesApi.ts b/Front/client/src/services/userOverridesApi.ts index 1c122f1..0e7b668 100644 --- a/Front/client/src/services/userOverridesApi.ts +++ b/Front/client/src/services/userOverridesApi.ts @@ -65,6 +65,16 @@ export type ImageOverride = { }; export type ImageOverridesOverride = Record; +/** + * IMP-55 #93 u1 — bool intent marker that gates whether persisted + * `zone_sections` are consumed by the backend pipeline. Frontend sets + * `true` only on a real user drag-drop (Home.tsx handleSectionDrop, u6) + * and `false` on layout apply/cancel auto-carry (u5/u12). Mirrors the + * Python KNOWN_AXES (`manual_section_assignment`) added in u1 and the + * Vite KNOWN_USER_OVERRIDES_AXES allowlist entry added in u1. + */ +export type ManualSectionAssignmentOverride = boolean; + /** Full on-disk schema. All axes optional — file may carry any subset. */ export interface UserOverrides { layout: string; @@ -72,6 +82,7 @@ export interface UserOverrides { zone_geometries: ZoneGeometriesOverride; zone_sections: ZoneSectionsOverride; image_overrides: ImageOverridesOverride; + manual_section_assignment: ManualSectionAssignmentOverride; } /** Partial-mutation payload. `null` is the explicit clear sentinel (mirrors u4). */ diff --git a/Front/client/src/types/designAgent.ts b/Front/client/src/types/designAgent.ts index 91e42fc..178197b 100644 --- a/Front/client/src/types/designAgent.ts +++ b/Front/client/src/types/designAgent.ts @@ -213,6 +213,20 @@ export interface UserSelection { // `image_overrides` axis (KNOWN_AXES, src/user_overrides_io.py u1) and // the typed-client `ImageOverridesOverride` (services/userOverridesApi.ts u3). image_overrides: Record; + // IMP-55 (#93) u3 — bool intent marker gating whether the backend + // consumes persisted `zone_sections` as a user override. Set to `true` + // only by the real drag-drop path (Home.tsx handleSectionDrop, u6); set + // back to `false` by the layout apply/cancel auto-carry path (u5/u12). + // handleGenerate (u7) reads this flag to decide whether to forward + // `overrides.zoneSections` to the backend, replacing the pre-IMP-55 + // self-compare against `effectiveSlidePlan`. Seeded `false` in + // `createInitialUserSelection` and only restored on reopen when the + // persisted value is a real boolean (slidePlanUtils.ts u3 layering). + // Mirrors the on-disk axis added in u1 — Python KNOWN_AXES + // (src/user_overrides_io.py), Vite KNOWN_USER_OVERRIDES_AXES + // (Front/vite.config.ts), and `ManualSectionAssignmentOverride` + // (services/userOverridesApi.ts). + manual_section_assignment: boolean; }; } diff --git a/Front/client/src/utils/slidePlanUtils.ts b/Front/client/src/utils/slidePlanUtils.ts index f2aa653..8b7d08b 100644 --- a/Front/client/src/utils/slidePlanUtils.ts +++ b/Front/client/src/utils/slidePlanUtils.ts @@ -85,6 +85,20 @@ export function applyPersistedNonFrameOverrides( ) { next.image_overrides = { ...persisted.image_overrides }; } + // IMP-55 (#93) u3 — restore the bool intent marker only when the persisted + // value is a real `boolean`. A missing axis, `null` (the u4 clear sentinel + // observed post-flush), or any non-boolean shape (string "true", 1, {}) + // intentionally falls through to the `createInitialUserSelection` seed of + // `false`. This is the fail-closed half of the marker contract: the + // backend pipeline (u9) consumes persisted `zone_sections` only when + // `manual_section_assignment is True`, so anything other than a real + // `true` MUST end up as `false` in memory to avoid resurrecting stale + // auto-carry assignments as user intent. Both `true` and `false` are + // restored verbatim (the explicit `false` from u12's apply/cancel write + // is meaningful — it pins the marker off across reopens). + if (typeof persisted.manual_section_assignment === "boolean") { + next.manual_section_assignment = persisted.manual_section_assignment; + } return { ...selection, overrides: next }; } @@ -160,6 +174,14 @@ export function createInitialUserSelection(slidePlan?: SlidePlan | null): UserSe // here via `saveImageOverride` (SlideCanvas drag/resize handler) and // are seeded on reopen via `applyPersistedNonFrameOverrides`. image_overrides: {}, + // IMP-55 (#93) u3 — bool intent marker seeded `false` so a fresh + // MDX open (no persisted file, or persisted file with axis absent) + // never forwards `overrides.zoneSections` to the backend. The marker + // flips to `true` only via the real drag-drop path (Home.tsx u6) and + // is reset to `false` by layout apply/cancel auto-carry (u5/u12). + // `applyPersistedNonFrameOverrides` may restore a persisted boolean + // verbatim on reopen — see the bool-only guard there. + manual_section_assignment: false, }, }; } diff --git a/Front/client/tests/user_overrides_endpoint.test.ts b/Front/client/tests/user_overrides_endpoint.test.ts index ff34d15..954b4fe 100644 --- a/Front/client/tests/user_overrides_endpoint.test.ts +++ b/Front/client/tests/user_overrides_endpoint.test.ts @@ -310,16 +310,44 @@ describe("KNOWN_USER_OVERRIDES_AXES (IMP-52 u4)", () => { // The on-disk schema is shared with backend pipeline fallback (u2). // Any drift here means a PUT could write an axis that the Python // load() ignores, or vice-versa, silently losing user overrides. + // Mirror is Python-minus-`slide_css` (known IMP-45 #74 gap — the + // frontend never writes slide_css). IMP-55 #93 u1 adds the bool + // `manual_section_assignment` axis as a first-class allowlist entry. expect(KNOWN_USER_OVERRIDES_AXES).toEqual([ "layout", "zone_geometries", "zone_sections", "frames", "image_overrides", + "manual_section_assignment", ]); }); }); +describe("mergeUserOverrides (IMP-55 #93 u1) — manual_section_assignment bool axis", () => { + it("merges bool true / false literally and clears on null", () => { + // The PUT handler must treat the bool axis like any other allowlisted + // axis: replace on write, preserve when absent, delete on null. Tests + // both true→false flip and explicit null-clear so the backend (u9) + // sees the exact frontend intent. + let merged = mergeUserOverrides({}, { manual_section_assignment: true }); + expect(merged.manual_section_assignment).toBe(true); + + merged = mergeUserOverrides(merged, { manual_section_assignment: false }); + expect(merged.manual_section_assignment).toBe(false); + + merged = mergeUserOverrides(merged, { manual_section_assignment: null }); + expect("manual_section_assignment" in merged).toBe(false); + }); + + it("preserves bool axis when partial touches only a sibling axis", () => { + const existing = { manual_section_assignment: true, layout: "old" }; + const merged = mergeUserOverrides(existing, { layout: "new" }); + expect(merged.manual_section_assignment).toBe(true); + expect(merged.layout).toBe("new"); + }); +}); + describe("mergeUserOverrides (IMP-52 u4)", () => { it("only mutates KNOWN_AXES present in partial", () => { const existing = { diff --git a/Front/client/tests/user_overrides_restore.test.ts b/Front/client/tests/user_overrides_restore.test.ts index cb3f9ea..021d307 100644 --- a/Front/client/tests/user_overrides_restore.test.ts +++ b/Front/client/tests/user_overrides_restore.test.ts @@ -54,6 +54,11 @@ function makeSelection(overrides?: Partial): UserSel // axis declared on `UserSelection.overrides`. Empty by default so the // existing IMP-52 cases remain unchanged in shape. image_overrides: {}, + // IMP-55 (#93) u3 — bool intent marker is REQUIRED on + // `UserSelection.overrides` (not optional). Default to `false` so every + // pre-existing fixture matches the `createInitialUserSelection` seed + // and stays compile-clean after u3 widened the type. + manual_section_assignment: false, ...overrides, }, }; @@ -460,3 +465,88 @@ describe("image_overrides axis — saveImageOverride (IMP-51 u11)", () => { expect(sel.overrides.image_overrides).toEqual(before); }); }); + +// ─── IMP-55 (#93) u3 — manual_section_assignment bool axis ────────────────── +// Restore-on-reopen / seed coverage for the bool intent marker. Production +// branch lives at `slidePlanUtils.ts` — `applyPersistedNonFrameOverrides` +// guards with `typeof persisted.manual_section_assignment === "boolean"`, +// and `createInitialUserSelection` seeds the axis to `false`. The marker +// gates whether `handleGenerate` (u7) forwards `overrides.zoneSections` +// to the backend; the pipeline (u9) consumes persisted `zone_sections` +// only when the marker is exactly `true`, so any non-boolean payload MUST +// end up `false` in memory (fail-closed). + +describe("manual_section_assignment axis — applyPersistedNonFrameOverrides (IMP-55 #93 u3)", () => { + it("restores literal true verbatim", () => { + const sel = makeSelection(); + const next = applyPersistedNonFrameOverrides(sel, { + manual_section_assignment: true, + }); + expect(next.overrides.manual_section_assignment).toBe(true); + }); + + it("restores literal false verbatim (u12 apply/cancel write must survive reopen)", () => { + // Seed `true` so the assertion proves `false` overwrites; a truthiness + // check instead of `typeof === \"boolean\"` would silently keep `true` + // and resurrect stale auto-carry assignments as user intent. + const sel = makeSelection({ manual_section_assignment: true }); + const next = applyPersistedNonFrameOverrides(sel, { + manual_section_assignment: false, + }); + expect(next.overrides.manual_section_assignment).toBe(false); + }); + + it("leaves the in-memory marker unchanged when the persisted axis is absent", () => { + const sel = makeSelection({ manual_section_assignment: true }); + const next = applyPersistedNonFrameOverrides(sel, { layout: "horizontal-2" }); + expect(next.overrides.manual_section_assignment).toBe(true); + expect(next.overrides.layout_preset).toBe("horizontal-2"); + }); + + it.each([ + ["null clear sentinel", null], + ['string "true"', "true"], + ['string "false"', "false"], + ["number 1", 1], + ["number 0", 0], + ["object {}", {}], + ["array []", []], + ])("ignores non-boolean payload (%s) — keeps prior in-memory value", (_label, payload) => { + const sel = makeSelection({ manual_section_assignment: true }); + const next = applyPersistedNonFrameOverrides(sel, { + manual_section_assignment: payload as unknown as boolean, + }); + expect(next.overrides.manual_section_assignment).toBe(true); + }); + + it("seeds an empty selection with manual_section_assignment=false (createInitialUserSelection)", () => { + const sel = createInitialUserSelection(); + expect(sel.overrides.manual_section_assignment).toBe(false); + }); + + it("returns a NEW selection object (no input mutation) when restoring the marker", () => { + const sel = makeSelection({ manual_section_assignment: false }); + const next = applyPersistedNonFrameOverrides(sel, { + manual_section_assignment: true, + }); + expect(next).not.toBe(sel); + expect(next.overrides).not.toBe(sel.overrides); + // Input still pristine — proves the helper does not flip the fixture. + expect(sel.overrides.manual_section_assignment).toBe(false); + }); + + it("layers the bool axis alongside other persisted axes in a single call", () => { + const sel = makeSelection(); + const next = applyPersistedNonFrameOverrides(sel, { + layout: "vertical-2", + zone_sections: { top: ["03-1"], bottom: ["03-2"] }, + manual_section_assignment: true, + }); + expect(next.overrides.layout_preset).toBe("vertical-2"); + expect(next.overrides.zone_sections).toEqual({ + top: ["03-1"], + bottom: ["03-2"], + }); + expect(next.overrides.manual_section_assignment).toBe(true); + }); +}); diff --git a/Front/client/tests/user_overrides_service.test.ts b/Front/client/tests/user_overrides_service.test.ts index d359daf..fb7115a 100644 --- a/Front/client/tests/user_overrides_service.test.ts +++ b/Front/client/tests/user_overrides_service.test.ts @@ -559,3 +559,67 @@ describe("saveUserOverrides (IMP-51 #79 u3) — image_overrides axis", () => { }); }); }); + +// ============================================================================ +// IMP-55 #93 u1 — manual_section_assignment axis (7th axis) parity coverage +// +// The bool intent marker rides on the same per-axis coalescing rails as the +// 6 sibling axes. These tests lock the typed client behavior so a regression +// in the boolean serialization (e.g., coercion to "true" string, dropped +// `false` due to truthy filtering) fails here instead of in Home.tsx (u6/u7) +// or the backend gate (u9~u11). +// ============================================================================ + +describe("saveUserOverrides (IMP-55 #93 u1) — manual_section_assignment axis", () => { + it("PUT body carries only manual_section_assignment when it is the sole mutated axis", async () => { + fetchMock.mockResolvedValue(mockResponse({})); + void saveUserOverrides("03", { manual_section_assignment: true }); + vi.advanceTimersByTime(300); + await drainMicrotasks(); + + const body = lastPutBody() as Record; + expect(Object.keys(body)).toEqual(["manual_section_assignment"]); + expect(body.manual_section_assignment).toBe(true); + }); + + it("later-wins coalesces true → false within a single debounce window", async () => { + // Drag-then-cancel inside 300 ms — server must see only the final + // `false`, not a transient `true` that would re-enable backend + // consumption of stale zone_sections. + fetchMock.mockResolvedValue(mockResponse({})); + void saveUserOverrides("03", { manual_section_assignment: true }); + void saveUserOverrides("03", { manual_section_assignment: false }); + + vi.advanceTimersByTime(300); + await drainMicrotasks(); + expect(putCallsCount()).toBe(1); + expect(lastPutBody()).toEqual({ manual_section_assignment: false }); + }); + + it("forwards null sentinel verbatim (explicit clear)", async () => { + fetchMock.mockResolvedValue(mockResponse({})); + void saveUserOverrides("03", { manual_section_assignment: null }); + + vi.advanceTimersByTime(300); + await drainMicrotasks(); + expect(lastPutBody()).toEqual({ manual_section_assignment: null }); + }); + + it("coalesces with zone_sections sibling into a single PUT (drag-drop pair)", async () => { + // Real-world drag flow (u6): one save() sets the bool + zone_sections + // together. Asserts both axes survive coalescing as a single PUT body. + fetchMock.mockResolvedValue(mockResponse({})); + void saveUserOverrides("03", { + zone_sections: { left: ["03-2"], right: ["03-1"] }, + manual_section_assignment: true, + }); + + vi.advanceTimersByTime(300); + await drainMicrotasks(); + expect(putCallsCount()).toBe(1); + expect(lastPutBody()).toEqual({ + zone_sections: { left: ["03-2"], right: ["03-1"] }, + manual_section_assignment: true, + }); + }); +}); diff --git a/Front/client/tests/user_overrides_write.test.ts b/Front/client/tests/user_overrides_write.test.ts index aabb231..b978027 100644 --- a/Front/client/tests/user_overrides_write.test.ts +++ b/Front/client/tests/user_overrides_write.test.ts @@ -85,6 +85,22 @@ function sliceHandler(source: string, name: string): string { return source.slice(start, end); } +/** + * IMP-55 #93 u8 — strip JS/TS line + block comments so source-pattern + * regex checks assert against LIVE code only. The u5 / u7 docblocks in + * Home.tsx intentionally reference removed identifiers (e.g. `defaultByZone`, + * `sameAsDefault`, `zoneSectionsDiff`) and the marker axis name in prose to + * document the Stage 1 root cause for future readers — those references are + * documentation, not behavior, and must not trigger negative-match guards. + * Strips `// ...` to EOL and `/* ... *​/` (incl. multi-line) — keeps string + * literals intact because we only consume the result for regex-match tests. + */ +function stripComments(source: string): string { + return source + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); +} + describe("Home.tsx write-side wiring (IMP-52 u10) — source pattern", () => { it("handleSectionDrop persists zone_sections behind uploadedFile gate", () => { const block = sliceHandler(HOME_TSX, "handleSectionDrop"); @@ -567,3 +583,220 @@ describe("restore-on-reopen end-to-end (IMP-52 u10)", () => { expect(remapPersistedFramesToZoneFrames(plan, persisted.frames)).toEqual({}); }); }); + +// ─── IMP-55 #93 u8 — manual_section_assignment intent marker contract ───── +// Verifies four axes of the marker contract introduced in u3 (type) / u5 +// (apply reset) / u6 (drag flip + co-PUT) / u7 (generate gate): +// 1) Drag dual-axis persistence — handleSectionDrop persists BOTH +// `zone_sections` AND `manual_section_assignment: true` in the SAME +// PUT body (co-PUT atomicity — disk never sees post-drop zone_sections +// without the marker). +// 2) Apply / cancel reset — handleApplyPendingLayout writes explicit +// `manual_section_assignment: false` after the `...overrides` spread, +// and handleCancelPendingLayout relies on createInitialUserSelection +// (which u3 seeds to `false`) to drop a prior `true`. +// 3) Marker-gated forwarding — handleGenerate gates `overrides.zoneSections` +// forwarding strictly on `manualMarker === true` (NOT truthiness, NOT +// `!= null`, NOT presence). u3-seeded `false` and absent values both +// skip forwarding. +// 4) sameAsDefault NOT required — the Stage 1 anti-pattern (defaultByZone +// / sameAsDefault / zoneSectionsDiff self-compare loop) is gone from +// `handleGenerate` entirely; the marker is the source of intent. + +describe("IMP-55 #93 u8 — manual_section_assignment marker contract", () => { + it("handleSectionDrop sets marker true in-memory before persistence", () => { + const block = sliceHandler(HOME_TSX, "handleSectionDrop"); + // finalSelection literal (built from zoneSelected, then marker = true) + // must occur BEFORE the saveUserOverrides call so the in-memory state + // and the PUT body source from the same overrides shape. + const markerIdx = block.search(/manual_section_assignment:\s*true/); + const saveIdx = block.search(/saveUserOverrides\(/); + expect(markerIdx).toBeGreaterThan(-1); + expect(saveIdx).toBeGreaterThan(-1); + expect(markerIdx).toBeLessThan(saveIdx); + }); + + it("handleSectionDrop co-PUTs zone_sections + manual_section_assignment:true (single body)", () => { + const block = sliceHandler(HOME_TSX, "handleSectionDrop"); + // Single saveUserOverrides call carrying BOTH axes. The regex spans the + // call body to prove the two keys live in the same object literal — a + // future split into two PUTs would race the 300ms debounce and re-open + // the IMP-55 stale-disk window. + expect(block).toMatch( + /saveUserOverrides\([\s\S]*?zone_sections:[\s\S]*?manual_section_assignment:\s*true[\s\S]*?\)/, + ); + // Exactly ONE saveUserOverrides call in the handler. + const calls = block.match(/saveUserOverrides\(/g) ?? []; + expect(calls.length).toBe(1); + }); + + it("handleApplyPendingLayout resets the marker to false in overrides literal", () => { + const block = sliceHandler(HOME_TSX, "handleApplyPendingLayout"); + // After spreading `...p.userSelection.overrides`, the explicit + // `manual_section_assignment: false` overrides any prior-drag `true`. + // Without this the layout flip would carry the marker through, and u7 + // would forward auto-carried assignments as user overrides → the + // PARTIAL_COVERAGE regression that motivated IMP-55. + expect(block).toMatch(/\.\.\.p\.userSelection\.overrides[\s\S]*?manual_section_assignment:\s*false/); + }); + + it("handleCancelPendingLayout uses createInitialUserSelection (u3 seeds false)", () => { + const block = sliceHandler(HOME_TSX, "handleCancelPendingLayout"); + // Cancel discards all pending in-memory edits via the fresh-selection + // helper — the seed (u3) is the single source of truth for the + // in-memory marker on this path. u12 adds a separate disk-side + // saveUserOverrides PUT (covered by the u12 describe block below); + // the in-memory userSelection literal still has no explicit marker + // field — the seed handles it. + expect(block).toMatch(/createInitialUserSelection\(p\.slidePlan\)/); + // In-memory contract: no `manual_section_assignment` property appears + // inside the userSelection assignment. The only marker reference in + // live code lives inside the u12 saveUserOverrides(...) call body. + const codeOnly = stripComments(block); + expect(codeOnly).not.toMatch( + /userSelection:[\s\S]*?manual_section_assignment/, + ); + }); + + it("handleGenerate gates overrides.zoneSections on manualMarker === true (strict bool)", () => { + const block = sliceHandler(HOME_TSX, "handleGenerate"); + // Marker read AND strict-equality gate. `===` not `==`, not truthiness, + // not presence — so `false` / absent both skip forwarding (fail-closed). + expect(block).toMatch(/state\.userSelection\.overrides\.manual_section_assignment/); + expect(block).toMatch(/manualMarker\s*===\s*true/); + // The assignment to `overrides.zoneSections` must live INSIDE the + // marker-true branch. + const gateIdx = block.search(/if\s*\(\s*manualMarker\s*===\s*true\s*\)/); + const assignIdx = block.search(/overrides\.zoneSections\s*=/); + expect(gateIdx).toBeGreaterThan(-1); + expect(assignIdx).toBeGreaterThan(gateIdx); + }); + + it("handleGenerate filters forwarded zone_sections to valid zone_ids only (cross-layout safety)", () => { + const block = sliceHandler(HOME_TSX, "handleGenerate"); + // A stale persisted layout could carry zone_ids that do not exist in + // the current sourcePlan (e.g. horizontal-2 `top`/`bottom` while the + // current layout is vertical-2 `left`/`right`). Those foreign keys + // must be dropped before reaching the backend `--override-section- + // assignment` so they cannot trigger PARTIAL_COVERAGE. + expect(block).toMatch(/validZoneIds\s*=\s*new Set\(\s*sourcePlan\.zones\.map\(\(z\)\s*=>\s*z\.zone_id\)/); + expect(block).toMatch(/if\s*\(!validZoneIds\.has\(zoneId\)\)\s*continue/); + }); + + it("handleGenerate no longer contains the IMP-08 B-3 self-compare anti-pattern", () => { + // Strip comments — the u7 docblock intentionally references the removed + // identifiers (`defaultByZone` / `sameAsDefault` / `zoneSectionsDiff`) + // in prose to explain the Stage 1 root cause for future readers; the + // regression we guard against is the LIVE code re-emerging. + const block = stripComments(sliceHandler(HOME_TSX, "handleGenerate")); + // The Stage 1 root cause: these identifiers compared user input against + // itself (sourcePlan === effectiveSlidePlan → zones === pendingZones, + // both derived from the same overrides.zone_sections). u7 deleted the + // entire block. + expect(block).not.toMatch(/\bdefaultByZone\b/); + expect(block).not.toMatch(/\bsameAsDefault\b/); + expect(block).not.toMatch(/\bzoneSectionsDiff\b/); + }); + + it("co-PUT payload contract: marker=true + zone_sections land in a single PUT body", async () => { + fetchMock.mockResolvedValue(mockResponse({})); + // Shape produced by handleSectionDrop after the u6 marker flip. + void saveUserOverrides("03_demo", { + zone_sections: { left: ["03-2"], right: ["03-1"] }, + manual_section_assignment: true, + }); + vi.advanceTimersByTime(300); + await drainMicrotasks(); + expect(fetchMock).toHaveBeenCalledTimes(1); + const body = lastPutBody() as Record; + // Both axes in the same PUT body — exact equality, not arrayContaining, + // because any extra axis would mean a foreign mutation leaked through. + expect(Object.keys(body).sort()).toEqual( + ["manual_section_assignment", "zone_sections"].sort(), + ); + expect(body.manual_section_assignment).toBe(true); + expect(body.zone_sections).toEqual({ left: ["03-2"], right: ["03-1"] }); + }); + + it("co-PUT payload contract: marker=false carries explicitly through saveUserOverrides", async () => { + // u12 will add the apply/cancel explicit `false` PUT; the typed client + // must already propagate the literal `false` through the debounce + // bucket. A truthiness-based coalesce in the bucket merge would drop + // the value and re-open the stale-disk window. This locks the wire + // contract independently of the u12 caller-site write. + fetchMock.mockResolvedValue(mockResponse({})); + void saveUserOverrides("03_demo", { manual_section_assignment: false }); + vi.advanceTimersByTime(300); + await drainMicrotasks(); + const body = lastPutBody() as Record; + expect(Object.keys(body)).toEqual(["manual_section_assignment"]); + expect(body.manual_section_assignment).toBe(false); + }); +}); + +// ─── IMP-55 #93 u12 — stale-disk marker reset on apply / cancel ─────────── +// u5 resets the in-memory marker on layout apply, and u3's seed via +// `createInitialUserSelection` resets it on cancel. But the disk persists +// independently — a prior drag wrote `true` via u6's co-PUT, so after a +// page reload the u3 restore branch would re-seed `true` and the u7 gate +// would forward auto-carried section assignments → PARTIAL_COVERAGE +// regression. u12 closes that window by writing `manual_section_assignment: +// false` to disk via saveUserOverrides on both apply and cancel paths. +describe("IMP-55 #93 u12 — stale-disk marker reset on layout apply/cancel", () => { + it("handleApplyPendingLayout source contains a marker=false saveUserOverrides PUT", () => { + const block = sliceHandler(HOME_TSX, "handleApplyPendingLayout"); + // Stripped-comment source so the u5 docblock prose doesn't satisfy the + // assertion — must be a real call expression. + const code = stripComments(block); + // Uploaded-file gate (mirrors the u6 / other handler pattern — the + // demo-mode initial render path must not PUT to an empty key). + expect(code).toMatch( + /if\s*\(\s*p\.uploadedFile\s*\)[\s\S]*?saveUserOverrides\([\s\S]*?manual_section_assignment:\s*false[\s\S]*?\)/, + ); + expect(code).toMatch(/deriveUserOverridesKey\(p\.uploadedFile\.name\)/); + }); + + it("handleCancelPendingLayout source contains a marker=false saveUserOverrides PUT", () => { + const block = sliceHandler(HOME_TSX, "handleCancelPendingLayout"); + const code = stripComments(block); + // Cancel handler converts from arrow-body to function-body for the + // disk PUT; the in-memory reset still comes from createInitialUserSelection. + expect(code).toMatch( + /if\s*\(\s*p\.uploadedFile\s*\)[\s\S]*?saveUserOverrides\([\s\S]*?manual_section_assignment:\s*false[\s\S]*?\)/, + ); + expect(code).toMatch(/createInitialUserSelection\(p\.slidePlan\)/); + }); + + it("apply path PUT payload: marker=false carries alone (no auto-carry leakage)", async () => { + // The apply handler issues a dedicated PUT for the marker reset that is + // independent of the (conditional) zone_geometries PUT and of the + // in-memory zone_sections rewrite. The wire contract for this PUT must + // contain only the marker — if zone_sections leaked into the same body + // it would re-arm the u9 backend fallback gate against u12's intent. + fetchMock.mockResolvedValue(mockResponse({})); + void saveUserOverrides("03_demo", { manual_section_assignment: false }); + vi.advanceTimersByTime(300); + await drainMicrotasks(); + expect(fetchMock).toHaveBeenCalledTimes(1); + const body = lastPutBody() as Record; + expect(Object.keys(body)).toEqual(["manual_section_assignment"]); + expect(body.manual_section_assignment).toBe(false); + }); + + it("apply path PUT is unconditional (does NOT gate on hadPriorGeoms)", () => { + // The u4 zone_geometries PUT inside handleApplyPendingLayout is + // conditional (`p.uploadedFile && hadPriorGeoms`). The u12 marker PUT + // must NOT inherit that gate — a stale disk `true` can exist without + // any prior zone_geometries, so the reset must always fire. + const code = stripComments(sliceHandler(HOME_TSX, "handleApplyPendingLayout")); + // Locate the marker PUT and verify its enclosing `if` clause is just + // `p.uploadedFile`, not the compound `... && hadPriorGeoms` guard. + const markerCallMatch = code.match( + /if\s*\(([^)]*)\)\s*\{[^}]*saveUserOverrides\([^)]*manual_section_assignment:\s*false[^)]*\)/, + ); + expect(markerCallMatch).not.toBeNull(); + if (markerCallMatch) { + expect(markerCallMatch[1].trim()).toBe("p.uploadedFile"); + } + }); +}); diff --git a/Front/vite.config.ts b/Front/vite.config.ts index b1663ef..82b1d68 100644 --- a/Front/vite.config.ts +++ b/Front/vite.config.ts @@ -219,19 +219,25 @@ function vitePluginStorageProxy(): Plugin { export const USER_OVERRIDES_KEY_RE = /^[A-Za-z0-9_][A-Za-z0-9_.\-]*$/; -// The five in-scope axes — exact mirror of KNOWN_AXES in -// src/user_overrides_io.py. Any payload key outside this allowlist is -// silently dropped by the PUT handler (u4) so the on-disk schema cannot -// drift from the backend pipeline (u2) contract. Foreign top-level keys -// already on disk are preserved verbatim (see mergeUserOverrides). +// The six in-scope axes — mirror of KNOWN_AXES in +// src/user_overrides_io.py minus `slide_css` (known gap, IMP-45 #74 — +// the Python side persists it for backend consumption; the Vite PUT does +// not write it because the frontend never mutates the slide-level CSS +// override). Any payload key outside this allowlist is silently dropped +// by the PUT handler (u4) so the on-disk schema cannot drift from the +// backend pipeline (u2) contract. Foreign top-level keys already on disk +// are preserved verbatim (see mergeUserOverrides). // IMP-51 (#79) u2: added `image_overrides` (image_id → {x,y,w,h} // percent-of-slide coordinates). +// IMP-55 (#93) u1: added `manual_section_assignment` (bool intent marker +// — drag-drop sets true, layout apply/cancel sets false). export const KNOWN_USER_OVERRIDES_AXES = [ "layout", "zone_geometries", "zone_sections", "frames", "image_overrides", + "manual_section_assignment", ] as const; export type KnownUserOverridesAxis = (typeof KNOWN_USER_OVERRIDES_AXES)[number]; diff --git a/src/phase_z2_pipeline.py b/src/phase_z2_pipeline.py index 4264a6c..e9f46db 100644 --- a/src/phase_z2_pipeline.py +++ b/src/phase_z2_pipeline.py @@ -8262,8 +8262,20 @@ if __name__ == "__main__": except (TypeError, ValueError): continue overrides_geoms = _accepted - # zone_sections — CLI empty → fill from file (dict[str, list[str]]). - if not overrides_section_assignments: + # zone_sections — CLI empty AND persisted manual_section_assignment is + # exactly True → fill from file (dict[str, list[str]]). IMP-55 (#93) u9 + # marker gate (fail-closed) : ``manual_section_assignment`` absent / + # False / non-bool (string/int/list/dict) all skip the fallback so a + # stale ``zone_sections`` axis left on disk from a prior drag cannot + # smuggle ``--override-section-assignment`` into the next run after the + # user reset intent via layout apply/cancel (u5 / u12 write + # ``manual_section_assignment: false`` explicitly). CLI + # ``--override-section-assignment`` payloads remain authoritative + # (u11 truth-table) — this gate only affects the file→fallback path. + if ( + not overrides_section_assignments + and _persisted.get("manual_section_assignment") is True + ): _file_sections = _persisted.get("zone_sections") if isinstance(_file_sections, dict): _accepted_sec: dict[str, list[str]] = {} diff --git a/src/user_overrides_io.py b/src/user_overrides_io.py index 2795fa8..6cdfaf4 100644 --- a/src/user_overrides_io.py +++ b/src/user_overrides_io.py @@ -5,16 +5,19 @@ auto-restores user choices without re-clicking. Source of truth = MDX-keyed file (stem of the MDX path), NOT ``data/runs//`` which mints a fresh run_id per ``/api/run`` invocation. -Schema (6 axes; stable order; IMP-51 #79 u1 added ``image_overrides``; -IMP-45 #74 u1 added ``slide_css``): +Schema (7 axes; stable order; IMP-51 #79 u1 added ``image_overrides``; +IMP-45 #74 u1 added ``slide_css``; IMP-55 #93 u1 added +``manual_section_assignment`` as a bool intent marker so the backend can +distinguish a user drag-drop from frontend auto-carry zone_sections): { - "layout": , - "zone_geometries": {: {"x": float, "y": float, "w": float, "h": float}}, - "zone_sections": {: [, ...]}, - "frames": {: }, - "image_overrides": {: {"x": float, "y": float, "w": float, "h": float}}, - "slide_css": + "layout": , + "zone_geometries": {: {"x": float, "y": float, "w": float, "h": float}}, + "zone_sections": {: [, ...]}, + "frames": {: }, + "image_overrides": {: {"x": float, "y": float, "w": float, "h": float}}, + "slide_css": , + "manual_section_assignment": } ``image_id`` is the stable identifier emitted by the user-content image @@ -55,10 +58,13 @@ from typing import Any, Optional _PKG_ROOT = Path(__file__).resolve().parent.parent DEFAULT_OVERRIDES_ROOT = _PKG_ROOT / "data" / "user_overrides" -# The six in-scope axes (IMP-51 #79 u1 added ``image_overrides``; IMP-45 -# #74 u1 added ``slide_css``). Any other top-level key in the file is -# preserved but ignored by callers — keeps the file forward-compatible -# with future axes (e.g., zone_sizes) without a schema bump here. +# The seven in-scope axes (IMP-51 #79 u1 added ``image_overrides``; IMP-45 +# #74 u1 added ``slide_css``; IMP-55 #93 u1 added +# ``manual_section_assignment`` — bool intent marker that gates whether +# persisted ``zone_sections`` are consumed by the backend pipeline). Any +# other top-level key in the file is preserved but ignored by callers — +# keeps the file forward-compatible with future axes (e.g., zone_sizes) +# without a schema bump here. KNOWN_AXES: tuple[str, ...] = ( "layout", "zone_geometries", @@ -66,6 +72,7 @@ KNOWN_AXES: tuple[str, ...] = ( "frames", "image_overrides", "slide_css", + "manual_section_assignment", ) # Key validation — MDX stem must be safe for filesystem use. Allow diff --git a/tests/test_user_overrides_io.py b/tests/test_user_overrides_io.py index c82edea..bfd2e3e 100644 --- a/tests/test_user_overrides_io.py +++ b/tests/test_user_overrides_io.py @@ -2,9 +2,10 @@ Covers the persisted axes called out in the Stage 2 plan (IMP-51 #79 u1 extended this to 5 axes by adding ``image_overrides``; -IMP-45 #74 u1 extended to 6 axes by adding ``slide_css``): +IMP-45 #74 u1 extended to 6 axes by adding ``slide_css``; +IMP-55 #93 u1 extended to 7 axes by adding ``manual_section_assignment``): -1. Round-trip ``save`` → ``load`` (6 KNOWN_AXES + foreign top-level keys). +1. Round-trip ``save`` → ``load`` (7 KNOWN_AXES + foreign top-level keys). 2. Unknown-key passthrough (foreign axes preserved across partial merges). 3. Missing / corrupt / non-object behavior (graceful ``{}`` + stderr warning). 4. Invalid keys (``InvalidOverrideKey`` raised on traversal / separators / @@ -121,19 +122,26 @@ def _full_payload() -> dict: "img-1": {"x": 10.0, "y": 20.0, "w": 30.0, "h": 25.0}, }, "slide_css": "", + "manual_section_assignment": True, } def test_known_axes_includes_image_overrides(): - """IMP-51 #79 u1 — ``image_overrides`` is a known axis (now 6 total).""" + """IMP-51 #79 u1 — ``image_overrides`` is a known axis (now 7 total).""" assert "image_overrides" in KNOWN_AXES - assert len(KNOWN_AXES) == 6 + assert len(KNOWN_AXES) == 7 def test_known_axes_includes_slide_css(): - """IMP-45 #74 u1 — ``slide_css`` is a known axis (6 total).""" + """IMP-45 #74 u1 — ``slide_css`` is a known axis (7 total).""" assert "slide_css" in KNOWN_AXES - assert len(KNOWN_AXES) == 6 + assert len(KNOWN_AXES) == 7 + + +def test_known_axes_includes_manual_section_assignment(): + """IMP-55 #93 u1 — bool intent marker is a known axis (7 total).""" + assert "manual_section_assignment" in KNOWN_AXES + assert len(KNOWN_AXES) == 7 def test_save_then_load_round_trip(tmp_path): @@ -161,6 +169,7 @@ def test_save_partial_payload_preserves_other_axes(tmp_path): assert loaded["frames"] == _full_payload()["frames"] assert loaded["image_overrides"] == _full_payload()["image_overrides"] assert loaded["slide_css"] == _full_payload()["slide_css"] + assert loaded["manual_section_assignment"] is True def test_save_partial_image_overrides_preserves_other_axes(tmp_path): @@ -183,6 +192,7 @@ def test_save_partial_image_overrides_preserves_other_axes(tmp_path): assert loaded["zone_sections"] == _full_payload()["zone_sections"] assert loaded["frames"] == _full_payload()["frames"] assert loaded["slide_css"] == _full_payload()["slide_css"] + assert loaded["manual_section_assignment"] is True def test_save_axis_replaces_not_deep_merges(tmp_path): @@ -226,6 +236,24 @@ def test_save_preserves_foreign_top_level_keys(tmp_path): assert loaded["schema_version"] == pre_seed["schema_version"] +def test_save_manual_section_assignment_round_trips_both_booleans(tmp_path): + """IMP-55 #93 u1 — bool axis round-trips true/false and clears on None. + + Asserts the bool is preserved literally (not coerced to int / string) so + the backend pipeline (u9) can branch on ``is True`` without false-positive + matches from truthy-but-not-True values seeded by older callers. + """ + key = "03" + save(key, {"manual_section_assignment": True}, root=tmp_path) + assert load(key, root=tmp_path)["manual_section_assignment"] is True + + save(key, {"manual_section_assignment": False}, root=tmp_path) + assert load(key, root=tmp_path)["manual_section_assignment"] is False + + save(key, {"manual_section_assignment": None}, root=tmp_path) + assert "manual_section_assignment" not in load(key, root=tmp_path) + + def test_save_creates_parent_directory(tmp_path): nested = tmp_path / "deep" / "nest" assert not nested.exists() @@ -241,6 +269,7 @@ def test_save_writes_pretty_sorted_json_for_diffability(tmp_path): pos_frames = raw.index('"frames"') pos_image_overrides = raw.index('"image_overrides"') pos_layout = raw.index('"layout"') + pos_manual = raw.index('"manual_section_assignment"') pos_slide_css = raw.index('"slide_css"') pos_zg = raw.index('"zone_geometries"') pos_zs = raw.index('"zone_sections"') @@ -248,6 +277,7 @@ def test_save_writes_pretty_sorted_json_for_diffability(tmp_path): pos_frames < pos_image_overrides < pos_layout + < pos_manual < pos_slide_css < pos_zg < pos_zs diff --git a/tests/test_user_overrides_pipeline_fallback.py b/tests/test_user_overrides_pipeline_fallback.py index d7c0d06..05b29f4 100644 --- a/tests/test_user_overrides_pipeline_fallback.py +++ b/tests/test_user_overrides_pipeline_fallback.py @@ -48,6 +48,14 @@ def _exec_main_block( override_zone_geometries=None, override_section_assignments=None, override_image_overrides=None, + # IMP-55 (#93) u9 — mirror the live ``run_phase_z2_mvp1`` signature so + # the __main__ dispatch in src/phase_z2_pipeline.py:8332 does not raise + # TypeError on kwargs added by IMP-45 #74 (``override_slide_css``) and + # IMP-43 #72 (``reuse_from``). The u9 truth-table assertions only read + # the section-assignment axis; the new kwargs are captured here so any + # follow-up test can pin them without re-touching this harness. + override_slide_css=None, + reuse_from=None, ): captured["mdx_path"] = mdx_path captured["run_id"] = run_id @@ -56,6 +64,8 @@ def _exec_main_block( captured["override_zone_geometries"] = override_zone_geometries captured["override_section_assignments"] = override_section_assignments captured["override_image_overrides"] = override_image_overrides + captured["override_slide_css"] = override_slide_css + captured["reuse_from"] = reuse_from monkeypatch.setattr(_pz2, "run_phase_z2_mvp1", _fake_run) monkeypatch.setattr(sys, "argv", argv) @@ -97,6 +107,13 @@ def _write_full_payload(tmp_path: Path, stem: str = "03") -> Path: "top": ["03-1"], "bottom": ["03-2", "03-3"], }, + # IMP-55 (#93) u9 — seed the new ``manual_section_assignment`` + # marker True so the per-axis / CLI-wins / partial-merge tests + # below continue to exercise the file→fallback path under the + # gate added in src/phase_z2_pipeline.py (``is True`` identity + # check). Truth-table coverage for False / absent / non-bool + # belongs to u10 and lives in its own test section. + "manual_section_assignment": True, "image_overrides": { "img-file-a": {"x": 10.0, "y": 15.0, "w": 30.0, "h": 25.0}, "img-file-b": {"x": 50.0, "y": 50.0, "w": 40.0, "h": 40.0}, @@ -408,3 +425,178 @@ def test_image_overrides_fallback_coerces_int_values_to_float(tmp_path, monkeypa assert coerced == {"img-int": {"x": 10.0, "y": 20.0, "w": 30.0, "h": 40.0}} for axis_value in coerced["img-int"].values(): assert isinstance(axis_value, float) + + +# -- 8. IMP-55 (#93) u10 — manual_section_assignment marker truth-table ---- +# +# The backend file-fallback gate added in u9 +# (``src/phase_z2_pipeline.py`` ``_persisted.get("manual_section_assignment") +# is True``) is exercised here with the full truth-table of marker values a +# stale or hand-edited overrides file could legally contain. The gate is +# fail-closed by ``is True`` identity, so only literal Python ``True`` +# (JSON ``true``) propagates ``zone_sections`` into +# ``override_section_assignments``; everything else — absent, ``False``, +# truthy non-bool (``"true"``, ``1``, ``[]``, ``{}``), and ``None`` — must +# leave the axis as ``None`` (``or None`` collapses an empty dict on the +# call site). No hardcoding of section IDs in the assertion logic; the +# values ``03-1`` / ``03-2`` here are sample payload literals, not pinned +# behavior. ``_write_marker_payload`` reuses the IO loader by way of the +# ``__main__`` block (no direct import of the pipeline gate). + +_MARKER_ABSENT = object() + + +def _write_marker_payload( + tmp_path: Path, marker: Any, stem: str = "03" +) -> Path: + """Write a minimal overrides file with ``zone_sections`` + optional marker. + + ``marker is _MARKER_ABSENT`` → omit ``manual_section_assignment`` key. + Any other value (including ``None``) → write it verbatim as JSON. + """ + payload: dict[str, Any] = { + "zone_sections": {"top": ["03-1"], "bottom": ["03-2"]}, + } + if marker is not _MARKER_ABSENT: + payload["manual_section_assignment"] = marker + path = tmp_path / f"{stem}.json" + path.write_text(json.dumps(payload), encoding="utf-8") + return path + + +def test_marker_true_fills_section_assignments(tmp_path, monkeypatch): + """marker=True + zone_sections in file + CLI empty → file value flows.""" + _redirect_overrides_root(tmp_path, monkeypatch) + _write_marker_payload(tmp_path, True, "03") + + captured: dict[str, Any] = {} + _exec_main_block(captured, ["src.phase_z2_pipeline", "03.mdx"], monkeypatch) + + assert captured["override_section_assignments"] == { + "top": ["03-1"], + "bottom": ["03-2"], + } + + +@pytest.mark.parametrize( + "marker", + [ + _MARKER_ABSENT, + False, + "true", # JSON-string truthy must NOT pass the ``is True`` gate. + 1, # int truthy must NOT pass. + [], + {}, + None, + ], + ids=["absent", "false", "string_true", "int_one", "empty_list", "empty_dict", "null"], +) +def test_marker_non_true_skips_section_assignments( + tmp_path, monkeypatch, marker +): + """marker absent / False / non-bool → gate fail-closed → axis is None. + + Even though ``zone_sections`` is present in the file, the gate refuses + to forward it because ``manual_section_assignment`` is not literal + ``True``. ``or None`` on the call site collapses the empty dict back + to ``None``. + """ + _redirect_overrides_root(tmp_path, monkeypatch) + _write_marker_payload(tmp_path, marker, "03") + + captured: dict[str, Any] = {} + _exec_main_block(captured, ["src.phase_z2_pipeline", "03.mdx"], monkeypatch) + + assert captured["override_section_assignments"] is None + + +# -- 9. IMP-55 (#93) u11 — CLI ``--override-section-assignment`` wins over +# persisted manual-marker fallback ---------------------------------------- +# +# The u9 gate only fires on the file→fallback branch +# (``not overrides_section_assignments and _persisted.get(...) is True``). +# When the CLI supplies ``--override-section-assignment`` the +# ``overrides_section_assignments`` dict is truthy before the gate is +# evaluated, so the persisted ``zone_sections`` axis (and the +# ``manual_section_assignment`` marker that would otherwise unlock it) is +# bypassed entirely — CLI is authoritative on the section-assignment +# axis. The three cases below pin this contract: +# +# 1. CLI + persisted marker True (file value present) → CLI wins. +# 2. CLI + persisted marker False (file value present) → CLI wins; the +# marker is irrelevant on the CLI-wins path (no truth-value coupling). +# 3. CLI with no overrides file at all → CLI value flows through; the +# marker is a gate on file→fallback only, never a precondition for +# any CLI ``cli_override`` to take effect. +# +# Cross-axis bystanders (layout / frames / geometries / images) are +# intentionally not seeded here; this section locks CLI-vs-marker +# semantics on the section axis only. Cross-axis CLI-wins behavior is +# already covered by the IMP-52 #80 tests in section 3 / 3b above. + + +def test_cli_section_assignment_wins_over_persisted_marker_true( + tmp_path, monkeypatch +): + """marker=True + file zone_sections + CLI value → CLI wins per-axis.""" + _redirect_overrides_root(tmp_path, monkeypatch) + _write_marker_payload(tmp_path, True, "03") + + captured: dict[str, Any] = {} + _exec_main_block( + captured, + [ + "src.phase_z2_pipeline", + "03.mdx", + "--override-section-assignment", + "top=cli-section", + ], + monkeypatch, + ) + + # CLI value wholly replaces the file zone_sections (per-axis win); the + # gate's ``not overrides_section_assignments`` precondition is false. + assert captured["override_section_assignments"] == {"top": ["cli-section"]} + + +def test_cli_section_assignment_wins_with_persisted_marker_false( + tmp_path, monkeypatch +): + """marker=False + file zone_sections + CLI value → CLI wins; marker unread.""" + _redirect_overrides_root(tmp_path, monkeypatch) + _write_marker_payload(tmp_path, False, "03") + + captured: dict[str, Any] = {} + _exec_main_block( + captured, + [ + "src.phase_z2_pipeline", + "03.mdx", + "--override-section-assignment", + "bottom=cli-section", + ], + monkeypatch, + ) + + assert captured["override_section_assignments"] == {"bottom": ["cli-section"]} + + +def test_cli_section_assignment_works_without_persisted_file( + tmp_path, monkeypatch +): + """No overrides file → CLI value flows; marker is not a CLI precondition.""" + _redirect_overrides_root(tmp_path, monkeypatch) + + captured: dict[str, Any] = {} + _exec_main_block( + captured, + [ + "src.phase_z2_pipeline", + "03.mdx", + "--override-section-assignment", + "top=cli-only", + ], + monkeypatch, + ) + + assert captured["override_section_assignments"] == {"top": ["cli-only"]}