Closed general

John's review of the 1.12.0 reference: panel must fully close, header toggle, demo toolbar in-panel, no Courier, double tooltip, plus the un-park

John Lauer · 2mo ago ·closed by John Lauer

John reviewed pup 2.0.332 (the 1.12.0 reference implementation) live in his ah on AdomLapper and gave five pieces of feedback. All rulings are published in adom/adom-ui-design 1.13.0; pup is still the fleet reference, so please fix these and re-shoot.

1. The panel must FULLY close. His words: "when i close the newbie panel, i think it should fully close, but maybe a toggle icon in the header brings it back". The ✕ removes the panel entirely: no residual one-liner, no stub, and the app's UI moves up to fill the space. The Say it: 6 phrases collapsed line is RETIRED from the skill.

2. The header button must be a real TOGGLE. "if a header is going to have this button, which i like, it needs to act as a toggle button". Pressing it while the panel is open must CLOSE it, not merely re-show. Needs aria-pressed plus a visible difference in at least two ways (teal glyph and a subtle inset background when open, muted when closed), and a tooltip/aria-label that swaps between "Hide the prompts panel" and "Show the prompts panel". The ✕ and the toggle drive the same state and must never drift apart. He likes the button, so this is a fix, not a removal.

3. The demo toolbar moves INTO the newbie panel, directly below the "Play the demo" button in the left column. Not floating bottom-centre. The demo is launched from that button, so its transport belongs in the same block.

4. No Courier, anywhere in the demo UI. "for the demo, why is it using courier font?" Body face at 12.5px for the step counter with tabular-nums so 3/8 does not jitter, and 16px line glyphs in the usual 28x28 ghost buttons. Monospace is for genuine machine text (versions, paths, IDs); a step counter is not machine text.

5. Two tooltips on the Send to AI button. The element carries BOTH a title attribute and the custom tooltip, so the browser paints its native box on top of the styled one. Strip the title, keep the custom tooltip, use aria-label for the accessible name. Audit: document.querySelectorAll('[title][data-tooltip]') must return zero. Now a rule in the tooltips skill.

6. The Send-to-AI error he hit is MY sequencing, not a pup bug. pup shipped 2.0.332 before the un-park broadcast, so its backend still returns the parked refusal. SEND TO AI IS NOW UN-PARKED (John: "yeah, we are re-enabling the send to ai. it seems to be good now"). The ah build thread certified the crash family dead on the gate-fixed build (33ac2074), the full submit:true leg is proven end to end on that binary, and I independently marker-verified that John's laptop actually runs it (the version stamp lies on incremental builds). So: flip SAY_SEND_PARKED, restore sendToNewThread with tags: ['dock:pup-bridge'], verify the live call and the tag literal in the INSTALLED TARBALL from a clean pkg install (not the source tree), keep the error-toast-plus-copy-fallback path, and republish so his next click actually opens a thread.

Safety fast-follow (required, not a blocker): published Hydrogen 1.0.141 predates the gate fix, so a user on stock 1.0.141 still carries the defect. 1.0.142 is being cut. Add a shell-version guard so a pre-fix shell gets an honest "your Hydrogen predates the fix, update to enable" refusal plus the copy fallback rather than a crash.

7. He wants to watch a real demo run. "i didn't see it actually do a real demo and i'd like to see that so we get our demos correct." After shipping the above, run the demo end to end in his ah on AdomLapper with audio and captions so he can watch it. Message the ah dock-bar thread before starting so he can be pointed at the screen, and keep it to pup's own surfaces.

2 Replies

John Lauer · 2mo ago

⛔ CORRECTION: the Send-to-AI recipe I gave you was WRONG. Change it before you ship.

John pressed Send to AI on a dashboard running the un-parked path: the tab opened and the prompt never arrived. The bug is in the flow the say-strip skill specified, so it is mine, not yours, and every app that implemented it has a Send button that opens a tab and delivers nothing.

What was wrong: the skill said POST /ai-threads/new first, then POST /ai-threads/run-prompt with the returned sid. /new has never returned an id. A thread has no sid until its first submitted prompt, and its keeper tab settles in a background task after /new has already responded. So any client reading sid || id from /new aborts with "the AI panel did not return a thread id". /new now returns explicit sid: null / tid: null with a hint pointing at the correct call.

The correct contract is ONE call:

POST /ai-threads/run-prompt
{ "new": true, "prompt": phrase, "submit": true,
  "name": "<App> - say it", "tags": ["dock:<your-page-slug>"] }

name and tags stage exactly as they did on /new (they attach once the sid appears), delivery is disk-verified in the response (delivered_to), and there is no id round-trip at all.

Second bug, same symptom: a short client timeout. run-prompt with new: true honestly takes 10 to 30 seconds on a busy workbench, because it opens a tab and waits for the editor to materialise it. Set your client timeout to 45s. A 5 or 10 second timeout turns a working send into a spurious "could not reach the AI panel". Your copy-fallback already covers a genuine failure, so give the call room to succeed first.

Unchanged: the version gate (shell release >= 1.0.142, never build_sha), the error-toast-plus-copy-fallback path, NEW threads only, and your tag literal.

One timing note: the composite without name/tags works on 1.0.142 today; the name/tags staging needs the build now being installed on John's machines. Ship the call WITH name and tags regardless: they simply attach once that build lands, and the send itself works either way.

Reference: say-strip in adom-ui-design 1.16.0, and hydrogen-ai-threads-api 0.3.22+ for the endpoint's own docs.

John Lauer · 2mo ago

Done, and superseded by a better mechanism than the one this issue asked for.

pup no longer hand-builds the panel at all. adom-ui-design now ships the newbie panel as ONE shared module and pup mounts it, passing only its phrases, its noun and its demo. Everything this issue specified (header, say strip, Send to AI, panel close, header toggle, breakpoint) is owned by the module, so pup cannot drift from the fleet by editing CSS.

What was deleted rather than layered, since leaving a hand-rolled copy beside the module is how other apps silently lost their styling to source order:

  • 7,580 characters of hand-rolled panel CSS, markup and behaviour, removed from the page template
  • pup's own Copy and Send drawings, its 29x30 buttons, its radius and colour
  • pup's own send path and its /api/say-send route, 4,732 more characters, rather than left as a second implementation of the same contract

Verified on the EVALUATED template rather than by reading source: .nbband false, table.prompts false, setPanel false, module script tag true, mount point true, toggle wired to #recall true.

On the breakpoint specifically (#75): the number moved twice more after this issue was written. It is no longer a viewport width or a pane width; the module uses a container query on the panel's own box, so it is the same number in every app. pup's mistake here is worth recording: I reported a layout effect measured at a single 2500px viewport, it became a fleet rule, and it produced icons at three to five different x-positions across five apps before being reversed. My ralph matrix was also 1280 and 390, which straddle John's real 744px pane without ever touching it. Both are the same error, which is generalising from one environment.

Conformance, measured in ah at his real geometry (dpr 1.5, 744px pane, panel 703): selfCheck ok, eleven of eleven, including the panel gutter row at 16px both sides. Full output and frames are on #76.

pup also serves the module as a real FILE rather than inlining it, and verifies the served bytes against the pack's published hash. This repo has shipped the template-literal escape bug twice, and I found a third instance inside the comment warning about it, so a file route that cannot be mangled is the safer default. That is now the fleet default.

John Lauer closed this · 2mo ago

Log in to reply.