From fcbd2230c4119cbab4c7d245d49e6fbeba856393 Mon Sep 17 00:00:00 2001
From: Fini <fini.yang@gmail.com>
Date: Sun, 10 May 2026 14:59:00 +0800
Subject: [PATCH] =?UTF-8?q?fix(ai):=20prune=20hidden=20subtrees=20in=20con?=
 =?UTF-8?q?trast=20walk=20=E2=80=94=20kill=20double-fix=20FP?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Third Codex stop-hook in this thread caught a bug introduced by the
previous fix. The "skip ancestor whose node-level opacity=0 / visible
=false" guard correctly stopped a hidden wrapper from being read as
the bg, BUT the walk still descended into the hidden subtree and
flagged its text against whatever bg sat above. Hidden text doesn't
render at all, so flagging its contrast is a textbook false positive.

Move the check up: if a node has `opacity === 0` or `visible === false`
the whole subtree gets pruned at walk time. Text inside is never
inspected. The earlier `ancestorBgColor` guard is left as defense-
in-depth (cheap and protects against direct callers).

The two tests added in 65e115e8 had the wrong expectation — they
asserted the detector flagged hidden-wrapper text. Both now flip to
"does NOT flag" matching the corrected semantic. Hidden = invisible
= no contrast pair to score.

Distinction the test set still pins:
  - fill.opacity=0 (rectangle invisible, node visible)  → walk continues, ancestor walk picks real bg further up, FLAG
  - node.opacity=0 (whole subtree invisible)            → walk prunes, NO flag

Corpus replay holds at 14 hits — no regression.
---
 .../__tests__/detectors-typography.test.ts    | 21 ++++++++-----------
 .../src/diagnostics/detectors-typography.ts   | 13 ++++++++++++
 2 files changed, 22 insertions(+), 12 deletions(-)

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 9b8605c3b..563414146 100644
--- a/packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts
+++ b/packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts
@@ -216,11 +216,12 @@ 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.
+  it('does NOT flag text inside a node-level opacity=0 subtree (text itself is invisible)', () => {
+    // 2026-05-10 Codex round 3 — when a wrapper has node-level opacity=0
+    // the entire subtree is hidden from the user, so flagging the text's
+    // contrast (against whatever bg) is a false positive: there's no
+    // visible text-bg pair to read in the first place. The detector
+    // prunes hidden subtrees up front instead of walking into them.
     const wrap: PenNode = {
       id: 'wrap',
       type: 'frame',
@@ -230,12 +231,10 @@ describe('detectTextBgContrast', () => {
       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/);
+    expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0);
   });
 
-  it('skips wrapper with visible=false and uses real bg further up', () => {
+  it('does NOT flag text inside a visible=false subtree (text itself is invisible)', () => {
     const wrap: PenNode = {
       id: 'wrap',
       type: 'frame',
@@ -245,9 +244,7 @@ describe('detectTextBgContrast', () => {
       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/);
+    expect(detectTextBgContrast(root, emptyDoc)).toHaveLength(0);
   });
 
   it('still treats opacity=0.5 as visible (only opacity=0 is the alpha-0 sentinel)', () => {
diff --git a/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts b/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts
index 715d0f92a..8032fe9df 100644
--- a/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts
+++ b/packages/pen-ai-skills/src/diagnostics/detectors-typography.ts
@@ -134,7 +134,20 @@ export function detectTextBgContrast(
   walk(root, []);
   return issues;
 
+  function isHidden(node: PenNode): boolean {
+    // Whole subtree is invisible — node-level opacity=0 or visible=false.
+    // 2026-05-10 Codex round 3 caught this as a separate path from the
+    // ancestorBgColor guard: even if we'd correctly skip the wrapper as
+    // a bg, the TEXT inside still doesn't render, so flagging its
+    // contrast against whatever sits above is a false positive.
+    const n = node as PenNode & { opacity?: unknown; visible?: unknown };
+    if (n.opacity === 0) return true;
+    if (n.visible === false) return true;
+    return false;
+  }
+
   function walk(node: PenNode, ancestors: PenNode[]): void {
+    if (isHidden(node)) return; // prune the entire subtree
     if (node.type === 'text') {
       checkText(node, ancestors);
     }