Skip to content

render: document createSurface as an explicit no-op in Apply - #48

Merged
joestump merged 1 commit into
mainfrom
feat/47-createsurface-stance
Jul 18, 2026
Merged

render: document createSurface as an explicit no-op in Apply#48
joestump merged 1 commit into
mainfrom
feat/47-createsurface-stance

Conversation

@joestump

Copy link
Copy Markdown
Collaborator

What

Apply (render/composite.go) silently dropped createSurface messages. This PR makes the stance explicit and documented rather than implicit: a2tea deliberately ignores createSurface — a surface is established by its first updateComponents, host WithStyles owns theming, and the component catalog is compiled in.

How

  • Apply gains an explicit case m.CreateSurface != nil no-op with a doc comment, and the Apply doc comment spells out the rationale: the message's theme hints (primaryColor, iconUrl, agentDisplayName) have no honorable mapping in a deliberately monochrome renderer where the host theme wins, and catalogId is ignored because the catalog is compiled in, by design. (Inspected tmc/a2ui's CreateSurface/Theme to confirm nothing is trivially honorable — applying primaryColor would violate the monochrome-chrome contract.)
  • docs/wire-format.md moves createSurface from the Not yet list to a documented decision in the implemented section.

Tests

  • TestApplyCreateSurfaceIsNoOp: a createSurface (with catalog URI + theme) neither errors nor mutates state — rendered view is byte-identical before/after, theme text does not leak, the surface stays alive, and focus survives (Enter still activates the focused button).
  • TestApplyCreateSurfaceDoesNotCreateState: createSurface alone does not make a surface renderable; the createSurfaceupdateComponents batch shape agents actually send establishes it via the updateComponents.
  • Gate: go build ./... && go vet ./... && go test ./... all green locally.

Note: the ticket's Done-when also mentions the site's "Not yet"/implemented lists — website/ changes are deferred to the post-merge docs pass per repo policy.

Fixes #47

🤖 Posted on behalf of @joestump by Claude.

Apply silently dropped createSurface messages. The stance is now explicit
and documented: a2tea treats the first updateComponents as implicit surface
creation, host WithStyles owns theming (chrome stays monochrome so the host
theme wins), and the component catalog is compiled in — so createSurface's
theme (primaryColor/iconUrl/agentDisplayName) and catalogId hints carry
nothing honorable and are deliberately ignored.

- Apply gains an explicit no-op case for CreateSurface with a doc comment,
  and the Apply doc comment spells out the rationale.
- docs/wire-format.md moves createSurface from the "Not yet" list to a
  documented decision in the implemented section.
- Tests assert createSurface neither errors nor mutates state (view, data
  model, focus all survive; theme does not leak) and that a surface is only
  established by updateComponents, including the createSurface-then-
  updateComponents batch shape agents actually send.

Fixes #47

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@joestump joestump left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review of the createSurface no-op change (issue #47). Verdict: no blockers; two nits.

What I verified

  • Gate: go build ./... && go vet ./... && go test ./... all green on the branch; both new tests pass with -count=1 (not cache artifacts).
  • State preservation: the no-op case in render/composite.go:38 cannot mutate anything — confirmed by reading Apply/applyComponents; TestApplyCreateSurfaceIsNoOp asserts real behavior (byte-identical View().Content, theme text does not leak, focus survives and Enter still emits ButtonClicked{ID: "btn2"}), not mere execution.
  • Alive semantics: TestApplyCreateSurfaceDoesNotCreateState correctly pins that createSurface alone leaves rootID == "" (Apply returns false) and that the realistic createSurfaceupdateComponents batch establishes the surface. Matches Render's first-updateComponents establishment in a2tea.go:130-141.
  • Ticket Done-when: explicit case + doc comment in code ✅, docs/wire-format.md moved from "Not yet" to a documented decision ✅, tests ✅, website/ untouched and the deferral is called out in the PR body per repo policy ✅.
  • Type claims: checked tmc/a2ui v09 — Theme really is primaryColor/iconUrl/agentDisplayName, and nothing on CreateSurface maps onto the monochrome-chrome contract. The rationale in the doc comment is accurate.

Nits (non-blocking)

  1. sendDataModel is ignored but undocumentedrender/composite.go:21-26 and docs/wire-format.md enumerate theme and catalogId as the ignored payload, but a2ui.CreateSurface carries a fourth field, SendDataModel bool (v09/message.go:24), which this no-op also drops. Since the whole point of #47 is "no silent drops," add one line noting sendDataModel is likewise ignored (a2tea has no client→agent data-model channel; ActionEvent.Context already round-trips input values). Fine to fold into the post-merge docs pass.
  2. Switch-order shadowing on malformed messages — the new case m.CreateSurface != nil at render/composite.go:38 precedes UpdateComponents, so a spec-invalid message carrying both createSurface and updateComponents now drops the components where it previously applied them. The wire format is a one-of, and Scan validates upstream, so this is theoretical — but placing the no-op case last would make Apply strictly more forgiving at zero cost.

🤖 Posted on behalf of @joestump by Claude.

@joestump
joestump merged commit 1ba12a3 into main Jul 18, 2026
1 check passed
joestump added a commit that referenced this pull request Jul 18, 2026
Union merge of interactive tab switching (#45) with the five feature PRs
that landed on main since the branch forked (#48 createSurface no-op,
#49 ChildList templates, #51 Modal open/close, #52 editable inputs,
#53 InputSubmitted/ChoiceSelected dispatch). Nothing from either side
was dropped:

- render/render.go: Surface keeps ALL state — activeTabs (branch) plus
  fieldValues/checkValues/choiceValues/sliderValues/choiceCursor/
  openModals/scope (main). isInteractive now also covers non-empty Tabs
  bars. collectFocusables walks only the ACTIVE tab's subtree, so
  inputs and modals inside inactive tabs are excluded from the focus
  ring, and only an open modal's content joins it. Key routing:
  left/right step a focused slider and switch tabs on a focused tab
  bar; h/l switch tabs only on the tab bar and stay literal runes on a
  focused TextField/DateTimeInput; enter/space/esc keep main's
  button/checkbox/choice/modal/InputSubmitted behavior.
- render/composite.go: deleteSurface clears every state map including
  activeTabs; applyComponents preserves active-tab, focus, modal, and
  edited-value state across merges, with out-of-range tabs clamping to
  the first at read time.
- render/tabs_test.go: two new tests pin the union — slider-focused vs
  tab-bar-focused left/right disambiguation, and inputs/modals inside
  an inactive tab excluded from the focus ring.
- README.md / docs/wire-format.md: merged feature descriptions; the
  'not yet' lists are now empty (issues #42#47 all landed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@joestump
joestump deleted the feat/47-createsurface-stance branch July 18, 2026 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

createSurface: honor or explicitly document ignoring (theming/catalog hints dropped silently)

1 participant