From bc426dd34fa9217c028ebcf5bf764f8032d0ac12 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Sun, 20 Sep 2026 10:49:03 -0700 Subject: [PATCH] Clarify component review scope alongside full comp crops --- ui/component-review/model.test.ts | 18 ++++++++++++++++-- ui/component-review/model.ts | 18 ++++++++++++++++++ ui/component-review/review.ts | 18 +++++++++++++----- ui/component-review/styles.ts | 6 ++++++ 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/ui/component-review/model.test.ts b/ui/component-review/model.test.ts index 6380a95ec..b224317ad 100644 --- a/ui/component-review/model.test.ts +++ b/ui/component-review/model.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'bun:test'; -import { reviewUnits, decisionTargets, nextUnreviewed, approveRemaining, newDraft, submission, summarize, validBox, type ReviewPacket } from './model'; +import { reviewScope, reviewUnits, decisionTargets, nextUnreviewed, approveRemaining, newDraft, submission, summarize, validBox, type ReviewPacket } from './model'; const packet: ReviewPacket = { id:'review-1', revision:'packet-1', title:'Test', round:1, comp:{url:'/comp.png',width:100,height:100}, components:[{id:'art',revision:'art-1',name:'Art',medium:'Raster',note:'',box:{x:0,y:0,w:1,h:1},preview:{kind:'image',url:'/art.png'}},{id:'control',revision:'control-1',name:'Button',medium:'HTML',note:'',box:{x:0,y:0,w:.1,h:.1},preview:{kind:'page',url:'/page.html'}}] }; describe('component review drafts',()=>{ test('bulk approval still requires explicit inventory confirmation',()=>{const draft=approveRemaining(packet,newDraft(packet));expect(summarize(packet,draft).canSubmit).toBe(false);draft.inventoryConfirmed=true;expect(submission(packet,draft).requestId).toBe(packet.id);}); @@ -96,7 +96,7 @@ test('review units collapse explicit code groups while retaining individual rast draft.decisions['room-0']={revision:'control-1',action:'revise',feedback:'Fix this one',split:false}; const group=reviewUnits(p,draft)[0];expect(group.id).toBe('room-0');expect(group.representative.id).toBe('room-1'); for(const c of decisionTargets(p,draft,group.representative,true))draft.decisions[c.id]={revision:c.revision,action:'approve',feedback:'',split:false}; - expect(draft.decisions['room-0'].feedback).toBe('Fix this one');expect(reviewUnits(p,draft)[0]).toMatchObject({pending:0,kind:'feedback',stateLabel:'1 need work'}); + expect(draft.decisions['room-0'].feedback).toBe('Fix this one');expect(reviewUnits(p,draft)[0]).toMatchObject({pending:0,kind:'feedback',stateLabel:'1 needs work'}); expect(summarize(p,draft)).toMatchObject({approved:13,revisions:1,pending:1}); p.components[2].revision='changed';expect(reviewUnits(p,draft)[0]).toMatchObject({pending:1,kind:'pending'}); }); @@ -112,3 +112,17 @@ test('group approval cannot overwrite a reviewed representative',()=>{ expect(decisionTargets(p,draft,original,true).map(c=>c.id)).toEqual(['second']); expect(decisionTargets(p,draft,original,false).map(c=>c.id)).toEqual([original.id]); }); + +test('scope distinguishes other mapped pieces from explicit capture exclusions',()=>{ + const p=structuredClone(packet), card=p.components[0]; + card.note='Card outline'; + const child=p.components[1]; + child.box={x:.2,y:.2,w:.2,h:.2}; + p.components.push({...child,id:'background',box:{x:0,y:0,w:1,h:1}}); + expect(reviewScope(p,card).related.map(c=>c.id)).toEqual(['control']); + expect(reviewScope(p,card).excluded).toEqual([]); + card.preview.isolation={method:'dom-component-v1',selector:'#card',excludedComponents:['control']}; + expect(reviewScope(p,card).excluded.map(c=>c.id)).toEqual(['control']); + expect(reviewScope(p,child).related).toEqual([]); + expect(reviewScope(p,card).description).toBe('Card outline'); +}); diff --git a/ui/component-review/model.ts b/ui/component-review/model.ts index c1e6e2f8a..303d816f5 100644 --- a/ui/component-review/model.ts +++ b/ui/component-review/model.ts @@ -133,3 +133,21 @@ export function reviewUnits(packet: ReviewPacket, draft: Draft, history?: Review : feedback.length ? (members.length>1 ? `${feedback.length} ${feedback.length===1?'needs':'need'} work` : 'Feedback ready') : 'Approved'}]; }); } + +/** Nearby map geometry is context, not proof of capture exclusions. Only the + * capture's explicit exclusion list can identify deliberately hidden layers. */ +export function reviewScope(packet: ReviewPacket, component: Component) { + const excluded = new Set(component.preview.isolation?.excludedComponents ?? []); + const b = component.box; + const related = packet.components.filter(other => { + if (other.id === component.id) return false; + if (excluded.has(other.id)) return true; + const o = other.box; + const overlap = Math.max(0, Math.min(b.x+b.w,o.x+o.w)-Math.max(b.x,o.x)) * + Math.max(0, Math.min(b.y+b.h,o.y+o.h)-Math.max(b.y,o.y)); + // Strictly smaller, substantially contained regions; omit backgrounds and + // neighboring cards. This never changes the review or any reference pixels. + return o.w*o.h < b.w*b.h && overlap / (o.w*o.h) >= .98; + }); + return {description:component.note.trim(), related, excluded:related.filter(c=>excluded.has(c.id))}; +} diff --git a/ui/component-review/review.ts b/ui/component-review/review.ts index df24a8467..61ccf4a73 100644 --- a/ui/component-review/review.ts +++ b/ui/component-review/review.ts @@ -1,4 +1,4 @@ -import { reviewUnits, reviewPeers, decisionTargets, inReviewQueue, type InventoryFilter, componentPresentation, nextUnreviewed, approveRemaining, componentState, repairStatus, newDraft, submission, summarize, type Box, type Decision, type Draft, type ReviewPacket, type ReviewHistory } from './model'; +import { reviewScope, type Component, reviewUnits, reviewPeers, decisionTargets, inReviewQueue, type InventoryFilter, componentPresentation, nextUnreviewed, approveRemaining, componentState, repairStatus, newDraft, submission, summarize, type Box, type Decision, type Draft, type ReviewPacket, type ReviewHistory } from './model'; import { comparisonSize, hoverPan } from './viewport'; import { styles } from './styles'; import { icon } from './icons'; @@ -50,6 +50,13 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op let resize: ResizeObserver | null = null; const checkIcon = ''; const feedbackIcon = ''; + const scopeMarkup = (p: ReviewPacket, item: Component) => { + if (p.stage === 'hero') return ''; + const scope = reviewScope(p,item); + if (!scope.related.length) return ''; + const names = scope.related.map(other=>esc(other.name)).join(' · '); + return `

Reviewing ${esc(scope.description || item.name)}

Separate review items ${names}

${scope.excluded.length?`

Hidden in this preview: ${scope.excluded.map(other=>esc(other.name)).join(' · ')}

`:''}
`; + }; const boxStyle = (b: Box) => `left:${pct(b.x)};top:${pct(b.y)};width:${pct(b.w)};height:${pct(b.h)}`; function focusReview(id: string) { root.getElementById(id)?.focus({preventScroll:true}); @@ -182,6 +189,7 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op const useContext = !!(v?.context && outputMode === 'context'); const useFrame = v && (useContext ? v.context?.kind !== 'image' : v.preview.kind === 'page'); const sourceUrl = useContext && v?.context ? v.context.url : v?.preview.url; + const visibleScope = v ? reviewScope(vp,v) : null; const materialLabel = presentation?.code ? `${presentation.label} · ${presentation.captured ? 'captured from code' : 'live preview'}` : v?.material ? `${v.material.alpha === 'transparent' ? 'Transparent' : v.material.alpha === 'opaque' ? 'Opaque' : 'Transparency unverified'} ${v.material.format}` : 'Raster · transparency unverified'; root.innerHTML = `

${submitted?'Review record.':assembled?'Review the assembled page.':'Review the components.'}

${esc(packet.title)} · Round ${packet.round}

${submitted ? 'Submitted · read-only' : options.preview ? 'Interactive preview' : ''}
@@ -212,14 +220,14 @@ export function mountComponentReview(host: HTMLElement, packet: ReviewPacket, op `:''}

${esc(groupOverview?unit!.label:v!.name)}

${peers.length>1?`
${peers.length} instances${!groupOverview?'':'Select to inspect'}
`:''}
${priorComponent?`
`:''}
${v?.preview.kind==='image'?`${icon('external')}`:''}
- ${groupOverview?`
In the compProduced
${unit!.members.map((m,n)=>{const st=stateFor(m);return ``;}).join('')}
`:''} -
-
${viewingPrevious ? `Comp · Round ${history!.packet.round}` : assembled ? 'Approved comp' : 'In the comp'}
Reference region for ${esc(v!.name)}
+ ${groupOverview?`
Full comp cropComponent preview
${unit!.members.map((m,n)=>{const st=stateFor(m);return ``;}).join('')}
`:''} + ${!groupOverview&&v?scopeMarkup(vp,v):''}
+
${viewingPrevious ? `Comp · Round ${history!.packet.round}` : assembled ? 'Approved comp' : visibleScope?.related.length ? 'Full comp crop' : 'In the comp'}
Reference region for ${esc(v!.name)}
${viewingPrevious ? `Previous · Round ${history!.packet.round}` : assembled ? 'Assembled page' : useContext ? 'In context' : history ? `${presentation!.caption} · Round ${packet.round}` : presentation!.caption}
${!useFrame ? `Produced ${esc(v!.name)}` : ``}${overlay ? `Reference overlay` : ''}
${hasTransparency || v!.context ? `
${v!.context ? `
` : ''}${hasTransparency?`
`:''}
` : ''}
${!assembled&&!groupOverview?`
${icon(presentation!.code ? 'code' : 'image')}${esc(materialLabel)}${v?.material ? `${v.material.width} × ${v.material.height} px` : ''}
${vp.stage==='components'&&presentation?.captured&&!v?.preview.isolation?'

Legacy region capture · may include overlapping components.

':''}${v?.context?.layering&&(isRaster||useContext)?`

${esc(v.context.layering)}

`:''} -

${esc(v!.note)}

+

${!visibleScope?.related.length?esc(v!.note):''}

`:''}
${notice}${viewingPrevious?'

Viewing the previous round. Return to Current to make a decision.

':''}${submitted?`
${d?.action==='approve'?'Approved':d?.action==='revise'?'Changes requested':'Not reviewed'}Submitted in round ${packet.round} · read-only
`:`
${!assembled?`
Your review Round ${packet.round}${viewingPrevious?'

Return to Current to review this round.

':''}
`:''}${d && !assembled && !groupOverview ? `` : ''}
`} ${edit ? `
` : d?.action==='revise' ? `

${esc(d.feedback || 'No note — agent will diagnose.')}

` : ''} ${assembled?`

${esc(error || (submitted?'Your decision is saved.':sending?'Sending…':edit?'':'Approval confirms the composition and that nothing is missing.'))}

`:''}
` : missing ? `

This piece will be added to the unresolved inventory.

${(['x','y','w','h'] as const).map(k=>``).join('')}
` : '

No components supplied.

'} diff --git a/ui/component-review/styles.ts b/ui/component-review/styles.ts index a8f34ebe2..9fc4d98d7 100644 --- a/ui/component-review/styles.ts +++ b/ui/component-review/styles.ts @@ -128,4 +128,10 @@ export const styles = ` #comparison-dialog .group-overview{overflow:auto;min-height:0} #comparison-dialog .group-overview>.compare-toolbar{top:0} #comparison-dialog .group-overview>.expanded-title{padding-right:44px} + +.review-scope{font-size:12px;line-height:1.5;margin:8px 0 12px;color:var(--color-text,#292929);text-align:left;white-space:normal} +.review-scope p{margin:4px 0;overflow-wrap:anywhere}.review-scope strong{font-weight:600;margin-right:5px} +.review-scope .separate-reviews,.review-scope .scope-excluded{font-size:11px;color:var(--muted)} +.instance-row .review-scope{margin:12px 0 0} +#comparison-dialog .review-scope{flex:none} `;