rastrillo / fiesta Public

Fiesta 1.2.0 for Rastrillo part 2 (rastrillo branch fiesta-part-2, discussion rastrillo/rastrillo#231). Task review by Astra, one fix round, scoped re-review; copy by Luna (items 1-3 applied, item 4 kept: the 1.1 paragraph only gained a filename).

Astra task review

Spec Compliance

• ❌ Issues found: the pixel exception admits more than the ring fix; the hunk audit is incomplete; new code does not consistently follow the naming, comment and 80-column constraints. Details below.
• ✅ Required implementation is present: version/scripts registration (fiesta.go:12), transition example (default.html:6), mapping API (sources.go:16) and changelog (CHANGELOG.md:5).
• ⚠️ Cannot verify from diff: excluded release artifacts, stripped CSS size, complete shipped-code naming compliance, and the bytes Rastrillo will serve after switching. The controller should verify the final serving path, especially because the audit retains Fiesta’s additional CSS hunks (hack/catch-up-1.2.md:22).
• ⚠️ Completion remains pending: the report explicitly defers copy review and release/tag/deployment. Confirm that deferral is authorized before marking the full brief complete (task-1-report.md:211, task-1-report.md:225).

Strengths

• ✅ Script exceptions pin exact hashes and name dedicated regressions; subsequent edits cannot silently reuse the exemption (releases_test.go:365).
• ✅ Transition tests execute the actual scripts against real rejecting promises, including ordinary errors and unrelated rejections. Busy tests exercise claimed/native submissions in both load orders (transition_node.mjs:52, liveformbrowsertest.go:90).
• ✅ Sources() isolates callers through cloning; its tests compare both keys and fields against independently read JSON (sources.go:44, sources_test.go:16).
• ✅ Declaration exceptions constrain selector, media context, property and value, with negative cases for nearby unauthorized changes (baseline_test.go:221).

Issues

Critical (Must Fix)

• None found.

Important (Should Fix)

• Overbroad pixel exemption: paritybrowsertest.go:149 blanks the current link’s entire rectangle plus its surrounding ring. sameAreas checks only that rectangle, so changed text, icon colour or movement of children inside an unchanged link box can pass. The negative control proves detection of outer-box movement only. Restrict masking to the affected edge pixels, or independently compare the preserved interior and add a negative control for an internal change.
• Plan-mandated duplication: js/shell.js:92 and js/transition.js:6 duplicate the transition-observation/filtering block. Future fixes require synchronized edits. Byte-preserving imports explain the duplication but do not remove it under this review rubric. Resolve through one maintained source generating both standalone outputs, or obtain the controller’s explicit acceptance of this plan conflict.
• Incomplete required hunk audit: hack/catch-up-1.2.md:77 summarizes select’s ten hunks and datetime’s seventeen collectively; calendar receives another aggregate summary. Step 1 requires each differing hunk to be listed. Add individual locations, classifications and provenance so omissions can be checked.

Minor (Nice to Have)

• Timing-dependent regression test: liveformbrowsertest.go:118 substitutes a 500 ms sleep for completion of the native response. A slower response can still reproduce the acknowledged navigation race. Wait for the specific request to finish.
• Conflicting load-order contract: fiesta.go:89 still promises load order while placing the required early script after shell.js. Correct the public contract and gallery instructions consistently.
• Style constraints missed: baseline_test.go:205 introduces abbreviated names and long compound entries; paritybrowsertest.go:147 adds implementation-restating comments. Apply the requested style to newly authored code.

Assessment

Task quality: Needs fixes.

Reasoning: The runtime changes and dedicated regressions are substantive, but the preservation gate now hides additional changes inside the current link. The audit and plan-mandated duplication also need resolution before acceptance.

• Check performed: Read the supplied diff in sections. Completed the truncated busy-handler/release context at js/busy.js:95 to check late arming against held resubmissions; no additional defect found. No tests rerun or checkout mutations performed.

Astra re-review

1. Overbroad pixel exemption — ADDRESSED. check/rendering_exception_browser_test.go:114 limits masking to the outer band outside the kept box; :219 adds an internal text-colour negative control.
2. Duplicated transition observers — ADDRESSED. check/shell_test.go:71 compares both observers, allowing only their function names to differ; missing blocks fail.
3. Individual hunk provenance — ADDRESSED. hack/catch-up-1.2.md:77, :97, :123 list all 10/17/3 hunks with locations, newer side, provenance and disposition.
4. Wait for the liveform request — ADDRESSED. check/liveform_browser_test.go:110 registers the listener before submission; :121 awaits completion instead of sleeping 500ms.
5. Load-order consistency — ADDRESSED. fiesta.go:81, README.md:31 and internal/gallery/copy.go:136 explicitly identify transition.js as the early script.
6. New test-code style — NOT ADDRESSED (Minor). Long lines remain at check/shell_test.go:76 and check/rendering_exception_browser_test.go:252; abbreviated id, mu and e remain at check/liveform_browser_test.go:135; check/rendering_exception_browser_test.go:62 still describes what code does rather than why.
7. Approved copy — ADDRESSED. CHANGELOG.md:5 matches Luna’s entry verbatim; README.md:31 retains the instructed order exception; internal/gallery/copy.go:136 uses the approved plain-text blurb without backticks.

New Breakage in the Fix Diff: None.

Out-of-Scope Observations: None.

Check: Compared Luna’s CHANGELOG text mechanically, counted all hunk rows, and checked new test-line lengths. Check: The fix report names covering tests and includes pixel-control output at task-1-report.md:275; observer and request-wait results are summaries, with no complete captured output. No tests rerun; no git commands or mutations performed.

Fix round: Findings remain open — finding 6 (Minor: test-code style).(Finding 6 fixed in 470d08e.)

Luna

1. Corrected version:

```markdown ## 1.2.0

Fiesta includes fixes Rastrillo made to these files after Fiesta was copied from it. One changes how 1.0 markup looks: the current item in the console rail now shows its full ring and shadow. No other styles in 1.0.0 or 1.1.0 change appearance or layout.

- The console rail's current item now shows its full ring and shadow. Previously, the rail clipped them at its edges. No item moves. (Rastrillo cc7c9c69) - shell.js no longer logs an uncaught AbortError when the browser skips a page transition. It ignores only this error; other errors are still logged. (Rastrillo d9c9ed13, 023740c5, e5e35a3e and 6b1c5478) - For column, stage and topbar pages without shell.js, transition.js does the same. Load it before the stylesheets, without defer: the browser can skip a transition before a deferred script runs. The column, stage and topbar examples load it. Sidebar and console pages don't need it. - busy.js leaves submits from a form marked data-live-form to the script that handles them. A server that swaps forms in place sets the attribute, and its script marks each submit it handles with liveForm. busy.js shows the spinner only when that script leaves a submit to the browser. Without such a script, the attribute has no effect. (Rastrillo 8548e455) - fiesta.Sources() reports the source of each example state: the partial, idiom, shell, format or screen it was seeded from, the state within that source, and its locale and view. A server can use it to check its templates against the examples. ```

The intro and several phrases were grammatically unclear; the revisions clarify what is logged, handled and reported.

1. Corrected version: “transition.js prevents an error when the browser skips a page transition on a column, stage or topbar page. Load it before the stylesheets, without defer. Sidebar and console pages don't need it because shell.js does the same.”

“Stops a page transition … from logging” makes it sound as if the transition logs the error, and “the one exception to the order” has no clear referent.

1. Corrected version: “Prevents an error when the browser skips a page transition on column, stage and topbar pages. Load it before the stylesheets, without defer. Sidebar and console pages don't need it.”

“Stops a skipped page transition logging” is awkward and unclear.

1. Corrected version: “fiesta.js closes menus, lets you drag a switch's knob, adds the pressed state for iOS and runs the colour-scheme menu; load it first. busy.js, light.js, anchor.js, shell.js, transition.js, copy.js, select.js, datetime.js and calendar.js do nothing until a page uses the features they enhance, and each can be deleted on its own. Load calendar.js last.”

“Gives iOS its pressed state” and “each deletable” are awkward; the revision makes both actions clearer.