Commit Graph

24 Commits

Author SHA1 Message Date
Michael Ramos e181b824cc Mobile-safe plan and code comment composition (#1297)
* feat: harden mobile comment composition

* fix(ui): keep mobile app inside Safari viewport

* docs: record physical mobile triage

* fix(ui): extend plan canvas behind Safari controls

* fix(ui): let mobile plans drive Safari chrome

* fix(ui): release Safari top edge on mobile plans

* docs: triage mobile feedback and close phase 1b

* fix(ui): harden compact touch behavior
2026-08-13 08:58:55 -07:00
Michael Ramos 7ad4d39ed9 feat(comments): reference agent skills with / or $ in plan review and annotate comments (#1229)
* feat(comments): reference agent skills with / or $ in plan and annotate comments

Typing / or $ at the start of a word in the document-UI comment composer
opens a picker of the user's global agent skills (~/.claude/skills,
~/.codex/skills, ~/.agents skills roots), served by a new GET /api/skills
on the plan and annotate servers in both runtimes (Bun + Pi mirror).
Multiple references per comment are supported; references live in the
comment text itself and are appended to exported feedback as a
'Skills referenced' block so the acting agent knows which skills to apply.

Human-invocation-only skills (disable-model-invocation: true frontmatter)
stay listed and selectable but render dimmed with a badge, warn in the
menu and composer, and are marked in the export so the agent is never
asked to invoke something it cannot.

Discovery reuses the review-skill loader (same roots, precedence, and
skip-and-log discipline), reads only an 8KB head per SKILL.md, caps the
catalog at 500 skills, takes no client input, and is never persisted;
any failure degrades to plain typing.

* fix(comments): harden skill references per review (trigger, IME, seam, fail-closed frontmatter)

Blockers:
- B1: a trigger now requires at least one query character. A bare / or $
  no longer opens the catalog, so Enter stays a newline and Tab still
  leaves the field ("This costs $" + Enter, "cd /" + Tab, bullets).
- B2: the menu ignores keys mid-IME-composition (nativeEvent.isComposing),
  matching the 16 existing guards; Enter committing a Pinyin/Telex/Korean
  candidate can no longer insert a skill.
- B3: the catalog request is a host seam (skillCatalogTransport via
  configurePlannotatorUI), defaulting to the existing GET /api/skills.
- B4: resetSkillCatalogCache() invalidates outstanding requests
  (generation counter), and a late-resolving stale request can no longer
  overwrite a newer cached value or the export registry. The catalog
  tests reset in beforeEach, so they hold in any file order.

Also:
- F1: skillReferences={false} is fully inert — the human-only notice memo
  and the cache seed are gated on the prop.
- F4: frontmatter flag parsing no longer fails open: trailing YAML
  comments are stripped, on/1 (and TRUE/yes etc.) read as true, the head
  read is 64KB, and truncated unterminated frontmatter fails CLOSED on
  disable-model-invocation.
- F5: extraction ignores markdown link destinations ([x](/name)), shell
  redirects (cat /x > out), and /-triggered FHS root names (/run, /tmp);
  menu insertion switches / to $ for those names so inserted references
  always survive extraction.
- F6: the 500-skill cap slices after sorting, so which skills survive no
  longer depends on readdir order.
- F3: /api/skills wiring guards for the Bun and Pi plan + annotate
  servers (skills-endpoint.test.ts).
- Keyboard state machine tests against the real CommentPopover in
  happy-dom (bare trigger, insertion, composition, Escape, highlight
  bounding, opt-out inertness), added to the CI DOM step.
- The insertion path dismisses the trigger start so the menu close is
  ordering-safe against React's select-plugin re-reading a stale caret.

* feat(comments): redesign the skill reference menu (bare triggers, no preselection, highlighted tokens)

Per maintainer direction, reversing the earlier bare-trigger opt-out
deliberately: typing a bare / or $ at the start of a word now opens the
full skill catalog immediately, and the safety story moves from the
trigger to the menu itself.

No preselection (the load-bearing rule): the menu opens with NO row
active, and while nothing is active every key behaves exactly as if the
menu were closed. "This costs $" + Enter is a newline; "cd /" + Tab
leaves the field (the proven regression that must never return). A row
activates only via ArrowDown/ArrowUp (Down from none lands on the first
row, Up on the last); only then do Enter/Tab insert. Pointer hover never
activates a row, because the menu floats exactly where the mouse rests
over the composer; a click inserts directly and never arms Enter.
Continuing to type re-filters and disarms any active row. Escape clears
the active row and dismisses when the user engaged (query typed or row
active); an unengaged bare-trigger menu passes Escape through so closing
the composer still costs one press.

Menu redesign to the reference look: icon, bold name, dimmed inline
description with ellipsis, right-aligned source column (Agents / Claude
/ Codex from the discovery roots), rounded generously padded rows, and a
subtle active-row background; human-only rows stay dimmed with their
badge and the warning now shows while such a row is ACTIVE.

Inserted references render highlighted in the composer via a mirrored
aria-hidden overlay behind a transparent-text textarea (identical font,
padding and wrapping metrics; scroll synced; tokens change color and
background only, drawn from the --primary theme token so every palette
works in light and dark). The caret keeps --foreground, selection uses a
translucent primary wash, and IME composition temporarily restores
native textarea text so composition underlines render normally.
skillReferences={false} still renders the plain pre-feature textarea.

Also, per review:
- extraction: dropped the over-broad shell-redirect exclusion (false
  negatives on prose like "use /animate <- this one"; the motivating
  case stays covered by the reserved-path rule)
- frontmatter: an unterminated frontmatter block now fails CLOSED on
  disable-model-invocation even in complete (untruncated) files
- the reserved-path / to $ insertion switch stays: extraction still
  reads /run as a path, and the new token highlight makes the switch
  self-explanatory (an unhighlighted insert would look broken)

The composition guard, transport seam, catalog generation counter,
enabled gating, and export rules are unchanged and re-covered by the
rewritten DOM test matrix.

* fix(comments): give the skill reference menu adaptive, viewport-clamped placement

The menu rendered bottom-full with a fixed max-h-64: always upward, up to
256px, with no viewport awareness. With the comment popover near the top of
the viewport (annotating near the top of a document), typing a trigger ran
the menu off the top of the screen with its upper rows unreachable.

Placement now mirrors the popover's own computePosition idiom: measure the
space above and below the composer wrapper against window.innerHeight,
prefer above (the shipped direction; keeps the action row and human-only
notice visible), flip below when the list fits below but not above, and when
neither side fits pick the roomier side. The list's max height is clamped to
the available space (still capped at the former 256px), so the menu never
extends past a viewport edge. Recomputes on every commit (drag moves,
popover flips, filtering changing the item count, warning-footer toggles)
plus capture-phase scroll and resize listeners, matching the popover's
tracking. Visual design of the menu and rows is unchanged.

* feat(comments): inject human-only skill instructions into exported feedback

A human-only skill (disable-model-invocation: true) referenced in a review
comment used to export as a dead name the agent could do nothing with. A
human referencing a human-only skill IS the human invocation, so the export
now injects the skill's SKILL.md body verbatim (frontmatter stripped) inside
clearly delimited BEGIN/END SKILL INSTRUCTIONS markers, with the absolute
skill directory and the resolve-relative-paths pointer so references/,
scripts/, and assets/ stay actionable. Model-invocable skills keep exporting
as names the agent can invoke itself.

Transport is lazy: a new GET /api/skills/content?name= endpoint (Bun and Pi)
serves one discovered skill's body, capped at 20k chars with an explicit
truncation notice pointing at the file; the client fetches contents only for
the human-only skills actually referenced, keyed off comment state, and the
catalog now carries each skill's absolute dir so every failure path (deleted
skill, unreadable file, race with submit) degrades to naming the skill plus
its directory. Names are matched against discovery only and never used as
paths, so traversal cannot escape the skill roots. A per-export dedupe
injects each skill once even when several comments reference it, and
GLOBAL_COMMENT annotations run through the same block.

The referenced-skills header now says the reviewer is asking for the
invocation, and the human-only menu footer and composer notice explain that
the skill's instructions will be included with the feedback instead of
warning that the reference will not work.

* polish(comments): quiet, progressive human-only skill treatment

The human-only surfaces shipped with too much emphasis: a dimmed row plus
a bordered uppercase badge, an amber warning footer, and a persistent
amber notice in the composer after insertion. Human-only is a property of
a skill, not an error state, so the treatment is now quiet and
progressively disclosed:

- Menu rows render at full strength with a small muted 'human-only' pill
  (bg-muted / muted-foreground tokens; no border, no dimming).
- The plain-language explanation (a model cannot invoke it, so its
  instructions will be included with your feedback) appears as a muted
  footer only while a human-only row is active (keyboard) or hovered
  (pointer). Hover disclosure is purely visual state local to the menu;
  it never touches activeIndex, so the no-preselection invariant and the
  hover-never-arms-Enter rule are unchanged and re-asserted by a new test.
