SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

fix(ai): prune hidden subtrees in contrast walk — kill double-fix FP

Browse source 
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.
This commit is contained in:
Fini 2026-05-10 14:59:00 +08:00
parent 8ddefec481
commit fcbd2230c4
2 changed files with 22 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
packages/pen-ai-skills/src/__tests__/detectors-typography.test.ts
View file

@ -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)', () => {

13
packages/pen-ai-skills/src/diagnostics/detectors-typography.ts
View file

@ -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);
}