Repository navigation
refactor: comments on drops live in ViteHub Comments, with the same numbers, replies, and API - #64
Conversation
… what they say Drop turns on ViteHub Comments (server/comments.ts). Opening the Console turns the toolbar on for admins: they pin comments on any page, reply, resolve, and see each other live. Each thread keeps its page, element, Vue components and their files, browser, and build. Agents read and resolve threads through Drop's MCP (comments_* tools, admins only) and ViteHub MCP. Other people get the pages unchanged. ViteHub moves to the Comments stack (vite-hub/vitehub#2864).
…umbers, replies, images, and API Drop's own comment tables go: each comment on a doc, app, image, folder, or site is now a ViteHub Comments thread, its first comment the comment and the rest its replies. A doc's versions share one list by their link (`drop:<link>`), and a site shares one with its screenshots (`site:<id>`), so numbers still count across versions and screenshots. The thread's metadata keeps the version and anchor, and the API answers the same DropComment JSON. Deleting a drop deletes its threads. Replies still reach resolved comments: the thread reopens for the reply and resolves again. ViteHub moves to e493b58, whose Comments finds a reply's thread (`getByComment`) and suggests only the authors of Drop's pages for `@`, so drop commenters stay out of the overlay. Migration 0003 drops `comments` and `comment_replies`, so existing comments and replies go. comment_attachments loses its foreign keys and keeps its rows, so the images of old comments stay out of embedded files.
onmax
changed the title
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
vitehub-drop | 56216d3 | Oct 26 2026, 08:38 AM |
Latest result Pull request merged. Final answersSession Pull request merged. Session Waiting for GitHub merge request 7adc6cef-d2aa-4ce0-8928-46290de37573. Session Pushed 9b7572c, resolving merge conflicts, P1 bulk-resolution transaction overlap, and P2 test-counter isolation. Prior Pullfrog findings remain fixed. All 94 focused tests passed with one worker; focused lint, formatting, drizzle-kit check, and 15 SQLite upgrade replays passed. Host dependency refresh encountered a pnpmfile mismatch; normal frozen installation succeeded. Awaiting checks. Session Pushed 9b7572c, resolving merge conflicts, P1 bulk-resolution transaction overlap, and P2 test-counter isolation. Prior Pullfrog findings remain fixed. All 94 focused tests passed with one worker; focused lint, formatting, drizzle-kit check, and 15 SQLite upgrade replays passed. Host dependency refresh encountered a pnpmfile mismatch; normal frozen installation succeeded. Awaiting checks. Prepared d9a28ed, resolving merge conflicts and P1 bulk-resolution transaction overlap. Prior Pullfrog findings remain fixed. All 95 focused tests now pass; lint, formatting, drizzle-kit check, and 15 SQLite migration replays pass. Host installation encountered pnpmfile/sandbox restrictions; installed tools completed validation. pushRepair rejected publication because the candidate lease changed; remote remains 76e68f0. Refresh the publication context and retry the verified commit. [vitehub] Provider Agent Driver cleanup failed. Session Pushed 76e68f0 fixing formatting and outdated Comments error assertions. pnpm fmt --check, focused lint, and all 18 focused tests pass. The host install’s ignore-pnpmfile checksum failure was avoided with a successful normal frozen install. Historical Pullfrog findings remain fixed. Awaiting hosted CI and parent #63’s merge. Pushed 76e68f0 fixing formatting and outdated Comments error assertions. pnpm fmt --check, focused lint, and all 18 focused tests pass. The host install’s ignore-pnpmfile checksum failure was avoided with a successful normal frozen install. Historical Pullfrog findings remain fixed. Awaiting hosted CI and parent #63’s merge. |
|
P2: screenshot deletion can retain threads outside the 500-thread window. At 511b572, I reproduced this by creating a screenshot thread followed by 500 newer site threads, then deleting the screenshot: the drop disappeared but its thread remained. |
… stays resolved
ViteHub Comments now takes { allowResolved: true }, so replyTo no longer
reopens the thread, replies, and resolves it again.
![vitehub-bot[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Permanent cleanup can leave durable comment data behind, including for screenshots once a site exceeds the primitive list limit.
Reviewed changes across the full 29-file diff: the migration from Drop-owned comment tables to scoped ViteHub Comments threads, API and MCP compatibility, review-site integration, attachment ownership, database migration, deletion lifecycle, and focused test coverage.
- Thread-backed Drop API — Maps primitive threads, opening comments, replies, authors, metadata, resolution state, and numbering back to the existing
DropCommentcontracts. - Version and site scopes — Shares numbering across document versions and across a site with its screenshots while filtering each viewer's list.
- Images and review flows — Keeps attachment drops in Drop storage while comment/reply identity moves to primitive IDs, including screenshot creation and rollback.
- Schema and lifecycle — Rebuilds
comment_attachments, removes the legacy tables, and adds comment cleanup to permanent Drop deletion. - Validation — The focused review, MCP, and D1 migration suites pass locally: 55 tests across three files.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focused on the permanent-deletion cleanup fix and its failure paths.
- Added durable cleanup work — Queued each Drop's comment scope in the same database batch that permanently deletes the Drop, preserving retry state across request and process failures.
- Removed the screenshot list cap from cleanup — Selected screenshot thread ids directly from the primitive schema in bounded batches, so deletion covers threads outside the 500-item viewer list.
- Added retry coverage — Exercised primitive failure, queue insertion rollback, acknowledgement failure, process restart, and cleanup beyond 500 site threads.
- Extended the schema and migration — Added the foreign-key-free
comment_cleanuptable and included it in the database configuration and migration snapshots.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focused on the parent merge and migration upgrade paths.
- Resequenced the migration journal — Placed auth events before ViteHub Comments and Drop's destructive legacy-comment migration, with strictly increasing timestamps.
- Preserved preview upgrade paths — Covered databases whose auth and Comments migrations were recorded at current or historical timestamps, while retaining primitive comments and legacy attachment rows.
- Expanded migration invariants — Verified auth indexes, cleanup-table creation, legacy-table removal, foreign-key integrity, and migration idempotence across fresh and existing databases.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focused on the parent suggestion merge and migration reconciliation.
- Preserved suggestion semantics — Stored replacement text, including deletion suggestions, in thread metadata while keeping suggestion-only and image-only comments compatible with the primitive's non-empty body requirement.
- Maintained API compatibility — Converted placeholder primitive bodies back to Drop's empty comment body and defaulted older thread metadata to no suggestion.
- Reconciled migration histories — Kept the parent suggestion ledger entry as a no-op on this child, advanced the stacked timestamps, and made destructive or additive migrations safe to replay over earlier preview schemas.
- Expanded upgrade validation — Covered parent and child preview ledgers while retaining primitive comments, legacy attachment rows, and durable cleanup work; the focused migration, MCP, and review suites pass all 60 tests.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
Important
Suggestion acceptance can publish the edit but leave its thread open in a state where the Accept action cannot finish on retry.
Reviewed changes since the prior Pullfrog review, focused on the parent merge's adaptation of suggestion acceptance and migration replay coverage.
- Moved acceptance to primitive threads: Read suggestion metadata and resolution state from the ViteHub Comments thread, then recorded the browser identity when resolving it.
- Expanded migration replay cases: Covered ledgers that already contain Comments, Drop comment removal, or durable cleanup entries while preserving legacy attachments and primitive data.
- Updated focused assertions: Verified accepted suggestions resolve primitive threads with the expected resolver and retained document version behavior.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focused on the recoverable suggestion-acceptance fix.
- Added an atomic acceptance receipt — Committed the suggestion thread id with the published Drop version so publication cannot succeed without durable retry state.
- Made resolution retries idempotent — Detected an existing receipt before applying the suggestion and retried only primitive thread resolution, including after restarts and later document versions.
- Refreshed failed client state — Reloaded the Drop, comments, tree, and usage after an acceptance error because publication may already have completed.
- Added migration and failure-path coverage — Introduced the receipt table and verified resolution outages, rollback on receipt failure, concurrent requests, permission checks, deletion cleanup, and migration upgrades; the focused suites pass all 8 tests.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focused on the parent merge's primitive-thread adaptation and recoverable undo behavior.
- Added recoverable suggestion undo — Republished the preceding document text, atomically recorded the reversal version, and retried only thread reopening after a Comments failure.
- Preserved acceptance after undo — Removed completed undo receipts during reacceptance and inserted a fresh active receipt without allowing concurrent accepts or reversals to publish duplicates.
- Adapted suggestion routes to primitive threads — Centralized browser and MCP acceptance around validated thread metadata, stable thread ids, and existing Drop edit permissions.
- Extended migration and failure coverage — Added the nullable undo receipt column and tested reopening outages, restarts, later edits, authorization, receipt rollback, concurrent undo, and reacceptance; the focused 8-test suite and Nuxt typecheck pass.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focused on the released-parent migration merge and the child schema catch-up.
- Added a post-release catch-up migration — Reapplied the Drop comment move after the released parent ledger so parent databases receive cleanup and suggestion-recovery state.
- Preserved preview data during convergence — Rebuilt attachment ownership before removing legacy comment tables and retained existing cleanup work, acceptance receipts, undo state, and primitive comments.
- Expanded upgrade-path validation — Covered fresh, released-parent, historical preview, cleanup, acceptance, undo, pre-catch-up, and fully migrated ledgers with foreign keys enabled.
- Regenerated the migration chain — Updated journal ordering and snapshots so the released parent migration precedes the final child schema.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focused on the merge with current main and the resulting Comments integration adjustments.
- Serialized primitive resolutions — Changed multi-comment resolution from overlapping write batches to sequential operations on the shared SQLite connection.
- Adopted the released ViteHub stack — Removed preview dependency patches and local schema duplication now supplied by the published
0.1.0packages and merged parent. - Reconciled migration histories — Preserved attachments, primitive comments, cleanup work, acceptance receipts, and undo state across current-main and child-preview ledgers.
- Isolated comment numbering tests — Cleared primitive scope counters during harness reset so test cases cannot inherit thread numbers.
- Validated affected paths — The focused migration, suggestion recovery, cleanup, and MCP suites pass all 53 tests.
gpt-5.6-sol ｜ 𝕏
Drop kept its own comment tables, numbering, and reply code for comments on drops, apart from the ViteHub Comments primitive that #63 brings in. Two comment systems meant twice the code to keep right.
With this change, Drop's comments on drops are ViteHub Comments threads, in scopes of their own. Users see no change: same numbers, carry-over between versions, replies, images, review tokens, MCP tools, and API JSON. The old
commentsandcomment_repliestables go, with their rows. Maxi allowed dropping existing comments to keep the move simple; drops stay.Stacked on #63. Uses the new primitive API from vite-hub/vitehub#2864.
How it works
Scopes:
drop:<link>, because every version shares the link;site:<site id>.The thread
numberis the#nthat people and agents use. These scopes never show in the Comments toolbar, the Console, ViteHub MCP, or@suggestions.Metadata keeps what the row columns kept: the drop and version it was written on,
kind,selector,ox/oy/ow/oh,quote,label,page, and the element point.server/utils/comments.tsturns threads back intoDropComment, so the client and the review script get the same JSON.Routes, MCP tools (
read_drop,reply,update_drop), publishresolves, and site review now use the primitive. Drop still checks who may, as before.Replies on a resolved comment are allowed, as before:
comments.reply(…, { allowResolved: true })adds the reply in one write, and the comment stays resolved.Deleting a drop removes its threads in
deleteDrop, which every deletion path goes through:removeScope;Images:
comment_attachmentsstays, and now points at ViteHub comment ids. Migration0003rebuilds it without foreign keys to the old tables before it drops them. It keeps old rows, so images of old comments stay out of their docs' embedded files.vite-hubmoves to the stack-top build (35beeca):getByComment(),@suggestions limited to one scope,allowResolved, and the review fixes.Evidence
On a production Node build of this branch (
DROP_HOST=vps, SQLite), against the database from #63's run:0003:commentsandcomment_replieswere gone, andvitehub_comment_threads,vitehub_comments, andcomment_attachmentsremained.v1badge, 1 reply). A comment on v2 got feat: add code image rendering #2. v1 lists only refactor!: migrate Drop to portable Sandbox package projects #1. The threads are in scopedrop:<link>, next to the admins'pagesthreads from feat: admins comment on Drop's own pages, and agents fix what they say #63.Validation
pnpm install(with postinstall) andpnpm typecheckpass.vp test: 44 files and 304 tests pass, 3 skipped. The affected files also pass on their own (test/review/review.test.mjs,test/mcp/tools.test.mjs,test/cost/round-trips.test.mjs,test/auth/console.test.mjs: 63 tests). New tests cover:@suggestions leaving out drop commenters.drizzle-kit checkpasses. In-memory SQLite with foreign keys on applies0000to0003over a seeded old comment, reply, and image, andforeign_key_checkis empty.vp checkandpnpm knipare clean.Changes you can notice
round-trips.test.mjsrecords the new numbers.guest:<uuid>author id. They are still never "mine".Not done yet
0003rebuildscomment_attachmentsbefore it drops the old tables for that reason, but it has not run on D1.🤖 Generated with Claude Code
Parent update (
857abee): merged #63 atde7a836. The migration journal now places the Drop comment move after auth events and ViteHub Comments. Migration tests preserve the parent upgrade cases while verifying attachment preservation with foreign keys enabled, removal of the legacy comment tables, and creation of the durable cleanup queue. All 78 focused migration, cleanup, review, MCP, round-trip, and Console tests passed, along with Nuxt typecheck, changed-file lint/formatting, anddrizzle-kit check. Both review repairs from0133a42remain intact. #63 is now merged into main at78d77f7, and the registry confirms thatvite-hub@0.1.0and@vite-hub/comments@0.1.0are published; the parent and release prerequisites are fulfilled.