- When not disclosed, the same sentence stays in the DOM sr-only and
  human-only rows point at it with aria-describedby, so the state reaches
  assistive tech as text rather than as a purely visual badge (this does
  not attempt the #1233 combobox semantics, and does not worsen them).
- After insertion, the highlighted token itself carries the quiet inline
  marker (a dotted primary underline; text-decoration cannot move glyphs,
  so overlay alignment is untouched) and the standing amber notice is
  replaced by a native <details> disclosure: a single muted 'Includes
  skill instructions' summary line that expands to the full accurate
  sentence, operable by pointer, keyboard, and AT alike.

No amber remains; every color is a theme token (muted, muted-foreground,
border, primary, ring), so the treatment follows every palette in light
and dark. Copy is unchanged where it was accurate. Behavior is unchanged:
human-only skills stay selectable and injection still happens.

* fix(comments): harden human-only skill injection per adversarial review

Three findings on the injection path, each with tests that fail pre-fix:

1. Marker forgery: an injected SKILL.md body containing our own
   `--- BEGIN/END SKILL INSTRUCTIONS ---` markers (or an
   `[Instructions truncated:` notice) could close the block early — making
   everything after it read as the reviewer's own words — forge a block for
   a skill nobody referenced, or forge a truncation notice pointing at an
   attacker-chosen path. Body lines matching the structural marker forms
   (leading-whitespace and case variants included) are now visibly
   neutralized before injection: kept verbatim but prefixed, never silently
   deleted (neutralizeSkillMarkerLines).

2. Forged human invocation: POST /api/external-annotations is
   unauthenticated on localhost, so any local process could submit a
   comment referencing a human-only skill and cause its instructions to be
   injected "at the reviewer's request". Annotations carrying a `source`
   now still LIST their skill references but never cause verbatim
   injection — human-only references fall back to naming the skill plus
   its directory, with an honest reason. The content-prime effect skips
   external texts for the same reason. A human referencing a human-only
   skill IS the human invocation; a tool is not.

3. Unbounded read: readReferenceSkillContent read the whole SKILL.md
   before slicing to the 20k cap, so an unauthenticated no-cors fetch loop
   could balloon RSS by file size per request (measured +64.4MB for a 64MB
   file). It now uses the same bounded readFileHead as the catalog,
   reading only frontmatter allowance + 4 bytes per capped char + slack;
   truncation detection is unchanged for any file whose frontmatter fits
   the catalog bound, and frontmatter that overflows the read falls back
   to null rather than serving raw YAML. Measured: 12 reads of a 64MB
   SKILL.md now cost +5.1MB total.

Also: the fast-fail guard no longer rejects legitimately discovered names —
`name.includes("..")` 404'd a real `v1..2` skill dir forever (and `\` is
legal in POSIX names) while defending nothing, since the name is only ever
matched against discovery output and never joined into a path. It now
rejects exactly the names that can never be a readdir entry: empty, `.`,
`..`.
2026-08-07 09:50:42 -07:00
Michael Ramos c08b188812 perf(ui): single Shiki highlighter, palette-matched code blocks, drop highlight.js (#1218)
* perf(build): stub out the dead Oniguruma WASM in every bundle

@pierre/diffs picks its Shiki engine with a runtime ternary:

    engine: preferredHighlighter === "shiki-wasm"
      ? createOnigurumaEngine(import("shiki/wasm"))
      : createJavaScriptRegexEngine()

Plannotator pins `preferredHighlighter: 'shiki-js'` (and Pierre's own
default is 'shiki-js'), so the Oniguruma branch never executes. Because
the choice is a runtime ternary, bundlers keep the `import("shiki/wasm")`
edge anyway and inline `@shikijs/engine-oniguruma/wasm-inlined`, a
~622 KB base64 blob, into the single-file HTML builds. The review app
paid for it twice: once on the main thread (via
`highlighter/shared_highlighter.js`) and once inside the `?worker&inline`
Pierre worker.

Alias `shiki/wasm` to a stub that throws if it is ever reached. Wired via
`resolve.alias` rather than a plugin because `resolve.alias` is shared
with Vite's worker build and `plugins` are not.

Highlighting output is unchanged: the JS regex engine was already the one
doing the work. Opting back into 'shiki-wasm' now fails loudly instead of
silently costing every user a megabyte of dead bytes.

    apps/review/dist/index.html  19,424,646 -> 18,180,545  (-1,244,101 raw / -463,348 gzip)
    apps/hook/dist/index.html    23,032,467 -> 22,410,416    (-622,051 raw / -233,485 gzip)

* perf(ui): consolidate code highlighting onto Shiki, drop highlight.js

The app shipped two highlighters. Shiki already tokenised the code-review
diff pane (via @pierre/diffs, JavaScript regex engine); highlight.js
separately coloured markdown fences and review suggestion snippets at
~982 KB minified for a full build of ~190 grammars. That second
highlighter is now gone.

Every call site moves onto `packages/ui/utils/codeHighlight.ts`, a thin
wrapper over Pierre's SHARED Shiki instance:

  CodeBlock, Viewer, PlanCleanDiffView   markdown fences
  InlineMarkdown                          code-file hover preview
  HighlightedCode                         review suggestion snippets

Reusing Pierre's instance rather than standing up a second fine-grained
one is deliberate. Pierre imports Shiki's full bundle, so every grammar
and theme is ALREADY inlined in the single-file builds: a separate
highlighter with a curated language list would have duplicated a subset
of bytes that are already there. Sharing costs nothing, gives every
language Shiki bundles instead of a shortlist, and — the point of the
change — guarantees fences resolve the exact same theme the diff pane
resolves.

Theming. `SHIKI_THEME_MAP` / `resolveSyntaxTheme` move from
`packages/review-editor/hooks/usePierreTheme.ts` to
`packages/ui/utils/syntaxTheme.ts`; usePierreTheme re-exports them, so
the review editor's imports are unchanged. `useFenceTheme()` feeds the
components and re-highlights on palette or mode change. Code blocks now
follow the active palette across all ~52 themes in both light and dark,
instead of always rendering github-dark and relying on hand-written
`.hljs-*` override stacks to stay legible. Those stacks are deleted:
`packages/editor/index.css`'s light-mode token palette, and
`colorblind.css`'s hand-tuned tokens which existed to APPROXIMATE
@pierre/theme's protanopia-deuteranopia themes that are now simply used.

Behaviour held fixed:

  - Language-less fences stay plain text (#1212). No auto-detection
    anywhere, including the hover preview, which previously called
    `hljs.highlightAuto`. `HighlightedCode` derives its language from
    the caller's file path; an unknown extension renders plain.
  - `applyHighlight(el, ...)` keeps the imperative `hljs.highlightElement`
    DOM contract the annotation layer reaches into, and writes plain text
    at final size first so async highlighting causes no layout shift.
    Already-attached grammars highlight synchronously — no flicker on
    cached highlights.
  - It also verifies the rendered text is byte-identical to the source
    and falls back to plain otherwise, because annotations address code
    blocks by text offset.
  - `@plannotator/ui`'s public API is unchanged: the highlighter is a
    module-level default like the package's other seams, no new props.

The `hljs` class on fenced `<code>` becomes `pn-code` (it is a
structural hook for blockTargeting, vim navigation and print.css, and it
named a library we no longer ship). `language-*` stays.

    apps/review/dist/index.html  18,180,545 -> 17,270,889  (-909,656 raw / -291,921 gzip)
    apps/hook/dist/index.html    22,410,416 -> 21,704,434  (-705,982 raw / -238,096 gzip)

Verified the diff pane is untouched: the rendered Pierre shadow-DOM
markup is byte-for-byte identical between an origin/main build and this
one (SHA-256 aa1ee88a…).

* fix(ui): strip stray NUL bytes from the code-highlight source

Two U+0000 bytes slipped into comments in the previous commit, which made
git treat the file as binary. Replaced with spaces; no behaviour change.

* fix(ui): keep code-block annotation marks across highlight swaps

Fenced code is annotated by hand: one `<mark data-bind-id>` inside the
`<code>` element, which `applyHighlight` also owns. Every highlight swap
(palette change, dark/light toggle, or the first async grammar attach
after load) replaces that element's children, so the mark was silently
wiped and nothing put it back. Annotation state, the sidebar panel and
exports were unaffected; the loss was purely visual, and deterministic.

`applyHighlight` now publishes every write through `onCodeHighlightSwap`,
synchronously, immediately after it. `Viewer` subscribes and re-paints the
fence's mark, so a swapped block ends up with BOTH the new theme's tokens
and its annotation. The shared painter (`paintCodeBlockMark`) moves the
token spans into the mark instead of flattening them to text, so creating
an annotation no longer costs a block its colours either.

Being driven by the swap also fixes the cousin race by ordering rather
than timing: share/draft restore runs on a timer after load, and on a slow
machine the first async swap could land after it and wipe the restored
marks per block. A restore that painted before the swap is now
re-established in the same task the swap ran in, and one that runs after
finds the mark already there.

Removal tombstones the id before re-highlighting, because the host drops
the annotation from state a tick later — without it the swap listener
would paint the just-removed annotation back in, and a fence carrying a
second annotation would end up bare.

Also closes the named gap in the WASM coverage: entry-assets only grepped
source, so a future @pierre/diffs bump could reintroduce the inlined blob
through a different import specifier unnoticed. It now greps the built
`apps/{review,hook}/dist/index.html` for the base64 WASM magic, skipping
on an unbuilt checkout and running for real in the CI job that builds the
bundles.
2026-08-05 21:54:40 -07:00
Zeke Sikelianos c072a16ca6 fix(ui): render language-less code blocks as plain text (#1212)
Bare fenced code blocks (no language tag) were passed to
hljs.highlightElement() unconditionally, so highlight.js auto-detected
a language and colored ordinary words. Skip highlighting when the block
has no language so it renders as plain monospaced text.

Fixes #1210
2026-08-05 19:45:10 -07:00
Ben Newman 6b542da8b9 feat(annotate): extend per-file version diff to folder sessions (#1105)
* refactor(annotate): extract per-file version history into a shared helper

Move the single-file annotate-history pipeline (slug derivation,
saveToHistory, previous-version lookup, degrade-on-error) out of the
Bun-specific annotate server and into packages/shared/annotate-history.ts,
built on node:fs/node:path/node:crypto only so other runtimes can vendor it
unmodified.

annotate.ts now calls computeAnnotateHistory() instead of inlining the
pipeline; behavior for single-file sessions is unchanged.

* feat(annotate): extend per-file version history to folder annotate sessions

Eligible folder files served through /api/doc now get snapshotted into the
same version history the single-file flow uses, and their doc responses
carry the same previousPlan/versionInfo/diffCurrent fields /api/plan already
returns for single-file sessions. The pipeline runs lazily on first open and
is memoized per resolved absolute path for the life of the server, so
reopening a file never re-snapshots it.

Eligibility mirrors the single-file source-save gates: a local file under
the session's folder root, markdown-branch documents only (.md/.txt, not
HTML, not a Turndown-converted doc), under the existing 2MB annotatable-file
cap, and gated by the same annotateHistory config toggle. Storage failures
degrade to a plain render (never a gate on the request) via the same
try/catch computeAnnotateHistory already wraps.

/api/plan/version and /api/plan/versions gain an optional path (+ base)
query param so folder sessions can ask for a specific file's history; the
slug is always derived server-side from the resolved, containment-checked
path — never accepted from the client, since it gets joined unsanitized
into a filesystem path. Omitting path keeps today's single-session-binding
behavior unchanged.

* test(annotate): cover folder annotate version history

Adds a new describe block exercising the folder-mode history pipeline added
in the previous commit: first-open snapshot + same-session memoization,
storage-level dedupe, cross-mode slug continuity with the single-file flow,
first-ever-open field shape, the config toggle, an ineligible (HTML) file
type, degrade-on-unwritable-history-dir, and the path-parameterized version
endpoints (including containment rejection and the no-path fallback).

* feat(ui): add a docKey seam to usePlanDiff for per-document resets

usePlanDiff's diff-base state (diffBasePlan, diffBaseVersion, versions, ...)
was seeded once from its constructor args and only ever synced later via a
"still falsy" guard - fine for a single root document, but switching to a
different document (a different previousPlan/versionInfo) would silently
keep the previous document's diff base around instead of adopting the new
one's.

Add an optional docKey param identifying which document the current
previousPlan/versionInfo belong to. When it changes between renders, reset
diffBasePlan/diffBaseVersion/versions (and in-flight loading/selecting
flags) to the newly-provided values. Omitting docKey (or keeping it stable)
preserves exactly today's one-time-hydration behavior, so the root
document's call site is unaffected until it opts in.

No caller passes docKey yet - this is purely additive.

* feat(ui): carry a per-document version-diff baseline through useLinkedDoc

/api/doc now returns previousPlan/versionInfo/diffCurrent for eligible
folder files (same shape /api/plan already returns for single-file
sessions). Extend LinkedDocLoadData with those fields and carry them
through the same activate/cache/back lifecycle annotations and markdown
already use, so a document's diff baseline:

- is captured once when the document is first opened
- persists in the per-filepath cache across back()/re-open, instead of
  being lost or needing a re-fetch
- resolves cache-first via the new resolveDiffBaseline helper, gated on
  whether a baseline was ever captured (versionInfo presence) rather than
  truthiness of previousPlan - a document at its first-ever version
  legitimately caches previousPlan: null, which is a resolved fact, not a
  cache miss

The hook exposes the active document's baseline as diffPreviousPlan/
diffVersionInfo, both null when no document is active or the active one has
no eligible history (every non-folder linked doc, since /api/doc never
populates these fields for those).

Not yet consumed by App.tsx - purely additive.

* feat(editor): render folder-doc version diffs via the active document

Folder annotate's version-diff UI (inline PlanDiffViewer blocks, the +N/-M
badge, and the Version Browser) was root-document-coupled: usePlanDiff was
fed only the root's previousPlan/versionInfo, and every render site keyed
off linkedDocHook.isActive to blank out the badge/version tab whenever any
linked or folder document was open.

Wire the two new per-document seams together instead:
- Feed usePlanDiff the active document's own previousPlan/versionInfo/
  filepath (falling back to the root document's when none is active), using
  the document's filepath as usePlanDiff's new docKey so switching documents
  resets the diff base instead of inheriting the previous one's.
- Add per-document fetchers (fetchVersion/fetchVersions with
  &path=<filepath>) so selecting a base version or listing versions targets
  the active document's own history, not the session-bound bare endpoints.
- Replace the root-only versionInfo/showVersionsTab reads with the active
  document's, so the Version Browser now reflects whichever document is on
  screen (previously it kept showing the root document's versions while a
  linked doc was open).
- Drop the blanket "linkedDocHook.isActive ? null/false : ..." suppression
  at the Viewer callsite and in DocBadges - planDiffStats/hasPreviousVersion
  already resolve to the active document's own (possibly absent) diff data,
  so the badge now shows for folder docs with history and stays hidden for
  every other document exactly as it did before.

Root-document behavior (single-file, plan, review, HTML surfaces) is
unaffected: none of those ever set a docKey or have an eligible document
history, so they fall through to the same defaults as before.

* feat(pi): extend per-file version history to folder annotate sessions

Mirrors the Bun runtime's folder annotate history support
(packages/server/annotate.ts + reference-handlers.ts) in the Pi Node server:

- Vendor the shared annotate-history helper (deriveAnnotateHistorySlug,
  computeAnnotateHistory) from packages/shared into generated/ via
  vendor.sh, and delegate the single-file version-history pipeline in
  serverAnnotate.ts to it instead of the hand-duplicated inline block.
  Behavior for single-file sessions is unchanged.
- Eligible folder files served through /api/doc now get snapshotted into
  the same version history the single-file flow uses, and their doc
  responses carry the same previousPlan/versionInfo/diffCurrent fields
  /api/plan already returns. The pipeline runs lazily on first open and
  is memoized per resolved absolute path for the life of the server, so
  reopening a file never re-snapshots it.
- /api/plan/version and /api/plan/versions gain an optional path
  (+ base) query param so folder sessions can ask for a specific file's
  history; the slug is always derived server-side from the resolved,
  containment-checked path (resolveAllowedDocPath in reference.ts) —
  never accepted from the client.

* test(pi): cover folder annotate version history

Adds apps/pi-extension/server/annotate-history.test.ts, the Node mirror of
packages/server/annotate.test.ts's folder-history describe block: first-open
snapshot + same-session memoization, cross-mode slug continuity with the
single-file flow, the config toggle, an ineligible (HTML) file type,
degrade-on-unwritable-history-dir, and the path-parameterized version
endpoints (including containment rejection and the no-path fallback).

History writes land in the real ~/.plannotator data dir rather than a
per-test PLANNOTATOR_DATA_DIR override: generated/storage.js caches its
data directory in a module-level constant at first import, so a per-test
env var override taken after that point silently no-ops. Each test uses
its own unique project namespace instead, same approach as the Bun-side
suite.

* ci: run docKey/linked-doc DOM tests in CI

usePlanDiff.test.tsx and useLinkedDoc.test.tsx use the test.skipIf(!hasDom)
pattern but were never added to the DOM_TESTS step, so they silently
skipped under CI's plain `bun test` and never actually ran.

* refactor(annotate): drop diffCurrent from the folder /api/doc path

diffCurrent equals the document's own markdown and the client never reads
it off /api/doc — it only exists on /api/plan for legacy single-file
shape parity, which is untouched. Stop merging it into folder /api/doc
responses and stop retaining it in the per-launch folder history memo
(Bun and Pi), and drop the now-unused field from LinkedDocLoadData.

- packages/server/reference-handlers.ts: new FolderAnnotateHistory type
  (AnnotateHistoryResult minus diffCurrent); applyDocOptions no longer
  copies diffCurrent onto the response
- packages/server/annotate.ts: the folder memo now stores/returns only
  slug/previousPlan/versionInfo
- apps/pi-extension/server/reference.ts + serverAnnotate.ts: mirrored
  changes for the Pi runtime
- packages/ui/hooks/useLinkedDoc.ts: removed the unused diffCurrent field
  from LinkedDocLoadData

* test(annotate): stop leaking history dirs; update diffCurrent expectations

The folder annotate history tests (Bun and Pi) minted a fresh project
namespace per test but never cleaned up, leaving hundreds of directories
under the real ~/.plannotator/history over repeated runs. Track every
minted project and remove its history directory in afterAll — this also
covers the stray non-directory artifact the "unwritable data dir" test
deliberately plants inside its own project's history dir, since removing
the project dir recursively takes it with it.

Also update the two assertions that expected diffCurrent on the folder
/api/doc response: that field is no longer propagated on the folder path
(see the preceding diffCurrent-removal commit), so both now assert its
absence instead.

* fix(ui): remember per-document diff-base selection across navigation

usePlanDiff reset diffBasePlan/diffBaseVersion to the newly-provided
document's defaults on every docKey change. That discarded a manually
selected base version when navigating away from a document and back
(e.g. root -> linked doc -> root), regressing behavior upstream relied
on keeping (nothing reset the selection before this seam existed).

Track each docKey's selection in a ref-held Map (keyed by docKey,
including null for the root document) and restore it on return instead
of re-seeding defaults; a key visited for the first time still seeds
from its own initialPreviousPlan/versionInfo exactly as before, and
selections never leak between distinct keys.

Adds two DOM-gated tests: restoring a manual selection after a detour to
another document, and confirming distinct docKeys don't leak into each
other.

* fix(annotate): match folder history eligibility to the single-file plain-text set

The folder /api/doc history gate was a hardcoded /\.(md|txt)$/i in both
runtimes, so any other annotatable plain-text file (.mdx, .yaml, .json,
.toml, ...) opened via a folder session silently skipped snapshotting —
breaking the cross-mode continuity this feature advertises (a .yaml with an
existing single-file version thread showed no diff when opened via its
folder).

Reuse the canonical predicate instead: isAnnotatableTextPath
(ANNOTATABLE_TEXT_REGEX in @plannotator/core/annotatable), the exact set the
single-file pipeline snapshots. HTML stays deferred and .env stays excluded,
both by that same definition. Tests extended in both runtimes: .mdx mints on
first open, .yaml single-file history serves as the folder baseline, .env
mints nothing, .html unchanged.

* feat(ui): label the folder diff badge with its baseline

The in-file version-diff badge in annotate/folder sessions shows +N/-M
against the file's last-reviewed snapshot, while the git badges in the file
tree count uncommitted-vs-HEAD — same numbers, different baselines. Give the
badge an optional baseline suffix and tooltip override (PlanDiffBadge
baselineLabel/baselineTooltip, threaded through DocBadges, Viewer, and
StickyHeaderLane) and have annotate mode pass 'since last review' /
'Changes since you last reviewed this file'. Plan review passes nothing and
renders byte-identically to before. DOM tests cover both the labeled and the
unchanged default rendering.

* fix(editor): exit diff view when the active document loses its baseline

Follow-up to the per-document diff baselines: with diff view active on file
A, opening a history-less file B left isPlanDiffActive latched on — the diff
viewer could not render for B, but the stale flag hid the annotation
toolstrip and sticky header until the user pressed Escape. Auto-exit the
diff view whenever the active (non-HTML) document has no baseline. The
--render-html surface is explicitly gated out: its diff view is driven by
htmlDiffHtml with usePlanDiff fed nulls, so hasPreviousVersion is always
false there and auto-exiting would kill the HTML diff toggle. Plan review is
unaffected — the root document's baseline never goes false mid-session. DOM
tests cover the exit, the keep-active document switch, the HTML gate, and
the no-baseline activation snap-back.

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-26 15:33:18 -07:00
Michael Ramos 070d9a5f6d Make the document UI reusable as published building blocks (#957)
* docs(adr): revert failed document-ui cutover, add ADR 004 with corrected reuse plan

The document-ui extraction/cutover (ADRs 002/003) was an AI-driven rewrite that
broke the app; the code was reverted. Add ADR 004 as the source of truth: share
@plannotator/ui as published building blocks for the Workspaces app, keep
Plannotator's app unchanged, gate on human-verified parity. Banner the reverted
ADRs and point AGENTS.md/CLAUDE.md at 004 so future agents don't rebuild the mess.

* docs(adr): add verified document-ui extraction plan, supersede draft inventory

36-agent verification of the reuse inventory: confirmed the /api coupling but
found the draft missed Viewer's transitive backend call, the cookie settings
layer, 3 React contexts + identity singleton, SSE transports, and harder
packaging blockers. Adds the verified per-subsystem extraction plan with a
parity guardrail on every step; flags the draft inventory as superseded.

* docs(adr): add document-ui extraction roadmap + parity checklist

Phase 0-7 execution roadmap (safety net -> packaging -> foundation seams ->
rendering -> navigation -> comments -> extras -> publish) and the reusable
'did it break?' parity checklist run after every step. Both enforce the law:
move + decouple, never rewrite; Plannotator's experience cannot change.

* build(ui): packaging unblock for external install (Phase 1) — no runtime change

Phase 0: captured parity baseline (typecheck/test/build + shipped-bundle hashes).
Phase 1 packaging fixes to packages/ui, metadata only:
- add phantom dompurify ^3.3.3 dep (imported in sanitizeHtml/aiChatFormat, was undeclared)
- align diff ^8.0.3 -> ^8.0.4 with root
- add peerDependencies (react, react-dom, tailwindcss, tailwindcss-animate); keep as devDeps
- add files allowlist (excludes tests); remove dead tsconfig @plannotator/shared alias

Verified byte-identical: typecheck pass, 1620 tests pass/0 fail, all 3 builds OK,
shipped plan+review bundle hashes unchanged from baseline. Remaining Phase 1
blocker (@plannotator/ai + @plannotator/shared workspace:* deps) deferred pending
a publish-vs-inline decision; logged in worklog.

* feat(ui): make image URL resolution host-overridable (Phase 2, seam 1)

getImageSrc now delegates to a module-level resolver defaulting to the verbatim
Plannotator /api/image logic; add setImageSrcResolver/resetImageSrcResolver so a
host (Workspaces) can resolve images via its own backend. All 5 consumers and the
signature unchanged. Verified: default URLs byte-identical, typecheck pass, 1620
tests pass/0 fail, builds OK. No Plannotator behavior change.

* feat(ui): make settings storage backend host-overridable (Phase 2, seam 2)

storage.ts cookie impl is now the default 'cookieBackend'; add setStorageBackend/
resetStorageBackend so a host (Workspaces) can persist settings via its own
storage. getItem/setItem/removeItem delegate to the active backend; the ~24
consumers and literal plannotator-* keys are unchanged. Verified: swap works,
typecheck pass, 1620 tests pass/0 fail, builds OK, theme persists across reload.

* feat(ui): make MarkdownEditor theme mode host-supplyable (Phase 3)

Add optional mode? prop; mode now mode ?? resolvedMode. Plannotator passes no
mode (App.tsx:4261) so it keeps using ThemeProvider's resolvedMode unchanged. A
host without ThemeProvider can supply mode directly. Verified: typecheck pass,
1620 tests/0 fail, builds OK, App.tsx untouched.

* feat(ui): allow hosts to opt out of code-path validation (Phase 3)

Viewer gains optional disableCodePathValidation? threaded to a new disabled? arg
on useValidatedCodePaths; when set, the /api/doc/exists probe is skipped. Default
undefined for Plannotator => validation stays on, /api/doc/exists fires exactly as
today. Verified: typecheck pass, 1620 tests/0 fail, builds OK, App.tsx untouched.
Also logs Phase 3 workflow outcome + remaining scroll/docfetch pieces.

* feat(ui): make code-file hover preview fetch host-overridable (Phase 3)

Add DocPreviewFetcher seam (default = verbatim /api/doc fetch) +
setDocPreviewFetcher/resetDocPreviewFetcher; route handleMouseEnter through it,
useCallback deps unchanged. No caller overrides it => Plannotator fetches /api/doc
identically. typecheck pass, 1620 tests/0 fail, builds OK.

* feat(ui): ship ScrollViewportProvider with the library (Phase 3 scroll)

Add render-transparent ScrollViewportProvider (createElement, keeps .ts) so the
scroll-viewport context travels with @plannotator/ui instead of living only in
App.tsx. Rewire App.tsx provider tags (3-line delta); identical tree/value/
position, sidebar TOC still reads the MAIN viewport. Fix stale OverlayScrollbars
doc-comment. typecheck pass, 1620 tests/0 fail, builds OK, eyeball: TOC tracks.

* fix(ui): disabled code-path validation should keep links clickable (self-review)

The Phase-3 disabled branch set ready=true with an empty map, which makes
gateCodePath demote every code link to plain text. Leave ready=false so the
no-validation fallback renders links optimistically. No Plannotator impact
(never disables). Logs Phase 3 completion + reusability note. typecheck pass,
1620 tests/0 fail, builds OK.

* feat(ui): make file-tree backend host-overridable (Phase 4)

Lift useFileBrowser's three backend wires (load-dir fetch, obsidian-vault fetch,
and the SSE live-watch effect moved VERBATIM) into an injectable FileTreeBackend
with default + setFileTreeBackend/resetFileTreeBackend, same pattern as the image
/storage seams. useFileBrowser() stays zero-arg; default fetch/SSE URLs identical.
Sidebar confirmed noop (zero backend wires, already reused by review-editor).

Verified: useFileBrowser.test.tsx passes 6/0 UNMODIFIED (DOM_TESTS=1), typecheck
pass, 1620 tests/0 fail, builds OK, App.tsx untouched, manual eyeball (annotate
adr/: tree loads, file-switch works, new file appears live via SSE). Plannotator
byte-unchanged. Logs two pre-existing bugs found during testing (not regressions).

* docs(adr): research + synthesis + spec for Phase 5 (comments/annotations/drafts)

Five-probe code research of the comment system. Key finding: most comment UI is
already portable (panel/popover/toolbar/highlighter prop-driven; review-editor
already reuses the hooks). Phase 5 narrows to 3 seams — draft transport (+ the
3-party generation protocol), external-annotation transport (SSE->polling, move
verbatim), and identity/authorship — plus 2 non-extraction items: renderer
coupling (document as a contract) and replies/threading (defer as a new feature).

* docs(adr): accept ADR 005 — make comments/annotations/drafts host-overridable (Phase 5)

Three seams (identity, draft transport, external-annotation transport), each
defaulting to today's behavior; renderer coupling documented as a contract;
replies/threading deferred as a new feature. Locks in the recommended choices
from the Phase 5 spec/synthesis.

* feat(ui): make annotation identity host-overridable (Phase 5 seam 1)

Add IdentityProvider + setIdentityProvider/resetIdentityProvider in identity.ts;
getIdentity/isCurrentUser now delegate to a module-level provider defaulting to
today's ConfigStore tater behavior. The ~9 author-stamp sites and 2 (me)-badge
sites delegate with zero call-site edits. No caller overrides => Plannotator
byte-unchanged. typecheck pass, 1620 tests/0 fail, builds OK.

* feat(ui): make draft persistence transport host-overridable (Phase 5 seam 2)

Add DraftTransport (load/save/remove) + getDraftTransport/setDraftTransport/
resetDraftTransport in useAnnotationDraft.ts, default = today's /api/draft fetches
verbatim. useCodeAnnotationDraft reads getDraftTransport() live. The generation
pre-increment, 500ms debounce, keepalive retry-gate, and pagehide/visibilitychange
flush stay in the hooks; getDraftGeneration() still escapes to the host. save
rejects-on-failure so the gated retry is preserved. No caller overrides =>
Plannotator byte-unchanged. shared/draft.test.ts 10/0, annotationDraftPersistence
13/0, typecheck pass, 1620 tests/0 fail, builds OK.

* feat(ui): make external-annotation transport host-overridable (Phase 5 seam 3)

Add ExternalAnnotationTransport<T> (subscribe/getSnapshot/CRUD) + setters in
useExternalAnnotations.ts; default = today's SSE->polling wire moved verbatim into
createDefaultTransport. The reducer (applyEvent), fallback-once gate, 500ms poll,
versionRef scoping, optimistic-before-await, and [enabled] gate stay in the hook.
A host (Workspaces) can implement the same event contract over Durable Objects.
No override caller => Plannotator byte-unchanged. external-annotations test green,
typecheck pass, 1620 tests/0 fail, builds OK. Logs Phase 5 completion.

* docs(adr): research + synthesis + spec for Phase 6 (versions, settings, sharing, AI)

Five-probe code research. Most of the four subsystems is already portable; the
real work is 5 seams (version fetchers + vscode-diff, config write-back, obsidian
detect, save-to-notes, AI transport) + 1 CSS move (block/raw diff classes from the
app shell into the package's theme.css). Fragile do-not-touch: the AI SSE reader
loop + epoch guards, and configStore debounce/deepMerge. Five Plannotator-only
pieces (OpenInApp, HooksTab, useUpdateCheck, useAgents/useAgentJobs) stay home.

* docs(adr): accept ADR 006 — make extras (versions/settings/sharing/AI) host-overridable (Phase 6)

Five seams + one CSS move, each defaulting to today's behavior. AI reader loop +
epoch guards and configStore debounce/deepMerge stay verbatim. Five Plannotator-
only pieces stay home. Locks the recommended choices from the Phase 6 spec.

* feat(ui): make version fetchers + vscode-diff host-overridable; move diff CSS into package (Phase 6 versions)

usePlanDiff gains optional fetchers (default /api/plan/version(s), error asymmetry
kept: selectBaseVersion alerts, fetchVersions silent). PlanDiffViewer gains optional
onOpenVscodeDiff (default /api/plan/vscode-diff). Relocate .annotation-highlight* +
.plan-diff-* block/raw CSS from editor/index.css into ui/theme.css (next to
.plan-diff-word-*) so the diff/highlights are self-styling from the package.
Verified: relocated CSS gone from index.css, present in shipped bundle (33x), diff
renders identical; typecheck pass, 1620 tests/0 fail, builds OK, App.tsx untouched.

* feat(ui): make config write-back + obsidian-detect host-overridable (Phase 6 settings)

configStore.setServerSync(fn) injects only the terminal POST /api/config; the 300ms
debounce, deepMerge batching, singleton, and eager cookie reads stay verbatim.
Settings gains optional onDetectObsidianVaults (default /api/obsidian/vaults), with
the [obsidian.enabled] effect dep + auto-select-first-vault verbatim. No override
caller => Plannotator unchanged. typecheck pass, 1620 tests/0 fail, builds OK.

* feat(ui): make save-to-notes host-overridable (Phase 6 sharing)

ExportModal gains optional onSaveToNotes (default = verbatim POST /api/save-notes);
showNotesTab = isApiMode && !!markdown kept byte-for-byte. Sharing utils already
parameterized (noop). No override caller => Plannotator unchanged. typecheck pass,
1620 tests/0 fail, builds OK.

* feat(ui): make Ask AI transport host-overridable (Phase 6 ai)

useAIChat gains a module-level AITransport (session/query/abort/permission) +
setAITransport/resetAITransport, default = the five /api/ai/* fetches verbatim. The
SSE reader loop, epoch/createRequest guards, and the supersede-abort position inside
createSession stay untouched. Capabilities + provider-resolution stay host-owned in
App.tsx. No override caller => Plannotator unchanged. ai.test.ts 97/0, typecheck
pass, 1620 tests/0 fail, builds OK.

* docs(adr): log Phase 6 completion (4 seams + diff CSS move)

* docs(adr): research + synthesis + spec for Phase 7 (carve @plannotator/core + publish)

Carve a browser-safe @plannotator/core: move the ~15 pure shared modules in,
extract types from the 3-4 node-bound ones (config/storage/workspace-status) so
nothing duplicates, shim @plannotator/shared so Plannotator's 99 import sites stay
unchanged, re-point @plannotator/ui to depend only on core, move wideMode.ts, then
publish core+ui (source-only). shared + ai stay private. Open: registry, versions,
CI job. Publish is the one outward-facing step — confirm before pushing.

* docs(adr): fold configurePlannotatorUI() front door + precompiled CSS into Phase 7 spec

Add the single typed configure() facade over the 9 global host-override setters
(zero-risk, additive) and an optional precompiled CSS bundle (smooths the
Tailwind-in-shared-lib wrinkle) to the Phase 7 publish scope. Both make the
published surface nicer to consume; neither touches Plannotator.

* docs(adr): lock Phase 7 publish decisions + carry over review fixes

Decided: ship JS as source (single internal consumer on controlled stack, no
build to maintain, no dist drift); precompiled CSS now REQUIRED (the @source glob
is fragile under pnpm symlinks); core CI typecheck node-free; pin ui->core exact.
Recorded the interrogation's carried-over Phase-5 code fixes (useExternalAnnotations
split-transport + fallbackRef reset, per-seam override tests, configStore loadFromBackend)
to do before publish.

* docs(adr): ADR 007 — carve @plannotator/core, complete settings provider, publish

Locks Phase 7 decisions: public npm; lockstep version at repo 0.21.0 (ui->core
pinned exact); JS ships as source + required precompiled CSS; core CI node-free;
ai stays unpublished-to-npm. Settings provider completed (loadFromBackend, prefetch
+sync) is now IN SCOPE — Workspaces uses the same UI settings stored in its own
backend. CI publish job wired but artifacts validated on-branch (pack + dry-run)
before merge; first publish gated. Carries the 2 override-path bug fixes + per-seam
override tests as pre-publish work.

* fix(ui): make external-annotation transport reads consistent + reset fallback on re-enable

Two override-path bugs found by the interrogation pass (both unreachable on
Plannotator's path; harden the host-override path for a real consumer):

1. Split-transport: the effect captured the transport at mount for subscribe/poll
   while the CRUD callbacks read the module global live, so a host swapping the
   transport after mount would split reads and writes across two backends. Capture
   once in a ref and use it in all four spots.

2. fallbackRef/receivedSnapshotRef were not reset on effect re-run, so an
   enabled false->true toggle inherited a stale 'already fell back' flag and
   silently stopped updating. Reset both at the top of the effect.

Plannotator unchanged: it never overrides the transport (same default singleton
captured) and enabled never toggles (reset is a no-op). typecheck clean; full
test suite shows zero delta (1605 pass / 45 pre-existing env failures, identical
with and without this change).

* docs(adr): align Phase 7 spec with ADR 007 (version 0.21.0 lockstep, CSS required, scope completeness)

* feat(core): carve @plannotator/core — move pure modules, extract node-bound types, shim shared (Phase 7 step 1)

* feat(ui): depend only on @plannotator/core — re-point all shared/ai imports (Phase 7 step 2)

* refactor(ui): relocate wideMode helper to @plannotator/ui/utils (Phase 7 step 3)

* feat(ui): add loadFromBackend settings rehydration + configurePlannotatorUI front door (Phase 7 step 4)

* build(ui): precompiled styles.css CSS build + madge circular-dep check (Phase 7 step 5)

* test(ui): per-seam override tests + configure routing test (Phase 7 step 6)

Add one override test per seam (setX(fake)→drive→assert→resetX()) for all
9 seams + loadFromBackend, modeled after the existing seam test pattern.
Fix configure.test.ts to defer mock.module() into beforeAll and restore with
captured real function references in afterAll so sibling seam test files are
not poisoned by spy replacements in the shared Bun worker module registry.

* fix(ui): apply Phase 7 review findings — version lockstep + seam consistency

- Bump @plannotator/ui to 0.21.0 (lockstep with @plannotator/core + repo, per ADR 007) [was the 1 critical review finding]
- useAnnotationDraft: route persistNow/dismissDraft save+remove through getDraftTransport() so all paths read the transport consistently (matches the load path; makes the single-global invariant explicit)
- configStore.loadFromBackend: document it must be called BEFORE init() or server values get overwritten
- packages/core/tsconfig: add explicit types:[] so the node-free invariant is first-class (verified: planted node:fs still fails TS2882)

* docs(adr): Phase 7 implementation plan (workflow-generated, durable artifact)

* fix(ui): reconcile #948 with the draft-transport seam + lockstep 0.21.1

Rebased onto origin/main (picks up #948 draft-deletion fix, the 0.21.1 bump, and
the #949/#950 editor fix). The rebase auto-merged #948's code-draft logic
(hasHadAnnotationsRef, empty-state tombstone, clearTimeout in restore/dismiss) with
the Phase-5 transport refactor cleanly — except the empty-state tombstone delete was
left as a raw fetch('/api/draft', DELETE). Route it through getDraftTransport().remove()
so a host backend tombstones its own stored draft on clear (the #948 guarantee, for
hosts). Plannotator unchanged (default transport hits the same endpoint).

Bump @plannotator/core + @plannotator/ui 0.21.0 -> 0.21.1 to match main's version
(lockstep per ADR 007).

Verified: typecheck clean, madge no-cycles, plain suite 1637 pass / 0 fail, #948
draft-clear test 3/0. (The 45 DOM_TESTS failures are the known server/network
integration tests that need a real OS env — same set on main, not regressions.)

* fix(ui): address review nits — host-path robustness + cleanups

- PlanDiffViewer: wrap onOpenVscodeDiff in try/finally so a host opener that throws
  can't wedge the VS Code button in a permanent loading state (default unaffected)
- useExternalAnnotations: (re-)capture the transport inside the effect on enable so a
  host that installs a transport before enabling annotations is honored, not the stale
  default — keeps the split-transport fix (effect + CRUD share one ref)
- configure.ts: import ServerSyncFn from configStore instead of duplicating the type
- repoint the 2 remaining @plannotator/shared test imports to @plannotator/core
- AGENTS.md/CLAUDE.md: document the new packages/core package

All host-path only — Plannotator behavior unchanged. typecheck clean, no cycles,
full suite green. Skipped (not simple/over-engineering): usePlanDiff prop->module-level
(design change), Obsidian late-bind, getSnapshot guard (inert), transport <any> (variance).

* docs: collapse 29 ADR process docs into one packages/ui/README.md

The branch had accumulated ~6,200 lines of ADR scaffolding (6 decisions, 7 specs,
10 research spikes/synthesis, 6 worklogs/roadmaps/plans) for this one effort. Replace
all of it with a single concise README that ships with the published package: what
@plannotator/ui + @plannotator/core are, why they exist (commercial reuse), how the
host-override seams work (configurePlannotatorUI), how a consumer installs/builds, and
the one rule (don't reimplement from scratch — add a seam). Repoint the CLAUDE.md banner
at the README. No code references the deleted docs; main's pre-existing adr/ docs untouched.

* docs(ui): add packages/ui/AGENTS.md guardrail + CLAUDE.md symlink

Directory-scoped agent guidance for anyone editing @plannotator/ui: don't rewrite from
scratch, add a seam (default = today's behavior, Plannotator byte-for-byte unchanged),
core stays node-free, never delete working code until human parity. Points to README.md
for the architecture. CLAUDE.md -> AGENTS.md symlink mirrors the repo root convention.

* build: remove madge circular-dep check (unmaintained)

madge is unmaintained (~3 years stale) and the check was never wired into CI, so it
was a dormant script + devDependency on a load-bearing path. Drop it: remove the
check:cycles script, the madge devDependency, and .madgerc.

The no-cycle invariant still holds by construction — @plannotator/core imports nothing
(zero @plannotator deps in its package.json), so any accidental core->shared/ui import
fails at publish-time bun pm pack (and review). No automated tripwire, but no stale
unmaintained tooling either.

* fix(ui): address review — TDZ guard, html-viewer export, doc corrections

- useExternalAnnotations: declare unsubscribe as let (not const) + guard calls, so a
  host transport that fires onError synchronously during subscribe falls back to polling
  instead of throwing a TDZ ReferenceError (Plannotator's EventSource fires async, never hit)
- package.json: add explicit ./components/html-viewer export (dir has index.ts; the
  ./components/* -> *.tsx wildcard can't resolve it, so external installers would fail)
- README: fix configurePlannotatorUI sample keys to the real option names
  (storageBackend/identityProvider/imageSrcResolver/externalAnnotationTransport)
- AGENTS.md: point the Ask-AI mapping at packages/core/agents.ts (shared/agents.ts is a shim now)

All publish/host-path/doc only — Plannotator unchanged. (#1 CSS-build font collision
deferred to publish-prep — it needs the asset pipeline + files allowlist, not a one-liner.)

* build(ui): don't bundle fonts in published styles.css — app loads fonts (review #1)

Industry standard for a shared UI package: ship theme + component CSS, let the consuming
app load fonts. Drop the @fontsource imports from styles-entry.css (the publish CSS entry);
the theme still defines --font-sans/--font-mono, and the app provides those families. Fixes
the asset-name collision (every emitted .woff2 was renamed styles.css) and shrinks the
published stylesheet 555kB -> 185kB. README documents the two-line @fontsource install.

Plannotator unaffected: its apps (editor/review-editor index.css) load fonts via their own
entry CSS — styles-entry.css is consumed ONLY by the publish CSS build.

* fix(ui): build styles.css on prepack, not prepublishOnly (review #4)

prepublishOnly doesn't run for npm pack / bun pm pack / git / file: installs, so the
package exported ./styles.css without shipping it. prepack runs on any pack, so the
stylesheet is always present. Verified: bun pm pack now emits styles.css.

* chore(ui): post-rebase reconciliation — version lockstep 0.21.3, awaitable AI abort seam

Rebased onto main (0.21.3). Bump @plannotator/core + @plannotator/ui to 0.21.3
to stay in lockstep with the repo version.

Resolve the useAIChat conflict: main added postServerAbort (an awaitable abort
that prevents session-busy races) using a raw fetch. Route it through the
AITransport seam by making AITransport.abort return Promise<unknown> instead of
void, so the host override is honored AND main's await-the-abort behavior is
preserved. Update the abort mocks in the seam/configure tests accordingly.

* fix(ui): make postServerAbort never reject regardless of AI transport

The await site in ask() relies on postServerAbort resolving so a superseding
query can proceed. main's original guaranteed this with its own .catch on the
fetch; routing through the AITransport seam delegated that guarantee to the
transport. Restore it at the call site (Promise.resolve(...).catch) so a host
override that rejects — or returns void at runtime — can't throw out of ask().

* fix(ui): address review — core import, abort sync-throw, snapshot guards

- useAIProviderConfig: import Origin from @plannotator/core/agents (was the only
  ui file still importing @plannotator/shared); drop the masking shared/* path
  alias from ui/tsconfig.json so a stray shared import now fails typecheck. The
  hook is part of the published surface — a standalone install had no
  @plannotator/shared to resolve.
- useAIChat.postServerAbort: defer the transport call into .then so a host abort
  that throws *synchronously* also can't reject (the .catch only caught async).
- useExternalAnnotations: default getSnapshot returns null (skip) on a malformed
  200 instead of coercing to []/0, so it can't clear annotations or reset the
  version cursor — restoring the pre-seam behavior.

* feat(ui): add upload + identity-editable seams for host backends

Two override points the Workspaces app needs that had no seam:

- UploadTransport (utils/upload.ts): image attachments hardcoded POST /api/upload
  with no override. Add a setX/resetX/getX seam (default = today's /api/upload,
  verbatim) and route AttachmentsButton through it. Workspaces sends bytes to its
  R2 asset API and returns the content-addressed URL.
- IdentityProvider.isEditable() (utils/identity.ts): the Settings rename/regenerate
  controls wrote to the cookie store, bypassing a host identity provider — so a
  host with server-owned identity could split one user across two author names.
  Add an optional isEditable() (default true) and hide the rename controls when a
  host returns false. Plannotator's cookie identity stays editable — unchanged.

Both wired into configurePlannotatorUI(); seam tests added; configure routing test
covers uploadTransport. HANDOFF.md updated with the Workspaces seam mapping from
the repo research (asset layer, identity, realtime, no-AI-infra, the Me
display-name backend follow-up). README publish command corrected to bun pm pack
+ npm publish.

* refactor(ui): capture sessionId synchronously in postServerAbort

Self-review: the deferred .then read sessionIdRef.current a microtask after the
guard checked it. Capture the id synchronously so the abort always targets the
session current at call time and there's no double-read.

* fix(ui): address review — seed host store, browser-safe timer type, harden abort

- configStore.loadFromBackend: seed the host StorageBackend with resolved defaults
  for keys it lacks. The constructor runs at module load (before a host installs
  its backend), so its default-seeding writes went to the cookie backend; without
  this a fresh host store was never populated and generated defaults (e.g.
  displayName) regenerated every reload. [P1, host path]
- Viewer.tsx: replace NodeJS.Timeout with ReturnType<typeof setTimeout> (2 refs)
  so a browser-only consumer compiling the published source doesn't need
  @types/node. Matches the pattern already used in configStore. [P1, published path]
- useAIChat: harden the create-session supersede abort the same way as
  postServerAbort, so a host transport that throws can't surface an unhandled
  rejection. No impact on Plannotator (default self-catches). [nit]
- .gitignore: correct stale 'prepublishOnly' comment to 'prepack'. [nit]

Plannotator behavior unchanged (it never calls loadFromBackend; the timer/abort
changes are behavior-preserving). Strengthened configStore seam test to assert
first-run seeding. typecheck clean, 1773 pass / 0 fail.

* refactor(ui): single-source the never-reject abort via safeAbort helper

Self-review: the hardened abort pattern (defer into .then + .catch so a host
transport that throws can't reject) was duplicated across postServerAbort and the
create-session supersede site — the exact drift the review flagged. Extract a
module-level safeAbort(sessionId) so both call sites share one hardened
implementation and can't diverge again. Behavior unchanged; reads aiTransport at
call time so a late override is honored.

* chore(ui): post-rebase version lockstep to 0.21.4

Rebased onto main (0.21.4, adds markdown math #878 + parser hardening). Bump
@plannotator/core + @plannotator/ui to 0.21.4 to stay in lockstep with the repo.
katex (main's math dep) merged into ui; typecheck clean, 1810 pass / 0 fail.

* docs(ui): consumer-lens handoff hardening + ADR 005

- HANDOFF.md: add supported-imports allowlist vs unsupported (hardcoded
  /api/*) list; document the annotation anchor schema, reattachment
  order, and untested stale-anchor degradation; state that the markdown
  editor cannot take CM6/Yjs extensions yet and the plan of record;
  note AI avoidability re-verified post-rebase; fix stale 0.21.3 ref.
- adr/decisions/005: record the publish-as-packages decision (packages
  over copy/vendor, core/ui split, seam-singleton pattern + SSR revisit
  condition, the law, lockstep publish model).

* fix(ui): make shipped source strict-TS clean for consumers + seam type barrel

Consumers compile the published TS source with their own compiler options,
and strict mode failed with 35 errors inside the package:
- settings.ts: satisfies SettingDef<unknown> is contravariantly illegal
  under strictFunctionTypes (33 errors) — use SettingDef<any>
- useDismissOnOutsideAndEscape: RefObject<HTMLElement> rejects React 19's
  useRef<T>(null) refs — widen to HTMLElement | null
- globals.d.ts: declare *.png / *.webp modules, referenced from each
  asset-importing component so any consumer program that includes one
  gets the ambient declarations

Also unscatter the seam contract types: configure.ts re-exports every
seam type next to configurePlannotatorUI, and ServerSyncFn is now
exported from config/index.ts (it was unreachable through the exports
map). Verified: standalone Vite consumer importing the full supported
surface passes tsc --noEmit under full strict (was 35 errors).

* fix(ui): keep KaTeX fonts out of published styles.css (back to ~187KB, was 1.6MB)

Main's math PR imports katex/dist/katex.min.css in theme.css; the
publish build (Vite lib mode) force-inlines all 60 KaTeX math fonts as
data URIs, ballooning styles.css to 1.6MB (977KB gzip) and breaking the
package's consumer-owns-fonts policy. Alias the katex stylesheet to an
empty stub in vite.css.config.ts only — theme.css stays untouched (no
rebase surface) and Plannotator's own apps, which import theme.css
directly, still bundle KaTeX as before. Hosts that render math load
katex.min.css themselves (bundler import, CDN tag, or self-hosted copy
per HANDOFF.md), which also gets them lazy font loading. Verified:
fresh build is 186.9KB / 30.8KB gzip with zero @font-face data URIs;
consumer vite build CSS drops 1.66MB -> 200KB.

* docs(ui): HANDOFF corrections from adversarial consumer review

- Math rendering section: KaTeX css/fonts excluded from styles.css by
  design; three one-time host setup options (self-hosted recommended,
  CDN tag, bundler import)
- styles.css size claim corrected (~187KB / ~31KB gzip) + strict-TS
  guarantee documented (verified against a standalone consumer)
- AI-avoidability claim made precise: configure.ts statically imports
  useAIChat for its setter; unused AI code tree-shakes to zero (bundle-
  verified) — the runtime claim holds, the static wording was wrong
- Loud warning on the loadSettingsFromBackend ordering footgun:
  configuring before hydration seeds generated defaults into the host
  backend and nothing re-runs hydration
- DraftTransport.load() tombstone-generation contract spelled out
- Seam-type barrel documented on the configure row; 'everything is
  importable' softened (some components/*.ts don't resolve via the
  *.tsx wildcard); stale diff stats refreshed

* docs(ui): math setup pointer in README + pnpm caveat on the katex bundler-import option

* fix(ui): lazy settings resolution — zero cookies on a configured host

The configStore resolved all settings eagerly in its constructor, at
module import — before a host's configurePlannotatorUI() could install
its StorageBackend — writing 17 plannotator-* cookies (including a
generated identity) onto the host origin. Resolution now runs lazily on
first settings access (get/set/init/loadFromBackend): by then the host
backend is live, so the initial reads AND default-seeding writes route
through it. A configured host gets zero cookies, ever.

Plannotator unchanged: same resolution, same cookie seeding, same
values — on first settings read (same page load) instead of at import.
New configStore.lazyInit.seam.test.ts proves the contract from a fresh
module graph; full suite + consumer strict tsc green.

* chore(ui): post-rebase version lockstep to 0.22.0

* fix(ui): round-2 review batch — dedupe asset declarations, CI seam tests, strict consumer gate, doc corrections

- components/types.d.ts: drop the *.png/*.webp declarations that
  globals.d.ts now owns — both shipping was a duplicate-identifier
  error for any consumer with skipLibCheck: false
- untrack packages/ui/styles.css (generated by prepack, gitignored;
  got scooped into the carve commit during the rebase by git add -A
  before the ignore entry existed in the replay)
- CI: the DOM test step now runs ALL packages/ui tests, so the seam
  contract tests (AI/draft/external-annotations/file-tree/inline-
  markdown) actually execute in CI instead of skipping
- new packages/ui/tsconfig.strict-consumer.json wired into root
  typecheck: type-checks the supported-import surface under full
  strict, so the consumer strict-TS guarantee can't silently rot
- HANDOFF: rot-proofed the diff stat, strict guarantee now cites the
  CI gate, CDN katex pinned-version wording, theme-vs-styles.css
  caveats (theme still imports KaTeX + needs Tailwind), Viewer
  required props, Yjs plan-of-record updated to the atomic-editor fork
- README: @source fallback wording (build entry isn't shipped)

* test(ui): make the lazy-resolution seam test deterministic

The test asserted lazy resolution on the module singleton and relied on
its test file getting a fresh module graph — an isolation assumption
that doesn't hold under all bun test orderings (CI failed with zero
observed reads because another file had already resolved the store).
Test the contract on a fresh instance instead: ConfigStore is exported
as @internal ConfigStoreForTest, the spy backend is installed before
construction, and the test asserts construction reads nothing while the
first get() resolves and seeds through the live backend. Deterministic
by construction.

* test(ui): poll for the debounced reconnect refetch instead of a fixed sleep

The reconnect-refresh assertion waited a fixed 150ms against the SSE
watcher's 120ms debounce — a 30ms margin that slower CI runners lose,
flaking 'refreshes after an SSE ready event from reconnect'. The
watched logic is unchanged (verified byte-identical to main's inline
version — the seam only relocated it into the default watchTrees and
added the onChange indirection). Poll for calls.length===2 up to 1s so
the pass/fail is hardware-independent.

* test(ui): poll the committed tree state, not the fetch call count

Prior fix polled calls.length===2, but the fetch call is counted one
tick before its result commits to React state — so the poll exited
early and the next assertion (dirs[0].tree === reconnectedTree) lost the
race on slow CI (toEqual failure). Poll on the committed tree itself,
which is exactly what the assertion checks: now the only way to fail is
a genuine no-refresh, not a timing margin.

* test(ui): give the reconnect-refetch poll a 10s ceiling + 20s test timeout

A CI runner was measured at 6x normal speed (1676ms for a ~275ms test),
blowing through the 1.5s poll ceiling before the 120ms debounce fired —
same commit passed on a faster runner. Raise the poll to ~10s and set an
explicit 20s test timeout (bun's 5s default would otherwise kill the
poll). Root cause is load, not logic: this timing-sensitive test only
started flaking when the CI DOM step was broadened to run the whole ui
suite in one process.

* ci: run the file-browser DOM test isolated; scope the DOM step to DOM files

Root-causes the intermittent 'refreshes after an SSE ready event from
reconnect' failure. The round-2 change ran the ENTIRE ui suite under
DOM_TESTS=1 to catch the seam contracts; that load intermittently
starved the test's 120ms real-timer debounce so the reconnect refetch
never fired (observed failing after a full 10s poll — not a margin
issue). The hook logic is byte-identical to main, and main runs this
test in its own process (green for months).

Fix at the CI layer, not the test: run useFileBrowser.test.tsx isolated
(matching main), and run the seam contracts + remaining DOM-gated tests
as an explicitly-scoped light batch. The test file is reverted to main
verbatim (today's timing-poll experiments dropped). Follow-up issue to
file: the underlying re-subscription race the load exposed.
2026-07-06 20:39:09 -07:00
Robert Dailey cbc6186a15 fix(ui): align list markers to first line in clean diff view (#838)
* fix(ui): align list marker to first line in clean diff view

* refactor(ui): extract list-item marker and body into ListItemBody

Three render paths (BlockRenderer and two in PlanCleanDiffView)
duplicated the same marker-plus-paragraph scaffold. A recent alignment
fix had to be applied to each surface independently, which exposed the
duplication. The shared structure now lives in ListItemBody; call sites
keep their own row wrapper with surface-specific concerns (indent, data
attributes, hover props, interactivity).
2026-06-02 11:18:52 -07:00
Michael Ramos fcf2ba4cf5 fix: indent loose list continuation content under parent bullet (#705)
Closes #704
2026-05-11 16:41:26 -07:00
Michael Ramos ba2e4d2a1d feat(ui): markdown reader parity — HTML blocks, GitHub alerts, GFM inline extras (#597)
* feat(ui): markdown reader parity — HTML blocks, GitHub alerts, GFM inline extras

Brings the in-app markdown reader to parity with GitHub's flavored rendering.
Additive across the parser + renderer; no behavior change for existing blocks.

Refactor:
- Extract InlineMarkdown (262 lines) out of Viewer.tsx into its own file
- Extract BlockRenderer + block-type components (CodeBlock, HtmlBlock, AlertBlock,
  Callout) into components/blocks/ — Viewer drops from 1279 to ~770 lines
- Each new block-level feature lands in BlockRenderer or a new blocks/*.tsx,
  not Viewer

Block-level features:
- Raw HTML blocks (<details>, <summary>, etc.) via balanced-tag parser branch,
  rendered through marked + DOMPurify for nested-markdown support; inner innerHTML
  set imperatively so React reconciliation doesn't collapse open <details>
- GitHub alerts (> [!NOTE] / [!TIP] / [!WARNING] / [!CAUTION] / [!IMPORTANT])
  with inline Octicons, title-case labels, GitHub's Primer colors (light + dark)
- Directive containers (:::kind ... :::) with arbitrary kinds for project-specific
  callouts (note, tip, warning, danger, info, success, question, etc.)
- Heading anchor ids — slugifyHeading() strips inline markdown, preserves unicode

Inline features (all in InlineMarkdown, all code-span-safe):
- Bare URL autolinks (https://...) with trailing-punctuation trimming
- @mentions and #issue-refs — render as clickable links when repo is GitHub,
  styled spans otherwise; threaded via repoInfo.display through BlockRenderer
- Emoji shortcodes (👋, 🚀, 29 curated codes) via transformPlainText()
- Smart punctuation (curly quotes, em/en dashes, ellipsis) applied only to
  plain-text fragments after code spans have been consumed

Safety:
- Render-time transforms live inside InlineMarkdown's plain-text push, which
  is only reached after code-span regex consumes code content. Backticks stay
  literal for shell/regex snippets.
- DOMPurify allowlist (no on* handlers, no style attrs, no scripts) gates every
  raw HTML block. Unsafe link protocols (javascript:/data:/vbscript:/file:)
  stripped by sanitizeLinkUrl.

Tests: +40 (149 total). New files:
- utils/slugify.test.ts (10) — unicode, markdown stripping, edge cases
- utils/inlineTransforms.test.ts (9) — emoji + smartypants
- utils/parser.test.ts — alert detection (5 cases), directives (5 cases), HTML
  block balancing (5 cases)

Fixtures for manual verification:
- tests/test-fixtures/11-html-blocks.md
- tests/test-fixtures/12-gfm-and-inline-extras.md (release-plan-shaped demo)

Known limitations (not blockers):
- Bare URL regex doesn't balance parens (https://en.wikipedia.org/wiki/Foo_(bar)
  drops the trailing ")")
- Duplicate heading text → duplicate anchor ids (browser picks first on hash nav)
- Directive body is inline-only (no nested headings/lists)

For provenance purposes, this commit was AI assisted.

* fix(ui): wire typecheck for packages/ui, address PR review findings

Root-cause fix for the missing-import bug caught in review: the UI package
had no tsconfig.json and no typecheck script, so missing references like
`getImageSrc` in the extracted InlineMarkdown slipped past vite/esbuild
(which only type-strip, they don't resolve imports).

Infrastructure:
- Added packages/ui/tsconfig.json with proper module resolution, JSX config,
  and bundler-style paths.
- Added globals.d.ts to accept side-effect CSS imports.
- Added @types/react, @types/react-dom, @types/bun, @types/dompurify as
  devDeps on packages/ui so React / Bun / DOMPurify types actually resolve.
- Wired `tsc --noEmit -p packages/ui/tsconfig.json` into the top-level
  `bun run typecheck` script.

With the typecheck running, 0 errors remain in this PR's scope. Four
pre-existing errors on main (plan-diff SVG type narrowing, sharing.ts
SharePayload shape) are unrelated and tracked separately.

Review fixes:
- InlineMarkdown: import getImageSrc from ImageThumbnail. Was calling the
  helper without importing it — markdown images with relative paths
  (`![alt](./foo.png)`) would throw ReferenceError at render. Regression
  caused by the InlineMarkdown extraction.
- useAnnotationHighlighter: findTextInDOM now retries with the rendered
  form (transformPlainText) when the raw originalText doesn't match.
  Annotations made before smart-punctuation / emoji shortcodes shipped
  (straight quotes, `👋` text) still re-bind after reload.
- sanitizeHtml: allow the `open` attribute so `<details open>` preserves
  its default-expanded state instead of always rendering collapsed.
- parser.test.ts: narrow a string->AlertKind assertion to satisfy strict
  typechecking.

Deferred (tracked as known limitation in PR description):
- HtmlBlock relative URL rewriting for nested <img src="./logo.png"> /
  <a href="note.md">. New-feature gap, not a regression.

For provenance purposes, this commit was AI assisted.

* fix(ui): HtmlBlock rewrites relative <img>/<a> refs to match markdown paths

Raw HTML blocks inject sanitized HTML verbatim, so nested <img src="./logo.png">
and <a href="notes.md"> resolved against the plannotator server URL instead of
the plan's directory — images 404'd, .md links navigated away instead of
opening in the linked-doc overlay. This is the path README.md content hits
(hero <img>, YouTube thumbnails, <details> sections with anchors).

Fix: after setting innerHTML, walk <img> and <a> elements and apply the same
rewriting markdown content uses:
- <img> relative src → getImageSrc(src, imageBaseDir), routing through
  /api/image?path=... with the plan's base directory.
- <a> relative href matching .md / .mdx / .html → click handler that calls
  onOpenLinkedDoc, same pattern as [label](./foo.md) markdown links.
- http(s):, data:, blob:, mailto:, tel:, and #anchor hrefs pass through
  untouched.

BlockRenderer now threads imageBaseDir + onOpenLinkedDoc into HtmlBlock.
React.memo equality extended to compare those props too, so legitimate
changes still re-run the rewrite pass without forcing re-renders on
every parent update.

Verified against the repo's own README.md — hero image, YouTube thumbnails,
and <details> sections all render correctly in annotate mode.

For provenance purposes, this commit was AI assisted.

* feat(ui): table conveniences — hover toolbar, popout dialog with sort/filter

Extracts table rendering into blocks/TableBlock and adds two companion
surfaces: a hover toolbar for quick copy, and a full-screen popout dialog
with TanStack-powered sort/filter/copy for power use. No pagination — plan
tables don't get that big.

Hover toolbar (blocks/TableToolbar.tsx):
- Floats above the table on mouse enter via React portal, positioned with
  getBoundingClientRect + scroll/resize listeners, entry/exit animations.
  Same positional pattern as AnnotationToolbar's top-right mode.
- Debounced hover state in Viewer (100ms leave → 150ms exit animation),
  mirroring hoveredCodeBlock's state machine.
- Three actions: Copy markdown (icon), CSV (short uppercase button,
  RFC 4180 escaping), Expand (opens popout).

Popout dialog (blocks/TablePopout.tsx):
- Radix Dialog, fullscreen-ish card with ~2rem backdrop visible for
  click-to-close. max-w-[min(calc(100vw-4rem),1500px)].
- Portaled into Viewer's containerRef so the annotation hook can walk
  into the popout's text nodes — selection-based annotations, text-search
  restoration, and shared blockId all work across the collapsed and
  popped-out views.
- TanStack Table for the grid: click column headers to sort (asc → desc →
  clear), global filter input, no pagination. Row count indicator shows
  "15 of 27" when filter reduces the set.
- Copy / CSV buttons in the header row: filter- and sort-aware. When
  visible rows < total, tooltips read "Copy 15 rows as markdown" /
  "Copy 15 rows as CSV". When no filter, copies whole table (normalized
  whitespace). Read is one-shot on click — no derived state to sync.
- Floating X close button (absolute top-right), no header bar.

Chrome stacking while popout is open (CSS-only, via :has()):
- body:has([data-popout="true"]) drops four element types behind the
  dialog: annotation sidebar, sticky header lane, app nav header, overlay
  scrollbars. :has() observes the dialog's presence directly — when the
  dialog unmounts, the selector stops matching and everything returns to
  natural stacking. No JS state, no useEffect cleanup.

Shared helpers in TableBlock.tsx (exported):
- parseTableContent — pipe-delimited markdown → { headers, rows }
- buildCsvFromRows / buildMarkdownTable — inverse, from parsed data
- buildCsv — thin wrapper for the hover toolbar's raw-block path

Dependencies added:
- @radix-ui/react-dialog ^1.1.15 (~6 KB gzipped)
- @tanstack/react-table ^8.21.3 (~14 KB gzipped)

Fixture:
- tests/test-fixtures/12-gfm-and-inline-extras.md — added a 27×11
  "Detailed feature backlog" table to exercise wide + deep tables,
  horizontal scroll in the popout, and the sort/filter flows.

For provenance purposes, this commit was AI assisted.

* fix(ui): table popout — annotation flow, chrome stacking, sidebar tabs

Tightens the popout so annotations work inside it and chrome doesn't
overlap the dialog.

Annotation flow inside popout:
- Radix Dialog modal={false} so the focus trap doesn't yank focus back
  from CommentPopover's textarea (CommentPopover portals to document.body,
  outside the dialog's DOM subtree).
- Dialog.Content onInteractOutside handler whitelists the annotation
  toolbar, CommentPopover, and FloatingQuickLabelPicker so clicking
  them doesn't dismiss the dialog. Backdrop click + Escape still close.
- aria-describedby={undefined} on Dialog.Content (Radix opt-out; the
  popout doesn't need a description).
- React.memo on TablePopout with a custom comparator (block id/content,
  open, container, imageBaseDir, githubRepo). Prevents upstream Viewer
  re-renders from re-running TanStack's flexRender on every cell, which
  conflicted with web-highlighter's live DOM mutations and caused a
  NotFoundError in React's reconciler.

Widget markers for :has()-based chrome stacking:
- [data-comment-popover="true"] on CommentPopover (both popover + dialog
  variants).
- [data-floating-picker="true"] on FloatingQuickLabelPicker.
- [data-sidebar-tabs="true"] on SidebarTabs (left-side TOC/Files/Versions
  flags that sit on top of the dialog otherwise).
- theme.css extended: sidebar tabs join the annotation sidebar, sticky
  header lane, app header, and overlay scrollbars in dropping to
  z-index -1 while body:has([data-popout="true"]) matches.

Known limitation (not addressed): annotations created inside the popout
show their <mark> only while the popout is open; when it closes, the
<mark> unmounts with the popout's DOM and does not reappear on the
collapsed table. The annotation itself persists in state (sidebar,
shared URLs, exports). Round-tripping visual marks between popout and
collapsed view requires either a second web-highlighter instance or a
switch to the CSS Custom Highlight API — out of scope here.

For provenance purposes, this commit was AI assisted.

* fix(ui): review findings — flags, alerts, forges, tabs, anchors, URL brackets

Six targeted fixes from the v0.19 PR review. Each is small and scoped;
the riskier items from the review (plan-diff block variants, HTML
relative non-doc links) are tracked as follow-ups.

Smart punctuation — CLI flags preserved:
- Narrowed the `--` → en-dash rule to only fire between digits
  (`pages 3--5` still converts; `bun --watch` stays literal).

GitHub alerts — list/code/heading bodies absorb correctly:
- Blockquote merge now always merges into a previous alert blockquote,
  regardless of whether the new line starts with a block marker. Without
  this, `> [!NOTE]\n> - item` split the list off into a plain italic
  quote and emptied the alert.
- AlertBlock got a mini block-level renderer for the body so `- item` /
  `* item` / `1. item` render as real <ul>/<ol>, not flattened prose.

Forge-aware mentions/issue refs:
- packages/shared/repo: new parseRemoteHost() extracts the host from
  the git remote URL; RepoInfo gains an optional `host` field.
- packages/server/repo: getRepoInfo populates host alongside display.
- Viewer only passes githubRepo to InlineMarkdown when the host is
  exactly "github.com". Non-GitHub repos render mentions/issue refs
  as styled text, no wrong github.com links.

HTML block external links:
- rewriteRelativeRefs now forces `target="_blank"` and
  `rel="noopener noreferrer"` on every external http(s) link inside
  raw HTML. Fixes two problems in one pass: external links no longer
  hijack the review tab, and pasted-HTML links can't tab-nab the
  plannotator tab via window.opener.

Heading anchor dedup:
- New buildHeadingSlugMap() walks all heading blocks and assigns
  `foo`, `foo-1`, `foo-2`, ... for repeats (GitHub convention).
  BlockRenderer receives the anchor id as a prop from Viewer via a
  memoized map rather than computing per-block; first occurrence
  keeps the bare slug so existing links stay stable.

URL autolink bracket balance:
- Trailing `)`/`]`/`}` in bare URLs are kept when they balance an
  earlier opener inside the URL. Wikipedia-style
  `https://en.wikipedia.org/wiki/Function_(mathematics)` now keeps its
  paren; `(see https://x.com)` still trims the orphan.

Tests: +8 (157 total).
- utils/slugify.test: buildHeadingSlugMap dedup behavior, non-heading
  skipping, empty-slug skipping.
- utils/inlineTransforms.test: CLI flags stay literal, `3--5` still
  converts.
- utils/parser.test: alerts with list body / code fence body, blank
  line ending an alert.

Fixture:
- tests/test-fixtures/13-known-issues.md — reproduces each of the
  review findings end-to-end; useful as a regression check going
  forward.

Deferred (tracked for follow-up):
- Plan diff view doesn't render html / directive / alertKind semantics
  (SimpleBlockRenderer has no cases for the new block variants).
- Relative non-doc links inside raw HTML (.pdf, .csv) don't get
  rewritten — only .md/.mdx/.html are routed through the linked-doc
  overlay today. Not a regression; narrow audience.

For provenance purposes, this commit was AI assisted.

* fix(ui): round-3 review — drop host gate, link paren balance, data/blob images

- Viewer: remove repoInfo.host === 'github.com' gate so @user/#123 links
  render for GitHub Enterprise and runtimes (Pi) that don't populate host.
- HtmlBlock: treat protocol-relative //host links as external and harden
  with target=_blank rel=noopener noreferrer.
- InlineMarkdown: data:/blob: image sources bypass /api/image rewrite.
- InlineMarkdown: replace [text](url) regex with a depth-tracking scanner
  so URLs with balanced parens (Wikipedia /Function_(mathematics)) and
  backslash-escapes no longer truncate. Empty text/url guard preserves
  prior fall-through behavior.
- InlineMarkdown: isLocalDoc accepts .md/.mdx/.html/.htm with optional
  #fragment; fragment stripped before onOpenLinkedDoc so guide.md#setup
  opens the linked doc instead of a broken anchor.

For provenance purposes, this commit was AI assisted.

* fix(ui): round-4 review — table pipe escape, callout lists, emoji h-splitter

- TableBlock: buildMarkdownTable now re-escapes literal | as \| in each
  cell. parseTableContent already unescapes on parse; without the mirror
  on serialize, the popout's copy-as-markdown produces extra columns for
  tables with pipes in regex, shell, or boolean content.
- AlertBlock + Callout: extract the shared paragraph-and-list body
  renderer into blocks/proseBody.tsx. Fixes directive callouts (:::note
  with a bulleted list) rendering as literal hyphens instead of a list.
  Paragraph lines join with '\n' so InlineMarkdown's hard-break handler
  still fires. Callout passes an empty text-color class so directive
  color tokens inherited from the container are preserved.
- InlineMarkdown: drop `h` from the plaintext chunk-break class; it was
  splitting emoji shortcodes like ❤️, 👍, 🤔 at the
  h, so the :word: pattern never reassembled and transformPlainText
  couldn't replace the shortcode. Bare URL detection moves inline via
  emitPlainTextWithBareUrls, which scans chunks for https?:// at word
  boundaries and emits anchors, passing surrounding text through
  transformPlainText so emoji + smart punctuation still apply to
  non-URL slices.
- InlineMarkdown: extract trimUrlTail (shared between the top-of-loop
  URL branch and the new inline scanner) — one balanced-paren trim
  implementation instead of two. +8 unit tests covering the trim cases
  (Wikipedia parens, unbalanced brackets, stacked punctuation).
- Fixture: section 9 in 13-known-issues.md demonstrates the table copy
  corruption for manual verification.

For provenance purposes, this commit was AI assisted.

* fix(ui): resolve pre-existing typecheck errors surfacing in CI

- PlanCleanDiffView: narrow heading Tag to 'h1'..'h6' so hover props
  resolve to HTMLHeadingElement instead of the SVGSymbolElement branch
  of keyof IntrinsicElements.
- VSCodeIcon: spread mask-type as a kebab-case attribute; React 19's
  typings no longer expose the camelCase maskType prop on SVG masks.
- useSharing / sharing: cast decompress() result to SharePayload — the
  shared compress module returns unknown by design; callers were
  implicitly any and TS 5.x now flags the assignment.

For provenance purposes, this commit was AI assisted.
2026-04-21 18:56:41 -07:00
Michael Ramos 4139999526 feat(plan-diff): word-level inline diff rendering (#565)
* feat(plan-diff): word-level inline diff rendering

Two-pass hierarchical diff (diffLines outer + diffWordsWithSpace inner)
so modified plan blocks render with inline insertions/deletions in
context instead of showing the whole old block struck-through above the
whole new block. Resolves #560.

Engine (packages/ui/utils/planDiffEngine.ts):
- computeInlineDiff runs a second-pass word diff on modified blocks
  that pass a whitelist gate (paragraph/heading/list-item with matching
  structural fields).
- Sentinel substitution atomizes inline-code spans, markdown links, and
  fenced code blocks before diffWordsWithSpace runs, so diff markers
  never land inside backticks, link hrefs, or across fence boundaries.
  Fence regex uses a backreference so variable-length (e.g., 4-backtick
  wrapping 3-backtick) fences are matched atomically.
- Annotation context for an inline-diffed modified block now captures
  both old and new content so comments on struck-through words preserve
  that text in the exported feedback.

Renderer (packages/ui/components/plan-diff/PlanCleanDiffView.tsx):
- New InlineModifiedBlock component renders a modified block as one
  structural wrapper with <ins>/<del> wrappers inside, parsed through
  the local InlineMarkdown in a single pass so markdown delimiter pairs
  survive across token boundaries.
- InlineMarkdown extended to recognize <ins>/<del> tag passthrough
  (with recursive parsing of the wrapped content) and to recursively
  parse link anchor text so diff markers inside links render correctly.
- Plain-text stop-char scanner includes '<' so <ins>/<del> dispatch
  re-enters the loop instead of swallowing tag text.
- Click-to-annotate works in every editor mode (not just comment), with
  the block-level onClick opening the popover directly.

Mode switcher (packages/ui/components/plan-diff/PlanDiffModeSwitcher.tsx):
- Adds a third "Classic" tab between Rendered and Raw. Rendered is the
  new word-level default (labeled "exp"); Classic forces the legacy
  block-level stacked fallback for every modified block.

Styling (packages/ui/theme.css, packages/editor/index.css):
- plan-diff-word-added / plan-diff-word-removed utility classes for
  inline highlights with box-decoration-break: clone across line wraps.
- Inline <code> inside the diff wrappers picks up a tinted background
  so code-pill changes read unambiguously green/red.
- New plan-diff-modified class (amber border) for inline-diff modified
  blocks, matching the GitHub/VSCode convention of green=add,
  red=remove, yellow=both.

Tests (packages/ui/utils/planDiffEngine.test.ts):
- 18 tests covering the engine's qualification gate, structural-field
  matching, sentinel round-trip (inline code / links / fences), token
  content for common edit patterns.

For provenance purposes, this commit was AI assisted.

* chore(demo): restructure default demo, add VITE_DIFF_DEMO stress test

Demo content changes that support the word-level diff work but do not
alter shipped app behavior — only what other devs see running dev:hook.

packages/editor/demoPlan.ts (default V3 editor content):
- Added a "Context" section at the top of the plan with prose that
  showcases the word-level engine in V2→V3 diff: bold phrase swap,
  inline-code pill swaps, a link URL change, and a single-line code
  edit inside a config block.
- Moved the mermaid architecture diagram and graphviz service map to
  an "Appendix: Diagrams" section at the end of the plan; they were
  rendering ugly mid-document.

apps/hook/dev-mock-api.ts (Vite mock for the diff API):
- PLAN_V1 / PLAN_V2 split into *_DEFAULT (original Real-time
  Collaboration plan — preserved identically from pre-branch state) and
  *_DIFF_TEST (the 20-case Auth Service Refactor diff-engine stress
  test, kept as an opt-in tool).
- Resolves which pair to serve based on VITE_DIFF_DEMO env var. Matches
  the V2 Context section to the new V3 Context, with differences that
  produce rich word-level inline diffs on first load.
- Diagrams moved to Appendix in V2_DEFAULT to match V3.

packages/editor/App.tsx:
- Both demo imports are active. VITE_DIFF_DEMO=1 swaps
  DIFF_DEMO_PLAN_CONTENT into the editor's default; unset renders the
  original Real-time Collaboration plan as before.

packages/editor/demoPlanDiffDemo.ts (new):
- 20-case stress test (paragraphs, headings, lists, tables, fences,
  blockquotes, known limitations). Each case has an identical
  "What to watch for" blockquote label in both V2 and V3 so the diff
  view cleanly isolates each case. Opt-in only.

.gitignore:
- Ignore .claude/ runtime lock/state files. Machine-specific content
  that should not be tracked.

For provenance purposes, this commit was AI assisted.

* style(plan-diff): refine modified-block visual — amber gutter, no fill

Drop the yellow background fill from .plan-diff-modified and keep only a
softened amber left border. Added/removed blocks remain loud (full fill +
strong border) because add/remove are block-scope events — the whole
block matters. Modify is a word-scope event — the individual changed
words carry loud inline red/green highlights, and a block-level fill
would compete with that inline work. The amber gutter at 75% opacity now
reads as a quiet "look inside, the change is in the text" marker that
sits coherently with the rest of the palette.

For provenance purposes, this commit was AI assisted.

* fix(plan-diff): sanitize link hrefs against javascript: / data: schemes

PlanCleanDiffView has its own local copy of InlineMarkdown (separate
from the one in Viewer.tsx). The link-rendering branch was passing the
captured URL directly to href with no validation, so a plan containing
  [click me](javascript:alert(document.cookie))
would render as a live clickable anchor in the diff view. Plan content
is attacker-influenced — Claude pulls from source comments, READMEs,
fetched URLs — so this is a real exploit path in the diff flow.

Port the same guard Viewer.tsx already has: sanitizeLinkUrl() rejects
javascript:, data:, vbscript:, and file: schemes (case-insensitive, with
optional leading whitespace). Rejected links render their anchor text as
plain text instead of a clickable <a>, so the content is still visible
to the reader but no longer dangerous.

For provenance purposes, this commit was AI assisted.
2026-04-14 18:43:38 -07:00
Michael Ramos b3fc1f724f fix(ui): render numerals for ordered list items (#520)
* feat(parser): detect ordered list markers and compute display indices

The block parser collapsed `*`, `-`, and `\d+.` markers into a single
`list-item` block type, discarding ordered/unordered status. Add
`ordered` + `orderedStart` to Block, capture the numeric marker in the
list regex, and introduce `computeListIndices()` — a pure helper that
walks a list group and assigns each ordered item a CommonMark-correct
display number (sequential renumbering, streak break/restart on
unordered items, deeper-level state truncation, top-level numbering
preserved across nested children).

21 new unit tests cover both the parser changes and the indexing
helper, including the tricky cases: `1./2./99.` renumbers as 1,2,3;
sub-bullets between ordered items keep the top-level streak alive;
nested ordered sublists number independently and reset between
siblings; numeric checkboxes set both `ordered` and `checked`.

For provenance purposes, this commit was AI assisted.

* feat(ui): render numerals for ordered list items

Branch the list-item marker span on `block.ordered`: render
`${index}.` (with `tabular-nums` and a 1.5rem min-width to keep
columns stable across single- and double-digit numerals) when the
source marker was numeric, otherwise fall through to the existing
`•`/`◦`/`▪` bullet symbols. Indices come from `computeListIndices()`
called once per list group; `groupBlocks` is unchanged so mixed
nested lists still share a single `data-pinpoint-group="list"`
hover wrapper and annotation anchoring is unaffected. Checkbox
items still take precedence over numerals.

Adds a real-world manual fixture (06-ordered-list-plan.md) whose
`## Verification` section exercises a 10-item ordered list, the
case that originally surfaced the bug.

For provenance purposes, this commit was AI assisted.

* fix(parser): merge consecutive blockquote lines into one block

Each `>` line was emitted as its own blockquote block, so the
renderer's `my-4` margin produced visible gaps between every line
of a multi-line quote (the parser had a literal TODO comment:
"Check if previous was blockquote, if so, merge? No, separate for
now"). Fix: append to the previous block when it's a blockquote
and the prior line wasn't blank, mirroring the list-continuation
pattern. A blank line still breaks the quote so two `>` runs
separated by a blank line stay distinct.

Adds 5 unit tests (merge, blank-line break, paragraph boundaries,
single-line) and a manual fixture (07-blockquotes.md) covering the
bug case, the blank-line-break case, sandwich-between-paragraphs,
and inline markdown across merged lines.

For provenance purposes, this commit was AI assisted.

* fix(ui): address code review — diff view, task lists, blockquote paragraphs

Three issues surfaced by PR review #520:

1. **Diff view flattened ordered lists to bullets.** PlanCleanDiffView's
   SimpleBlockRenderer duplicated Viewer's list-item JSX with hardcoded
   bullet symbols, so a denied+resubmitted plan with numbered steps
   showed numerals in the main view but `•` in the diff view — exactly
   the screen where "which step changed?" matters most. Fixed by
   threading computeListIndices through MarkdownChunk and sharing the
   marker rendering via a new ListMarker component used by both
   renderers, which also removes the root-cause duplication.

2. **Ordered task lists dropped their numbers.** `1. [ ] step` set both
   `ordered=true` and `checked=false` in the parser, but the renderer's
   checkbox branch took precedence and the numeral was never shown.
   GitHub renders `1. [ ]` as numeral + checkbox side by side; we now
   match that by rendering both glyphs in ListMarker when an ordered
   task list item appears.

3. **Multi-paragraph blockquotes collapsed.** After the blockquote-merge
   fix in the previous commit, `> a\n>\n> b` produced content
   `"a\n\nb"` but the renderer passed it straight to InlineMarkdown,
   which renders `\n\n` as whitespace — so two quoted paragraphs
   mashed into one line. Fixed by splitting blockquote content on
   `/\n\n+/` in both Viewer and PlanCleanDiffView and emitting one
   `<p>` child per paragraph.

Adds one unit test for the multi-paragraph blockquote content shape and
a manual fixture (08-ordered-edge-cases.md) covering ordered task lists,
multi-paragraph quotes, nested-bullet counter preservation, double-digit
alignment, and start-at-N numbering.

The fourth review comment — loose ordered lists with intervening non-list
blocks restarting numbering — is deferred. It requires parser-level
loose-list detection (CommonMark's indented-continuation rule) and the
bug only fires when users rely on lazy `1./1./1.` markers across a
break. Tracked as a follow-up.

For provenance purposes, this commit was AI assisted.

* fix(parser): don't merge blockquote lines containing block-level markers

Round-two review flagged a regression: `> 1. foo\n> 2. bar\n> 3. baz` was
merging into one blockquote whose content was `"1. foo\n2. bar\n3. baz"`.
The renderer split on `\n\n+` (paragraph breaks), found none, and emitted
a single `<p>` — so `\n` collapsed to whitespace in HTML and the user
saw `"1. foo 2. bar 3. baz"` as one run-on line. Worse than the pre-PR
behavior (which at least kept each line in its own box).

Pragmatic fix: don't merge a `>` line whose stripped content starts with
a block-level marker (`*`, `-`, `\d+.`, `#`, `` ``` ``, `>`). Those stay
as separate blockquote blocks so each marker line is visually distinct
(legible, matching pre-PR behavior for quoted lists). Wrapped prose
quotes — the original motivating case — still merge correctly because
prose lines don't start with markers.

Also check the PREVIOUS block's content for markers so a trailing prose
line after a `> 1. item` doesn't glue onto the list-item block.

7 new unit tests cover: quoted ordered list stays separate, quoted
unordered list stays separate, quoted heading stays separate, quoted
code fence stays separate, nested blockquote stays separate, wrapped
prose quote still merges (regression guard), and mixed prose+list where
prose merges and list lines stay separate.

Adds tests/test-fixtures/09-quoted-list-regression.md as a manual repro.

Known follow-ups (tracked separately, not in this PR):
- Consecutive separate blockquote blocks still get individual `my-4`
  margins, so a quoted list shows as stacked boxes with gaps between
  lines. The proper fix is recursive blockquote parsing (render the
  content as its own Block[] tree with an actual nested list inside
  the quote). Deferred — requires `children?: Block[]` on Block,
  parser rework, and exportAnnotations traversal changes.
- Clean diff view renumbers ordered lists from the start of each diff
  chunk when users rely on CommonMark's lazy `1./1./1.` markers. Same
  power-user population as the earlier deferred loose-list case.
- Pure code-hygiene items from the second review (non-list-block
  handling in computeListIndices, BULLET_BY_LEVEL modulo cycle,
  <ListGroup> extraction, CLAUDE.md Block interface drift,
  splitBlockquoteParagraphs helper) — batch into a follow-up cleanup.

For provenance purposes, this commit was AI assisted.
2026-04-08 06:58:58 -07:00
foxytanuki b7af16a54c fix: support underscore italics in markdown renderer (#504) 2026-04-07 08:02:41 -07:00
Michael Ramos d850b78ba6 fix: handle markdown hard line breaks and list continuations (#483)
* fix: handle markdown hard line breaks and list continuation lines

List items with indented continuation lines (no blank line separator) now
merge into the preceding bullet instead of becoming orphan paragraphs.
InlineMarkdown now converts two-trailing-space and backslash line breaks
into <br> elements. Synced to the diff view's InlineMarkdown copy.

Closes #482

For provenance purposes, this commit was AI assisted.

* fix: allow bold/italic to span across hard line breaks

Changed bold/italic regexes from .+? to [\s\S]+? so emphasis can match
across newlines (per CommonMark spec). Moved hard break check after all
^-anchored inline patterns so bold/italic get first crack, then the
recursive InlineMarkdown call inside <strong>/<em> handles the break.

For provenance purposes, this commit was AI assisted.
2026-04-04 18:03:58 -07:00
Michael Ramos 2ae4f2a292 fix(parser): indented fences, trailing text, table detection, and escaped pipes (#429)
Three fixes to parseMarkdownToBlocks and one to table cell rendering:

1. Indented closing fences — allow leading whitespace so `  ``` ` inside
   list items closes the code block instead of swallowing to EOF.
2. Trailing text after closing fence — drop end-of-line anchor so
   ` ``` some text` still closes the block.
3. False table detection — require lines start with `|` instead of
   matching any line with 2+ pipe characters.
4. Escaped pipes in table cells — split on unescaped `|` only, so
   `\|` renders as a literal pipe instead of creating extra columns.

Closes #427

For provenance purposes, this commit was AI assisted.
2026-03-29 13:25:17 -07:00
Michael Ramos 93c0f035b3 feat: annotatable diff view with diff context in feedback
* refactor: extract useAnnotationHighlighter hook from Viewer

Move annotation plumbing (web-highlighter lifecycle, toolbar/popover
state, text-selection handlers, findTextInDOM, applyAnnotations) out
of Viewer.tsx into a dedicated hook. Pure refactor — zero behavior
change. Viewer consumes the hook and keeps its own code block, global
comment, and pinpoint-specific logic.

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

* feat: block-level diff annotation with diffContext support

Add annotation support to plan diff view using block-level hover.
Hovering added/removed/modified sections shows the annotation toolbar.
No web-highlighter in diff mode — annotations live in React state only.

- diffContext field on Annotation type (added/removed/modified)
- PlanCleanDiffView: hover handlers, toolbar, comment/quicklabel flows
- Annotated blocks show persistent highlight ring via content matching
- View isolation: diff annotations filtered to diff view, normal to normal
- Share/draft restore filters diff annotations from Viewer DOM
- AnnotationPanel: neutral "diff" badge for diff annotations
- Export: [In diff content] label in feedback
- Toolstrip visible during diff mode for mode switching
- CLAUDE.md updated

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

* fix: scroll-to-selected and visible highlight ring for diff annotations

Add scroll-to-selected when clicking a diff annotation in the panel —
scrolls to the block and briefly glows (same focused effect as Viewer).
Replace invisible ring-1 ring-primary/20 with ring-2 ring-accent for
annotated blocks so they're visually distinct.

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

* cleanup: memoize annotation filters, fix timer leaks, use blockId for highlight rings

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

* fix: preserve normal annotations across diff toggle

Replace ternary rendering with display:none so the Viewer
stays mounted and web-highlighter DOM marks survive the toggle.

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

* fix: store full block content in diff annotations instead of truncating to 500 chars

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-15 23:47:35 -07:00
Michael Ramos fbd4ee4f06 Remove non-critical welcome dialogs (#280)
* chore: remove non-critical welcome dialogs

Remove UI Features Setup, Plan Diff Marketing, and What's New v0.11.0
dialogs from the first-run cascade. Only the Permission Mode Setup
dialog remains as it's the only one that affects agent behavior.

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

* refactor: extract demo plan content to separate file

Moves the 334-line PLAN_CONTENT string from App.tsx to demoPlan.ts to reduce clutter.

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

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-12 00:27:16 -07:00
Itay Grubman 5437a37da9 feat: add mobile compatibility (#260)
* feat: add mobile compatibility

- Add responsive hamburger menu with all header actions (MobileMenu)
- Annotation panel renders as full-screen overlay on mobile with backdrop and close button
- Panel starts closed on mobile (<768px)
- Touch support for resize handles, pinpoint annotations, and toolstrip buttons
- Mobile text selection creates annotations via highlighter.fromRange() bridge
- Card action buttons always visible on touch devices (hover:none media query)
- Settings modal uses horizontal tab bar on mobile
- CommentPopover width capped to viewport on small screens
- Replace mousedown with pointerdown for touch-compatible click-outside handling
- Add useIsMobile reactive hook for breakpoint detection
- Desktop layout (>=768px) unchanged

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

* fix: mobile layout polish — panel gap, button overlap, responsive labels

- AnnotationPanel: fix bottom gap on mobile overlay (inset-y-12 → top-12 bottom-0)
- Viewer: push in-plan action buttons below badges on mobile (mt-6), add
  clear-right before frontmatter, show short labels (Comment/Copy) on mobile
- App: reduce mobile horizontal padding to 8px (px-2)
- test-hook.sh: build review before hook to fix missing dist error

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

* feat: add plan width display setting with compact/default/wide options

Adds a configurable plan width preference (compact 832px, default 1040px,
wide 1280px) with an abstract layout preview in Settings. Dynamic max-width
flows through to Viewer, PlanDiffViewer, and the toolstrip. Default is
compact to preserve existing behavior.

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

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-03-11 16:25:48 -07:00
김영준E 5b28def742 feat: Improve Mermaid diagram viewing experience (#264)
* fix: stabilize Mermaid fullscreen rendering and Safari fit behavior

* fix: block browser zoom and preserve fullscreen Mermaid wheel zoom

* refactor: simplify Mermaid pinch zoom event handling
2026-03-11 07:39:15 -07:00
김영준E 2f5fd47f23 🐛 fix: render links inside bold/italic via recursive InlineMarkdown (#236)
Bold-wrapped links like **[text](url)** were rendered as plain bold text
because the bold regex matched first and treated inner content as a string.

Fix: recurse into InlineMarkdown for bold/italic inner content so nested
markdown (links, code, etc.) is properly parsed.

Affected:
- Viewer.tsx (main plan view)
- PlanCleanDiffView.tsx (diff view duplicate)
2026-03-07 07:07:37 -08:00
Michael Ramos 7145f190ec Add "Open in VS Code Diff" button to plan diff viewer (#180)
* Add feature plan for VS Code diff viewer button

Scoping document for a new PlanDiffViewer button that opens
the plan diff in VS Code's native diff viewer via `code --diff`.

https://claude.ai/code/session_01FYcJkGsWfuiGy53x3pwCSW

* feat: add "Open in VS Code" button to plan diff viewer

Add a button in the PlanDiffViewer toolbar that opens the current
plan diff in VS Code's native side-by-side diff viewer. The server
writes both versions to temp files and spawns `code --diff`.

- New POST /api/plan/vscode-diff endpoint in packages/server/index.ts
- VS Code button with loading/error states in PlanDiffViewer.tsx
- Wire currentPlan/basePlan/baseVersion props from App.tsx

https://claude.ai/code/session_01FYcJkGsWfuiGy53x3pwCSW

* chore: update bun.lock after install

https://claude.ai/code/session_01FYcJkGsWfuiGy53x3pwCSW

* refactor: server reads plan versions directly instead of receiving from client

The server already has the current plan in its closure and can read
any version from disk via getPlanVersion(). No need to send plan
text from the UI — just send baseVersion.

https://claude.ai/code/session_01FYcJkGsWfuiGy53x3pwCSW

* chore: remove plan.md scoping document from repo

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

* refactor: extract editor diff logic into shared packages/server/editor.ts

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

* feat: use official VS Code SVG icon in plan diff viewer button

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

* refactor: use history file paths directly for VS Code diff instead of writing temp files

Both plan versions already exist on disk in ~/.plannotator/history/.
Eliminates redundant file writes, fixes UPLOAD_DIR semantic misuse,
and corrects HTTP status codes for editor errors.

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

* refactor: rename editor.ts to ide.ts to avoid confusion with plan editor UI

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

* docs: add ide.ts and vscode-diff endpoint to CLAUDE.md

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

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-02-26 13:25:08 -08:00
Michael Ramos 01c5b92a82 chore: bump version to 0.9.1
Fix Plan Diff marketing dialog overflow on small viewports.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-23 00:55:04 -08:00
Michael Ramos aa1404fe99 fix: replace placeholder video URLs with actual YouTube link
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-23 00:26:13 -08:00
Michael Ramos effdd24c71 feat: Plan Diff marketing dialog, Pi origin, and docs (#177)
* feat: add Plan Diff marketing dialog, Pi origin support, and docs

- Add PlanDiffMarketing first-run dialog announcing the Plan Diff feature
  with per-origin video demo URLs (Claude Code, OpenCode, Pi)
- Add 'pi' as a first-class origin with display name and violet badge
- Add Plan Diff blog post (plan-diff-see-what-changed.md)
- Add brief Plan Diff mentions across READMEs and marketing docs


* polish: tighten blog post copy and reduce em dash usage


* fix: move PlanDiffMarketing to plan-diff/ and fix stale useEffect deps


* add plan diff preview screenshot for marketing dialog
2026-02-23 00:24:25 -08:00
Michael Ramos 819ba11f77 feat: plan diff UI with sidebar and dual view modes (#176)
* feat: add plan diff UI with sidebar, badge, and dual view modes

Shows what changed between plan iterations when Claude revises after
feedback. Adds a +N/-M badge below repo info that toggles the diff view,
a shared left sidebar with TOC and Version Browser tabs, and two diff
modes: rendered (color-coded borders) and raw markdown (+/- lines).

Closes #138, closes #111

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

* docs: update CLAUDE.md project structure and align first-run dialog labels

- Add plan-diff/ and sidebar/ component subdirectories to CLAUDE.md
- Add new hooks and utils to CLAUDE.md project structure
- Rename "Table of Contents" to "Auto-open Sidebar" in UIFeaturesSetup
  to match Settings.tsx label

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

* fix: address code review findings for plan diff UX

- Fix badge stats mixing block counts with line counts (modifications
  now fold into additions/deletions)
- Gate hasPreviousVersion on diffBasePlan being loaded to prevent
  "Show Changes" no-op and ModeSwitcher disappearing
- Make sidebar reactive to Settings toggle (useEffect on tocEnabled)
- Match PlanDiffViewer badge layout to Viewer (flex-col) so badge
  doesn't jump position on toggle
- Add "Exit Diff" label to the close button in diff view
- Remove dead CSS (plan-diff-removed-marker, plan-diff-modified)
- Clean up stale header comment and unused lines prop

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

* fix: second-round review cleanup for plan diff UX

- Fix stale "amber border" JSDoc in PlanCleanDiffView (actually green)
- Rename sidebar tab from "diff" to "versions" for clarity
- Gate VersionBrowser fetch on versionInfo being available
- Move .sidebar-tab-flag CSS into its own Sidebar section

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

* fix: add loading state for version selection in sidebar

Add isSelectingVersion to selectBaseVersion, mirroring the existing
isLoadingVersions pattern. Shows "Loading..." on the selected version
button while the fetch is in progress.

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

* fix: address third-round code review findings

- Fix duplicate border/backdrop on TOC inside sidebar (className override)
- Fix loading indicator targeting wrong version button (fetchingVersion state)
- Fix "Show Changes" button silent no-op (gate on hasPreviousVersion)

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

* refactor: move date to slug suffix, improve Other Plans UX

- Slug format changed from YYYY-MM-DD-{heading} to {heading}-YYYY-MM-DD
- Other Plans: single "coming soon" banner instead of per-item labels
- Strip date suffix from plan names in sidebar for readability
- Remove cursor-not-allowed from Other Plans items

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

* docs: add Plan Diff section to CLAUDE.md, alert on version fetch failure

- Document plan diff feature: engine, view modes, state management, sidebar
- Update slug format documentation to {heading}-YYYY-MM-DD
- Show native alert when version fetch fails instead of silent swallow

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

* fix: add table rendering to clean diff view

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

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-22 18:59:44 -08:00