Repository navigation
feat(hub): team documents — blocks, per-block holds, checked changes, snapshots and search - #266
Conversation
There was a problem hiding this comment.
🔆 CodeSolar review ✅ Looks good
Review of PR #266 — team documents feature for the hub service. The PR adds a new hub/ workspace with document block storage, rank-based ordering, in-memory locks, markdown parsing, and search integration. I examined the added source files (locks, rank, blocks, events, socket, auth, visibility, multipart, store, idempotency, app, index) and the shared markdown/search modules. The design is internally consistent and the test coverage is extensive (184 tests across 12 files).
✅ No issues worth flagging.
⚠️ The diff was large; some files were excluded from the review.
solar-pro4 (openrouter) · 55 files · +12600/-6 · 237.5s · CodeSolar
Comment /codesolar on this PR to re-run the review.
d84b118 to
13e1bc2
Compare
|
bugbot run |
There was a problem hiding this comment.
🔆 CodeSolar review ✅ Looks good
🔄 2nd review of this pull request · 22 new commit(s) since the last one
Large PR adding team documents (hub_docs, hub_doc_blocks, hub_doc_versions tables), block CRUD with version/hash conflict detection, in-memory locks, fractional-rank ordering, markdown round-trip, snapshots, and document search integration. 23 new test files/commits since last review. I read the migration SQL, schema, blocks validation, locks, rank, the full routes/applyOps logic, markdown parser, protocol types, search changes, and the three new test files.
✅ No issues worth flagging.
⚠️ The model's answer arrived truncated; only the complete part was recovered — some findings may be missing.
solar-pro4 (openrouter) · 21 files · +5082/-69 · 585.9s · CodeSolar
Comment /codesolar on this PR to re-run the review.
e9f4c37 to
15a7749
Compare
|
bugbot run |
There was a problem hiding this comment.
🔆 CodeSolar review ✅ Looks good
🔄 3rd review of this pull request · 1 new commit(s) since the last one
Review of PR #266 for team documents feature. The diff adds document blocks, per-block holds, checked changes, snapshots, and search across 38 files. Previous review found nothing; one new commit since then only updates test-ci.ts floor count. No code defects found in the visible diff.
✅ No issues worth flagging.
⚠️ The diff was large; some files were excluded from the review.
solar-pro4 (openrouter) · 34 files · +9193/-10 · 218.6s · CodeSolar
Comment /codesolar on this PR to re-run the review.
1565972 to
e3ac9e6
Compare
|
bugbot run |
There was a problem hiding this comment.
🔆 CodeSolar review ✅ Looks good
🔄 4th review of this pull request · 62 new commit(s) since the last one
Team documents feature: blocks, holds, checked changes, snapshots, and search integration. The PR adds 3 new tables (hub_docs, hub_doc_blocks, hub_doc_versions), document routes with batch ops and locking, in-memory lock storage, fractional-rank ordering, markdown serialization, and document search. 22 files changed, +6600/-69.
✅ No issues worth flagging.
⚠️ The diff was large; some files were excluded from the review.
solar-pro4 (openrouter) · 21 files · +6600/-69 · 202.9s · CodeSolar
Comment /codesolar on this PR to re-run the review.
…ges, snapshots and search Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
docs.test.ts 23, doc-markdown.test.ts 40, doc-rank.test.ts 10. Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
… version it answered A save that inserted a block and then replaced it in the same batch wrote the new row at version 1 while its answer named version 2, so the writer's next save naming that version was refused as laf:doc_block_changed for a block it had just written itself. The insert now stores the block's working version. Test: docs.test.ts "a block inserted and rewritten in one save keeps the version it answered". With `version: block.version` put back to `version: 1` in the insert it fails (stored 1, answered 2); restored, the hub's 190 pass. Hub floor 189 -> 190. Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
… documents add Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
e3ac9e6 to
e227886
Compare
|
bugbot run |
There was a problem hiding this comment.
🔆 CodeSolar review ✅ Looks good
🔄 5th review of this pull request · 13 new commit(s) since the last one
Team documents feature: blocks, holds, checked changes, snapshots, search. Review found no correctness bugs in the diff; the lock endpoint doubles as renew (same member re-taking updates until), audienceOf signature widened correctly, search doc-chunk integration is sound, and applyOps batch logic (all-or-none, version/hash checks, hold rules, renumbering) is internally consistent. One design note: GET /docs/:id/blocks/:bid has no audit record while other read routes do.
✅ No issues worth flagging.
⚠️ The diff was large; some files were excluded from the review.
solar-pro4 (openrouter) · 21 files · +6600/-69 · 254.4s · CodeSolar
Comment /codesolar on this PR to re-run the review.
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e227886. Configure here.
Stacked on #251 (
laf/hub-files→ #247): this branch starts fromorigin/laf/hub-filesand its diff againstmainincludes #247 and #251 until they land. Review the last two commits; the migration is0002_hub_docsafter #251's0001_hub_files(siblings that also carry a0001_*renumber at merge time).What this changes
The company hub keeps team documents as ordered blocks, the shape the design settled for documents people and 라피 edit together: block =
{id, kind, text, attrs, version, hash}, twelve fixed kinds (paragraph, heading1–3, list, check, quote, code, divider, table, file, task-link). There is no editor library, no CRDT and no new dependency; the markdown renderer and parser are our own code (shared/hub/doc-markdown.ts), so the VM side can use the same parser.Tables (migration
0002_hub_docs)hub_docs— title, folder, visibility (company/me:<id>/task:<id>, the same check as files),rev, soft delete.hub_doc_blocks— one row per block,(doc_id, id)primary key,rankfor order,version,hash, kind checked in SQL.hub_doc_versions— snapshots: the block list as JSON at the moment somebody saved a version.hub_chunkswithsource_kind = 'doc',source_id = <doc>/<block>, in the same transaction as the block write; a deleted block's or document's chunks go with it.Routes (
/hub/v1/docs, member pass; writes takeIdempotency-Key)GET /docs?folder=&cursor=POST /docsblocksormarkdown(one empty paragraph when neither)GET /docs/:idGET /docs/:id/blocks/:biddoc.block_changed(404 = deleted)GET /docs/:id/markdown<!-- <block id> <hash> -->PATCH /docs/:id·DELETE /docs/:idPOST /docs/:id/blocksreplace·insert_after·delete·movePOST /docs/:id/blocks/:bid/lock·/unlockPOST /docs/:id/versions·GET /docs/:id/versions·GET /docs/:id/versions/:vHow a change is checked. Every op that touches a block names what its writer read:
versionfor a person's save,expectHashfor 라피's. A request whosex-laf-originis notperson(a chat turn, a routine, a room) must useexpectHash. A mismatch is 409laf:doc_block_changedwith the block as it is now (block, null if deleted). A block somebody holds is 409laf:doc_block_busywith the holder — for 라피, anybody's hold, its own person's included; for a person's save, only somebody else's. The batch is checked in memory first and written only when every op passes, so a refusal leaves the tables as they were. Each batch takes the document's rowfor update, so batches on one document apply one at a time and a check cannot go stale between reading and writing. There is no whole-document replace op.Rank scheme: fractional strings. A rank is a base-36 fraction (
0-9a-z, never a trailing0), compared byte by byte (collate "C"in SQL,<in JS). An insert or move writes one row. Inserting again and again at one place grows a rank by about a character every five inserts; past 48 characters the document is renumbered evenly in the same write (ranks of 1–4 characters even at 5,000 blocks) and the VMs get onedoc.changedinstead of an event per block. No unique index on(doc_id, rank): the row lock already serialises a document's writes, and a unique index would refuse the one-statement renumbering halfway through.Lock storage: in memory, with expiry. A hold lives 15 s unless renewed every 5 s, so a table would take a write every five seconds for every person with a block open, on a hub with one core, for a fact nobody needs after a restart. The hub is one process (as a person's VM is —
docs/laf/deployment-model.md), so one map is every hold. A restart forgets them and loses nothing: the next renewal, at most 5 s later, takes the hold again, and a save never depended on a hold (it carries its version). Expired holds are dropped when read.Live updates. A batch emits
doc.block_changed(refId<doc>/<block>) per block it wrote or deleted, or onedoc.changedwhen it renumbered or touched more than 50 blocks; create, rename and delete emitdoc.changed. Events go only to members who may see the document (audienceOf). Holds travel as light frames on the same events socket —{kind: "doc.lock", doc, block, member, name, until}(until: null= let go) — sent to the sockets of members who may see the document on every take, renewal and release; not numbered, not kept. A VM that does not know the kind drops it (aslink.tsin #249 already does).Search.
POST /searchnow also looks in document blocks and titles, permission in the same query (visible_toon the chunk and again on the document row). A doc hit is{kind: "doc", id, title, where: {folder, block}, snippet, …}(blocknull when the title alone matched).kindsnarrows it (["file"],["doc"]); kinds the hub does not search yet are ignored, not refused, since #255 already sendsskill/room/memory. A hub without file space still searches documents (before,/searchanswered 503laf:hub_files_unsetoutright; it still does when only files are asked).Touched outside the new files, kept small:
events.ts/socket.ts(asendon each listener andsendTo(audience, frame), 20 lines — the sibling branches change the sameListenertype, so expect a small merge there),visibility.ts(audienceOftakes the database as well as a transaction),search.ts,app.ts,shared/hub/protocol.ts(doc types, two event types, two facts, the search hit union), one line oftests/search.test.ts(narrowing the new hit union).Where it runs
On the company hub VM only; nothing on a person's VM changes in this PR.
/hub/v1; the hold frames ride the existing events socket. No timers (expired holds are dropped on read).Boundary and audit
visible_toin SQL (document row, chunks); every write locks the document row through the same visible query, so a member cannot write to a document they cannot see (404 on every route, tested).doc.created,doc.blocks,doc.renamed,doc.deleted,doc.version,doc.readon whole reads) naming the document and counts, never words — tested by serialising the trail rows and asserting the typed text is not in them.doc.blocks, outcomelaf:doc_block_changed/laf:doc_block_busy/laf:request_invalid, with the op index, the block id and whether it was 라피 or a person) and a refused hold (doc.lock, outcomelaf:doc_block_busy, the holder's id). Tested for the stale-hash refusal.x-laf-origin, and saying "person" only buys the version check, which still refuses a block somebody else holds.Korean
laf:doc_block_changed,laf:doc_block_busy) get their Korean on the surface in the editor's PR (task d2).The gate
Run on head
d84b118cwithLAF_TEST_DB_SUFFIX=d1 LAF_TEST_WORKERS=2:bun run typecheck— every workspace exit 0.bunx biome lint .— no new findings (2 warnings, 1 info, all in files this PR does not touch; the same on the base).bun run format:check— clean.bun run test:ci— exit 0: server 4752 / 4711 · app 2751 / 2534 · agent-computer 651 / 632 · root 1780 / 1744 · hub 184 / 184.All four pass. Floor moved: hub 111 → 184 (its own commit):
docs.test.ts23,doc-markdown.test.ts40,doc-rank.test.ts10. The first gate run caughtknipflagging four contract types the hub did not use yet; the routes are now typed by them (and the hold answer carriesrenewMs).Each guarding test was seen failing with the line it guards reverted (below).
Proof
Acceptance, measured against a real hub (
bun hub/src/index.tson :7380, its ownpostgres:17container on :55612, headd84b118c), driven by a script over HTTP and the events socket — two members by registry body, 라피 as A's VM pass withx-laf-origin: chat. A locks block 2 and edits it while B edits block 3 at the same time; 라피 tries block 2 → busy; block 4 with the hash it read before B changed it → changed with the latest; re-reads → ok; search finds B's new word; the markdown export shows every block in order. Output as printed (ids are random per run):The hub's log had no error line. Container, volume directory and key removed afterwards.
Tests seen failing with the guarded line reverted (each run alone, then restored):
if (hold && …)→ never) → "a block a person is writing in…" fails.block.hash !== expectHash→ never) → "라피 with a stale hash…" fails.for updateremoved → "eight saves racing on one block…" fails three runs out of three (two or more saves stand). The two-save version passed without the lock — the requests did not overlap — which is why the eight-way test exists.visibleTodropped from the document query → "a document only its writer may see…" fails.x-laf-originignored → "a run's change must name a hash…" fails.<→<=→ "the list pages newest change first…" fails.Model calls: none.
Left for later
GET /docs/:id/versions/:v, tested to return the old list exactly); putting it back as the current document would be a whole-document replace, which the design leaves out. If a person-only restore is wanted, it is a batch of ops built from the snapshot.laf/hub-docs-ui), against this contract.🤖 Generated with Claude Code