Skip to content

Widget: page-level / unanchored comments (start a thread without a pin) - #70

Open
MateuszPaulski wants to merge 3 commits into
mainfrom
agent/issue-53
Open

Widget: page-level / unanchored comments (start a thread without a pin)#70
MateuszPaulski wants to merge 3 commits into
mainfrom
agent/issue-53

Conversation

@MateuszPaulski

Copy link
Copy Markdown
Collaborator

Page-level / unanchored comments (start a thread without a pin)

Implements the approved spec for #53: reviewers can leave general feedback about a page without first anchoring to an element.

What changed

  • Optional anchor + new anchor state. CreateThreadBody.anchor and the persisted thread's anchor are now optional. A thread born without a pin gets the distinct anchorState: "unanchored" (deliberately not "orphaned", which means lost its anchor and drives self-heal / the "anchor lost" warning). The server derives the state from the request; "orphaned" is still only reached later via updateAnchor.
  • Adapters round-trip the absent anchor (memory keeps undefined, mongo/postgres omit the key) — verified by one contract round-trip that auto-covers all three.
  • Client opts out of anchoring for page comments. The anchor runtime skips unanchored items at list ingestion, so they are never matched, re-matched, or orphan-reported (no rematch(undefined) crash, no spurious refreshAnchor).
  • Create affordance: a "Comment on this page" button in the panel-list header reveals an inline composer; on submit the thread is created with no anchor and its detail opens in the panel (its only surface). A pinless detail does not offer "return to pin" (that would arm the lost-anchor path for a pin the runtime never places).
  • Shared useCreateThread hook — both the pin flow (DraftPopover/MarkerLayer) and the new page flow now share one create path; MarkerLayer's existing tests pass unchanged.
  • Docs: ADR-0048 + architecture §5 anchorState update. Changeset added (patch, fixed group).

Scope (per spec)

Page-scoped only (scope stays z.literal('page'), pageKey still set). Global / cross-page threads, an on-page indicator for page comments, and a server guard on refresh-anchor are deferred.

Verification

TDD-first for backend. Green locally: core (120), server (141), adapter-memory (45), adapter-mongo (48), adapter-postgres (47), client (386). pnpm lint (biome ci) clean; full pnpm build succeeds.

Closes #53.

🤖 Draft opened by airside-agent — automated build of #53.

Add a "Comment on this page" affordance so reviewers can leave general
feedback about a page without first anchoring to an element. `anchor` is now
optional on CreateThreadBody and the persisted thread; a pinless thread is born
with the distinct anchorState "unanchored" and opts out of the anchoring
machinery entirely (never matched, re-matched, or orphan-reported). Both create
paths (pin and page) now flow through one shared useCreateThread hook. ADR-0048.

Closes #53.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@MateuszPaulski

Copy link
Copy Markdown
Collaborator Author

🤖 airside-agent reviewd2829f4 · 1 finding

Sev Location Finding
⚪ low .changeset/page-level-comments.md:1 Changeset uses patch; approved spec asked for minor. Request-side anchor widening is patch, but the Thread.anchor response widening to Anchor | undefined is arguably breaking for TS consumers (→ minor). Bump-level nit; the changeset itself covers the right six packages.

Spec-faithful implementation across core/server/adapters/client — TDD-first, no weakened tests, no crash on the pinless path, no critical/high/medium findings. Promote-ready pending CI.

@MateuszPaulski

Copy link
Copy Markdown
Collaborator Author

Review — verified findings

Clean implementation — mergeable as-is. One note on the explicitly deferred server-side guard:

The adapters diverge on the deferred refresh-anchor guard for unanchored threads. A direct refresh-anchor POST for an anchorState: "unanchored" thread is handled safely by the memory adapter (its updateAnchor keeps anchor absent via the t.anchor ? … : t.anchor branch), but the mongo adapter forges a malformed partial anchor: dotted $set of anchor.selectors/anchor.signals onto a doc with no anchor creates an object missing the required offset (failing Anchor zod validation on the next read), and anchorState is flipped regardless. Not reachable through the widget (the anchor runtime skips unanchored threads) and the deferral is by design per the ADR — but when the follow-up lands, put the guard server-side so all adapters are protected uniformly rather than each re-implementing it.

Minor nit: PanelListView.tsx types the submit payload as Parameters<typeof createPageComment>[0] — it resolves to exactly ComposerSubmit, which is exported and importable; naming the domain type reads better.

Changeset correctly covers all six touched publishable packages. ✅

MateuszPaulski added a commit that referenced this pull request Jul 3, 2026
Renumber this PR's ADR to ADR-0049 (0044-0047 taken on main, 0048 claimed
by PR #70) and update in-repo references. Cap GitHub issue body under the
65,536-char limit with a truncation marker so very large threads still
create an issue instead of failing with a 422.

Airside-Agent-Fix: true
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Airside-Agent-Fix: true
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@MateuszPaulski

Copy link
Copy Markdown
Collaborator Author

🤖 airside-agent: addressed "Review — verified findings" in 6227ee4 — swapped the Parameters<typeof createPageComment>[0] annotation in PanelListView.tsx for the named ComposerSubmit type (identical, reads better). Left the deferred server-side refresh-anchor guard untouched per the ADR — it stays a follow-up so all adapters get protected uniformly.

@MateuszPaulski

Copy link
Copy Markdown
Collaborator Author

resolve conflicts

Resolves the conflicts between the page-level comments feature and main's
controller event bus, PanelListView branding/icon refactor, and ADR-0049:
useCreateThread now emits via controller.emit({type:'created'}), PanelDrawer/
PanelListView carry both provenance and branding props, and ADR-0048 and
ADR-0049 both land in docs/adr.md.

Airside-Agent-Fix: true
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@MateuszPaulski

Copy link
Copy Markdown
Collaborator Author

🤖 airside-agent: addressed "resolve conflicts" in 2f2d563 — merged main into agent/issue-53, resolved 5 conflicts (incl. adapting useCreateThread to the new controller event bus); build/lint/tests green, PR now mergeable.

MateuszPaulski added a commit that referenced this pull request Jul 3, 2026
Resolves the docs/adr.md conflict: main landed ADR-0049 (which reserves 0048
for PR #70), so this PR's entry takes the next free slot, ADR-0050, keeping
newest-last order. Updates the SKILL.md cross-references to match. Also fixes
the import-order lint error inherited from main's PanelListView.tsx (biome
organizeImports autofix) so the branch lints green.

Airside-Agent-Fix: true
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

Widget: page-level / unanchored comments (start a thread without a pin)

1 participant