[codex] Fix Appendix IV manual sign crops#203
Conversation
cucumberfalse
left a comment
There was a problem hiding this comment.
Codex Review: blocking findings on head 15089f6.
Blocking finding: final crop outputs still contain visibly clipped/partial signs. Examples I verified locally include src/data/manual-signs/app4SignEntries.json:3507 (app4regulatory-p185-029-no-cambiar-de-carril-catalog-entry.png), :5596 (app4regulatory-p186-018-uso-de-cadenas-para-nieve-catalog-entry.png), :5716 and :5836 (both GIRO OBLIGATORIO variants), plus app4informational-p190-005-fin-de-camino-peatonal-a-100-m.png and app4traffic-lights-p197-010-avanzar-peatones.png. These rows are nevertheless marked reviewed-final-correct, and I found 56 sign-like rows where finalSourceRegionAtBaseScale is less than 60% of the baseline in at least one dimension. The source-limited counts/wording are otherwise separated correctly (286 source-limited, 0 true native/effective 3x passes), and sign-like runtime rows no longer use source-image-css-clip, but the PR does not satisfy the customer's crop-correction request until the bad crops and evidence are regenerated.
GitHub would not allow this actor to submit a formal REQUEST_CHANGES review on its own PR, so this is submitted as a blocking COMMENT review.
cucumberfalse
left a comment
There was a problem hiding this comment.
Codex Review: blocking finding on head 7bdf95f.
[P1] Some final warning crops are still contaminated by neighboring entries/unrelated source text while being stamped reviewed-final-correct.
The generated PNG for app4warning-p188-004-cruce-de-ciclistas-catalog-entry visibly includes unrelated source text at the top (...go eventual) and partial neighboring yellow warning signs on both sides, but src/data/manual-signs/app4SignEntries.json:14340 marks it cropAuditStatus: "reviewed-final-correct" and src/data/manual-signs/app4SignEntries.json:14383 records the audit as passing. The same class is present in nearby final PNGs such as app4warning-p188-005-jinetes-catalog-entry (src/data/manual-signs/app4SignEntries.json:14503 / :14545), app4warning-p188-009-presencia-de-vehiculos-extranos-tranvia (src/data/manual-signs/app4SignEntries.json:15148 / :15190), and app4warning-p188-010-presencia-de-vehiculos-extranos-tractor (src/data/manual-signs/app4SignEntries.json:15311 / :15353), where partial neighboring signs remain in the final assets/contact sheet.
Feature 037 explicitly requires final crops to exclude neighboring entries and unrelated source text, so the automated audit cannot mark these rows final-correct or let validation/contact-sheet evidence pass until the contamination is removed or those rows are held out of the final-correct count.
Notes from the follow-up review: the prior clipped/partial examples I spot-checked are improved, unconditional final-correct stamping is now constrained by cropAuditBasis, and the source-limited disclosure remains accurate in the data/evidence (286 sign-like rows are source-limited-exception, 0 true native/effective 3x passes, 286 direct assets, 316 total rows). This finding is specifically about crop contamination still passing the new audit.
cucumberfalse
left a comment
There was a problem hiding this comment.
Codex Review: blocking finding on head 5691ed4.
The originally cited CRUCE DE CICLISTAS, JINETES, Tranvía, and Tractor crops are now visually isolated, and the source-limited disclosure/counts still look accurate (316 rows, 286 sign-like assets, 286 source-limited exceptions, 0 true native/effective 3x passes, no sign-like runtime source-image-css-clip). However, the broader neighboring-content class is still not fixed: the new guard passes additional contaminated warning crops, so final evidence/contact sheets still overstate crop quality.
cucumberfalse
left a comment
There was a problem hiding this comment.
Codex Review: no findings on head 076cf8c.
Follow-up review notes: the previously blocked Curva / Chevron and FLECHA DIRECCIONAL PNGs are now visually isolated, and the regenerated warning contact sheet matches the final crop quality I spot-checked. The broader warning right-edge contamination guard is now meaningful for the reviewed class: warning rows cannot pass with right-edge contact, the inventory evidence shows 53 sign-like warning rows with 0 right-edge contacts and no guard failures, and tests cover the two newly cited rows plus all warning rows. Source-limited disclosure remains accurate: 316 total rows, 286 sign-like direct PNG assets, 286 source-limited exceptions, 0 true native/effective 3x passes, and no sign-like runtime source-image-css-clip.
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
💡 Codex Reviewhttps://github.com/cucumberfalse/cabadrive/blob/939c0071017378bd28176a03dda36c928d57a1d9/src/data/manual-signs/app4SignEntries.json#L4154 For the page 185 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
AI Review gate failed.
|
cucumberfalse
left a comment
There was a problem hiding this comment.
Codex Review: no findings on head 939c007.
Current-head review scope was the evidence-only diff from effective content head 076cf8c to 939c007. The diff only appends role-owned final validation/process-memory notes in specs/037-manual-sign-crop-resolution/feature-request.md and specs/037-manual-sign-crop-resolution/tasks.md; it does not change product behavior, runtime assets, source data, scripts, tests, validation logic, contact sheets, or screenshots.
These process-memory additions consistently preserve the effective content head already reviewed at 076cf8c, record Architect-before-Analyst validation ordering, keep the all-row source-limited 3x-output-pixel disclosure, and do not make the prior no-findings review stale. No new review findings.
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
|
AI Review gate failed.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 939c007101
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
💡 Codex Reviewhttps://github.com/cucumberfalse/cabadrive/blob/4602d05e2fc3c8b24e46fb02f8015787174bf19c/src/data/manual-signs/app4SignEntries.json#L4752 For ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
cucumberfalse
left a comment
There was a problem hiding this comment.
Codex Review: found 1 blocking issue on head 4602d05. The two Zona de Caudales PNGs now look clean and the source-limited 3x-output-pixel disclosure remains accurate, but the follow-up guard is still too narrow for the broader regulatory right-edge/source-label contamination class.
|
AI Review gate failed.
|
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
cucumberfalse
left a comment
There was a problem hiding this comment.
Codex Review: found 1 blocking issue on head 7147669. The page-185 parking/no-stopping focus appears fixed: app4regulatory-p185-028-no-estacionar-ni-detenerse-sobre-la-ciclovia.png is visually isolated now, all 9 page-185 NO ESTACIONAR / NO ESTACIONAR NI DETENERSE rows have regenerated evidence and passing page-185 guard fields, and the source-limited 3x-output-pixel disclosure remains accurate (286 source-limited rows, 0 true native/effective 3x passes). However, the broader regulatory right-edge/source-label contamination class still passes final-correct for at least one nearby regulatory row.
|
AI Review gate failed.
|
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
cucumberfalse
left a comment
There was a problem hiding this comment.
Codex Review: no findings on head dadae84.
Follow-up review notes: the previously blocked app4regulatory-p186-025-ceda-el-paso-a-ciclistas-y-peatones.png is now visually isolated; the gray source caption and right-edge neighboring sign/text are gone. The broader regulatory detached-label class is now explicit and covered for 14 attachment rows across pages 185/186, with detached-label trim mode, source-caption height guard, right-edge guard, and a source-derived right-edge meaningful-pixel guard for the rows whose crop geometry still touches the right edge. The regenerated regulatory contact sheet matches the sampled final PNGs. Source-limited disclosure remains accurate: 316 total rows, 286 sign-like direct PNG assets, 286 source-limited exceptions, 0 true native/effective 3x passes, and no sign-like runtime source-image-css-clip.
💡 Codex ReviewThis commit records and adds new official-source files under https://github.com/cucumberfalse/cabadrive/blob/dadae843c71d4935452adf543f37edfc7f2cf9c9/src/data/manual-signs/app4SignEntries.json#L481 Because ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
AI Review gate failed.
|
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
cucumberfalse
left a comment
There was a problem hiding this comment.
Codex Review: no findings on head 696bc40.
Reviewed the follow-up for the trusted AI Connector P1/P2 findings. The five Feature 037 official-source archive records under content/official-documents/originals/traffic-sign-source-evaluation-037/ are now registered in content/official-documents/manifest.json as evidence-only entries with primarySourceReader: false, currentClaim: false, claimUse: feature_evidence_only, currentness.status: unknown, currentness.validationStatus: pending, and exactTextValidation.status: pending. The learner primary-source reader/strict coverage path filters those entries out, so they do not become current primary guide sources.
The runtime manual-sign JSON is slimmed: src/data/manual-signs/app4SignEntries.json no longer contains the proof-only fields I checked (cropAuditBasis, noUpscaleProof, baseline geometry, output hashes, source-evaluation ids, tail-trim/audit proof fields), while detailed crop proof remains in specs/037-manual-sign-crop-resolution/evidence/final/manual-sign-crop-resolution-rows.json and visual QA still reads proof from there. Runtime/disclosure counts remain consistent: 316 rows, 286 sign-like direct individual-source-crop-3x PNG rows, 30 DOM headings, 286 source-limited exceptions, and 0 true native/effective 3x passes.
Previous crop fixes were not regressed in the sampled data/contact sheet: p186-025 remains clean, the page-185 caudales/ciclovia and warning rows retain passing final evidence guards, and no sign-like runtime rows use old source-image-css-clip.
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17cd7540ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
AI Review gate failed.
|
|
@codex review |
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17cd7540ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
AI Review gate failed.
|
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3393bceb10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Administrative trigger posted by the AI Review workflow. Prefer a trusted human-authored trigger if the native backend ignores bot comments. |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
individual-source-crop-3x, 30 category headings render as DOM/non-sign rowsSource-Limited Disclosure
All 286 sign-like rows meet the final file-dimension target of at least
ceil(3 * baselineCropNaturalWidth)byceil(3 * baselineCropNaturalHeight), but all remainthreeXStatus: source-limited-exceptionwithsourceLimitedDisposition: best-official-source-3x-output-pixels. This PR must not be presented as true native/effective 3x detail;trueNativeEffectiveThreeXPassRowsremains0under the 2026-06-07T21:36:51Z Architect disposition.Evidence
specs/037-manual-sign-crop-resolution/evidence/baseline/specs/037-manual-sign-crop-resolution/evidence/source-evaluation/specs/037-manual-sign-crop-resolution/evidence/final/specs/037-manual-sign-crop-resolution/evidence/contact-sheets/specs/037-manual-sign-crop-resolution/evidence/screenshots/Validation
node --check scripts/manual-sign-crop-resolution.mjsswiftc -parse scripts/manual-sign-crop-resolution.swiftnode scripts/manual-sign-crop-resolution.mjs --writenode --check scripts/manual-sign-inventory.mjsnode scripts/manual-sign-inventory.mjs --writepnpm install --frozen-lockfilenode --check tests/manual-sign-inventory.test.mjsnode --test tests/manual-sign-inventory.test.mjs(9/9)node --check scripts/manual-sign-visual-qa.mjsnode scripts/manual-sign-visual-qa.mjsnode scripts/manual-sign-visual-qa.mjs --runtime-url http://localhost:5174/pnpm run validate:manual-sign-inventorypnpm run test(443/443)pnpm run buildpnpm run preflight(82/82Playwright e2e included)git diff --checkNotes
Runtime QA used Vite at
http://localhost:5174/because the dev script did not forward the requested--port 5178argument. No merge is performed by this Implementation Agent.