Skip to content

fix(desktop): let assistant read comments by id#576

Open
ion-lion wants to merge 1 commit into
seed-hypermedia:mainfrom
ion-lion:ion-fix-assistant-read-comment-555
Open

fix(desktop): let assistant read comments by id#576
ion-lion wants to merge 1 commit into
seed-hypermedia:mainfrom
ion-lion:ion-fix-assistant-read-comment-555

Conversation

@ion-lion
Copy link
Copy Markdown
Collaborator

@ion-lion ion-lion commented May 7, 2026

Summary

  • keep the desktop assistant read tool URL-only ({ url: string }), no commentId parameter
  • support canonical comment URL formats from the HM URL doc: /:comments/<COMMENT_ID> and ?panel=comments/<COMMENT_ID>
  • route comment URLs through the same Resource read API path used for documents instead of calling the comments API directly
  • move the CLI markdown renderer into shared code at @shm/shared/hm-markdown, and use it from both CLI and desktop assistant reads

Fixes #555.

Validation run here

  • git diff --check
  • corepack pnpm --filter @shm/desktop run typecheck
  • corepack pnpm --filter @shm/shared run typecheck
  • corepack pnpm --filter @seed-hypermedia/cli run typecheck
  • corepack pnpm --filter @shm/desktop run test:unit (43 files / 413 tests passed)

Attempted but unavailable on this node:

  • CLI unit tests: bun is not installed (spawn ENOENT).
  • Manual desktop launch: this node has no DISPLAY, no xvfb-run, and Electron cannot start because libatk-1.0.so.0 is missing.

Comprehensive testing plan

CLI integration / live tests

  1. Create a fixture document with nested blocks, inline annotations, embeds/mentions/query-ish content, and at least one comment.
  2. Verify seed-cli document get <hm-url> emits markdown through the shared renderer, including block IDs/frontmatter for documents.
  3. Verify seed-cli document get <comment-url> / seed-cli document get hm://<COMMENT_ID> emits markdown for the comment body through Resource.
  4. Verify seed-cli comment get <id> --json still returns structured comment data unchanged.
  5. Verify seed-cli document get --resolve <hm-url> still resolves embeds/mentions through the shared renderer.

Desktop integration / E2E tests

  1. Add a desktop E2E fixture that publishes a document and a comment, then opens the assistant panel.
  2. Stub/select a deterministic local assistant provider so the test can request: “read this comment URL: <DOC_URL>/:comments/<COMMENT_ID>”.
  3. Assert the assistant tool call includes only {url: "<comment-url>"}.
  4. Assert the read execution calls the Resource path for the comment and returns the comment markdown body, author, target document URL/title, and comment ID/version.
  5. Repeat with ?panel=comments/<COMMENT_ID>.
  6. Repeat with /:comments without an ID to ensure comment-list behavior remains supported.
  7. Add a negative case for an invalid comment URL and assert a useful tool error.

Manual desktop smoke test

  1. Run the desktop app from a node with Electron runtime deps and a display server: corepack pnpm --filter @shm/desktop run dev:debug.
  2. Open a document with an existing comment.
  3. Copy the canonical comment URL (<DOC_URL>/:comments/<COMMENT_ID>).
  4. Ask the built-in assistant to read that comment URL.
  5. Confirm the assistant calls read with only url, renders the comment markdown, and does not invent/request a separate commentId field.
  6. Repeat with the panel URL form: <DOC_URL>?panel=comments/<COMMENT_ID>.

@ericvicenti
Copy link
Copy Markdown
Collaborator

ok but the CLI already has a sophisticated solution for loading any HM url as markdown. And we want to focus on that implementation. Can you re-use that code, and make sure it is shared code if not already?

also, include a comprehensive testing plan for both CLI and desktop. Integration and/or E2E tests, not just a basic unit test. On your node you should find a way to run the desktop app and use it manually

@ion-lion ion-lion force-pushed the ion-fix-assistant-read-comment-555 branch from 728c1ae to 321a088 Compare May 7, 2026 20:24
@ion-lion
Copy link
Copy Markdown
Collaborator Author

ion-lion commented May 7, 2026

Thanks — agreed. I revised the PR to reuse the CLI markdown renderer instead of keeping the desktop assistant on a one-off renderer.

What changed:

  • moved the CLI markdown implementation into shared code: @shm/shared/hm-markdown
  • kept CLI imports pointed at that shared implementation
  • updated the desktop assistant read path so direct commentId reads render comment content through the same shared markdown code
  • updated the PR description with a comprehensive CLI + desktop integration/E2E/manual testing plan

Validation I could run on this node:

  • git diff --check
  • corepack pnpm --filter @shm/shared run typecheck
  • corepack pnpm --filter @seed-hypermedia/cli run typecheck
  • corepack pnpm --filter @shm/desktop run typecheck
  • corepack pnpm --filter @shm/desktop run test:unit (43 files / 413 tests passed)

I also tried to find a way to run the desktop app manually here, but this node is missing the GUI/runtime pieces: no DISPLAY, no xvfb-run, and Electron fails on missing libatk-1.0.so.0. I documented that in the PR with the manual smoke-test steps for a node that has those desktop deps.

@ericvicenti
Copy link
Copy Markdown
Collaborator

the read tool call accepts commentId? thats crazy, it should just accept a comment URL to be passed into the read api. don't add a new parameter

see a summary of all our URL formats: https://seedteamtalks.hyper.media/discussions/ultimate-hm-url-doc

@ion-lion ion-lion force-pushed the ion-fix-assistant-read-comment-555 branch from 321a088 to 696a6e6 Compare May 7, 2026 21:52
@ion-lion
Copy link
Copy Markdown
Collaborator Author

ion-lion commented May 7, 2026

You’re right — I corrected the shape now.

Updated the PR so the assistant read tool is URL-only again ({url}), with no commentId parameter. It now supports the canonical comment URL formats from the URL doc:

  • <DOC_URL>/:comments/<COMMENT_ID>
  • <DOC_URL>?panel=comments/<COMMENT_ID>

For those URLs, the desktop assistant derives the comment resource IRI and reads it through the same Resource read API path as documents, instead of calling the comments API directly. The CLI markdown renderer is still shared through @shm/shared/hm-markdown and used by both CLI and desktop.

Re-ran validation:

  • git diff --check
  • corepack pnpm --filter @shm/desktop run typecheck
  • corepack pnpm --filter @shm/shared run typecheck
  • corepack pnpm --filter @seed-hypermedia/cli run typecheck
  • corepack pnpm --filter @shm/desktop run test:unit (43 files / 413 tests passed)

I also updated the PR testing plan to focus on comment URLs rather than a separate commentId field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assistant/Agent cannot read a comment

2 participants