feat(chrome-client): add on-page annotation indicators with bidirectional navigation - #225
feat(chrome-client): add on-page annotation indicators with bidirectional navigation#225songsterq wants to merge 6 commits into
Conversation
|
Automated reminder: thanks for the PR! This branch currently has a merge conflict with the base branch. When you get a chance, please rebase onto (or merge) the latest base branch, resolve the conflict, and push. After that, checks will re-run and the PR will get looked at again. Noted for lavish-axi#225 at |
|
Speaking as Kun's firstmate: Holding this. On-page annotation indicators (SDK dots on annotated elements plus a Conversation-panel Required CI is green (no-mistakes + build-and-test). That is not a merge vote on a dirty default-chrome change. |
|
Speaking as Kun's firstmate: Holding this. On-page annotation dots plus a persistent Conversation-panel list are default chrome UX: they appear for every annotated element without an opt-in. That matches "point at it and be understood," but I cannot auto-merge a default chrome change. The branch is CONFLICTING with main (5 ahead / 20 behind). I am not rebasing a PR that needs that call, and I am not flagging it while DIRTY. Required CI was green on this head the last time it ran. |
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (3): Last reviewed commit: "no-mistakes: apply CI fixes" | Re-trigger Greptile |
|
Speaking as Kun's firstmate: HOLD, waiting on you. Head VISION (inspected
This is still a default chrome/artifact-badge change. I cannot auto-merge it even after a green raise. I am not flagging that call while no-mistakes is still red. Not waiting on the captain. |
Annotated elements previously left no trace in the artifact, so a reviewer had no way to see what they had already marked up or to navigate between an annotation and its target. Sent annotations now persist as a durable session record and render in the conversation panel, and each annotated element carries a badge in the artifact. The two surfaces navigate to each other: clicking a badge scrolls to and highlights the matching panel row, and clicking a panel row scrolls the element into view and flashes a marker around it. Notes on two details that are easy to get wrong: - createSdkJs hand-declares the artifact-sdk.js helpers it inlines into the browser bundle. A helper called from createArtifactSdk but missing from that list is a silent ReferenceError with no build or type error, so a test now asserts every called helper is actually inlined. - The reveal marker is position:fixed while scrollIntoView settles asynchronously under `behavior: "smooth"`, so a rect read in the same tick strands the box over the element's old position. The marker re-reads its rect on a frame loop for the duration of the pulse instead. A headless-Chrome test pins this behaviorally by comparing live rects after the scroll settles and again after a later scroll; against the one-shot read it drifts by 1351px.
…or auto Object.entries change
4df93bb to
a6f0a9c
Compare
|
Speaking as Kun's firstmate: HOLD, waiting on you. Head is now VISION is unchanged: on-page |
What Changed
src/artifact-sdk.js,src/chrome-client.js,src/chrome.css).SessionStore.queuePromptsnow records non-message annotation prompts into a newsession.annotationsarray (separate from the write-onlysession.promptsoutbox and fromsession.chat), andnormalizePromptpreserves a prompt'sidso the client can correlate dots with panel rows;createChromeHtmlbootstrapsinitialAnnotationsfrom stored session state (src/session-store.js,src/server.js).createSdkJsto serializesrc/artifact-sdk.js's own exports automatically viaObject.entriesinstead of a hand-kept list per helper, removing the need to updatesrc/server.jswhenever a new SDK-internal helper is added (src/server.js,AGENTS.mddoc update).test/annotation-badge.browser.test.js,test/reveal-marker.browser.test.js, plus fixtures) and updates existing unit tests (test/artifact-sdk.test.js,test/chrome-client-queue.test.js,test/server.test.js,test/session-store.test.js,test/cli-version.test.js) and README's Reviewing on a phone/feature list with a new "Annotation indicators" bullet.Risk Assessment
✅ Low: The change is a well-scoped, additive feature (on-page annotation badges + a durable sent-annotations panel) that follows established repo invariants: badge messages are gated behind the existing artifact_load_token check, the SDK helper-export mechanism keeps the emitted bundle in sync automatically (with a real executing test verifying it resolves), the durable annotation record is only appended on success and exempted from double-recording during prompt restore, and new tests exercise real behavior (a headless-Chrome pinning test, exercised chrome-client message flows, and session-store round trips) rather than asserting on source text.
Testing
All targeted unit tests (475) and the two opt-in real-browser suites for the annotation-indicator feature pass, and a live manual run confirmed the on-page dot appears on an annotated element and its queued pill appears/links in the Conversation panel; the transient click-to-flash-marker animation itself wasn't caught in a still screenshot (timing), but that exact behavior (marker lands on and tracks its element through scroll) is covered and passing in reveal-marker.browser.test.js using real bounding-rect assertions in a headless browser, so it is not left unverified.
/var/folders/__/bc7f6bmn40z4ddn7crlrmprc0000gn/T/no-mistakes-evidence/01M0PGY40YCWR8JG43AGPDRRSV/01-initial.png)/var/folders/__/bc7f6bmn40z4ddn7crlrmprc0000gn/T/no-mistakes-evidence/01M0PGY40YCWR8JG43AGPDRRSV/02-annotated-dot-and-pill.png)/var/folders/__/bc7f6bmn40z4ddn7crlrmprc0000gn/T/no-mistakes-evidence/01M0PGY40YCWR8JG43AGPDRRSV/03-pill-click-flash-marker.png)Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
🔧 **Rebase** - 6 issues found → auto-fixed ✅
src/artifact-sdk.js- merge conflict rebasing onto origin/mainsrc/chrome-client.js- merge conflict rebasing onto origin/mainsrc/server.js- merge conflict rebasing onto origin/mainsrc/session-store.js- merge conflict rebasing onto origin/maintest/chrome-client-queue.test.js- merge conflict rebasing onto origin/maintest/server.test.js- merge conflict rebasing onto origin/main🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
node --test test/artifact-sdk.test.js test/chrome-client-queue.test.js test/server.test.js test/session-store.test.jsLAVISH_AXI_BROWSER_E2E=1 node --test test/annotation-badge.browser.test.js test/reveal-marker.browser.test.jsManual: pnpm run build; ran bin/lavish-axi.js against a demo HTML file; opened the session in a real Chrome tab via chrome-devtools MCP; toggled annotate mode; clicked a paragraph and queued an annotation; verified the on-page dot badge appeared beside the element and the corresponding pill appeared in the Conversation panel; clicked the pill and verified it expanded to show the target selector and prompt textnode bin/lavish-axi.js stop (cleanup); rm -rf temp manual test dir; git status confirmed clean worktree✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.