From 8ddefec4815dc04b376c2534db2f3a1efe5c7423 Mon Sep 17 00:00:00 2001
From: Fini <fini.yang@gmail.com>
Date: Sun, 10 May 2026 14:58:00 +0800
Subject: [PATCH] fix(ai): contrast detector skips node-level opacity=0 /
 visible=false too
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Second Codex stop-hook caught: the previous fix only guarded fill-level
opacity (`fill.opacity === 0` / 8-hex alpha 00). PenNodeBase has its
own `opacity?: number | string` and `visible?: boolean` fields that
hide the WHOLE wrapper including its fill. A wrapper with

    { fill: [{type:'solid', color:'#FFFFFF'}], opacity: 0, ... }

was still being treated as a white bg and masking the real bg further
up the chain.

ancestorBgColor() now skips ancestors whose node-level
`opacity === 0` or `visible === false`, complementing the
firstSolidColor fill-level guard. `opacity` can be a `$variable` ref
in PenDocument; resolving that to a literal 0 is not yet covered —
we only catch the literal-0 case for now (which is the AI-output
shape the corpus produces).

Two new test cases cover both paths.
---
 .../__tests__/detectors-typography.test.ts    | 34 +++++++++++++++++++
 .../src/diagnostics/detectors-typography.ts   | 10 +++++-
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts b/packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts
index d51020bfc..9b8605c3b 100644
--- a/packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts
+++ b/packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts
@@ -216,6 +216,40 @@ describe('detectTextBgContrast', () => {
     expect(issues[0].reason).toMatch(/bg=#FFF8E7/);
   });
 
+  it('skips wrapper with NODE-LEVEL opacity=0 and uses real bg further up', () => {
+    // 2026-05-10 second Codex stop-hook — node-level opacity is a
+    // separate path from fill-level opacity. A wrapper with white fill
+    // and node opacity=0 is fully invisible; the detector must not
+    // pick up its white as the bg.
+    const wrap: PenNode = {
+      id: 'wrap',
+      type: 'frame',
+      layout: 'vertical',
+      fill: solid('#FFFFFF'),
+      opacity: 0,
+      children: [text('t1', solid('#FFF8E7'))],
+    } as unknown as PenNode;
+    const root = frame('page', [wrap], solid('#FFF8E7'));
+    const issues = detectTextBgContrast(root, emptyDoc);
+    expect(issues).toHaveLength(1);
+    expect(issues[0].reason).toMatch(/bg=#FFF8E7/);
+  });
+
+  it('skips wrapper with visible=false and uses real bg further up', () => {
+    const wrap: PenNode = {
+      id: 'wrap',
+      type: 'frame',
+      layout: 'vertical',
+      fill: solid('#FFFFFF'),
+      visible: false,
+      children: [text('t1', solid('#FFF8E7'))],
+    } as unknown as PenNode;
+    const root = frame('page', [wrap], solid('#FFF8E7'));
+    const issues = detectTextBgContrast(root, emptyDoc);
+    expect(issues).toHaveLength(1);
+    expect(issues[0].reason).toMatch(/bg=#FFF8E7/);
+  });
+
   it('still treats opacity=0.5 as visible (only opacity=0 is the alpha-0 sentinel)', () => {
     // We don't try to math a 50% wash against the layer below; that is
     // outside the detector's scope. The fill stays as the bg.
diff --git a/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts b/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts
index 43310a6a9..715d0f92a 100644
--- a/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts
+++ b/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts
@@ -150,7 +150,15 @@ export function detectTextBgContrast(
     // default and avoids the "everything fails" report when a doc has
     // no explicit page fill.
     for (let i = ancestors.length - 1; i >= 0; i--) {
-      const fill = (ancestors[i] as unknown as { fill?: unknown }).fill;
+      // Node-level opacity=0 / visible=false hides the WHOLE wrapper
+      // including its fill. The 2026-05-10 Codex review caught this as
+      // a separate path from fill-level opacity (the inner `firstSolid
+      // Color` guard) — both have to skip or a node-opacity-0 wrapper
+      // masks the real bg further up.
+      const ancestor = ancestors[i] as PenNode & { opacity?: unknown; visible?: unknown };
+      if (ancestor.opacity === 0) continue;
+      if (ancestor.visible === false) continue;
+      const fill = (ancestor as unknown as { fill?: unknown }).fill;
       const raw = firstSolidColor(fill);
       if (!raw) continue;
       const resolved = resolveColorRef(raw, variables, theme);