refactor(lokee): one declaration per wire contract, and stop casting node data - #257
Merged
Conversation
Migrate already snapshots Lokee before and after. Assert the History graph for demo_b: fn_order_total shows Source (no Table growth) and customers shows growth after the non-destructive demo_a → demo_b sync. Co-authored-by: huy.phan9 <huyplb@users.noreply.github.com>
The skippable wizard can appear after /signup/state. Clicking Skip only in a short loop left Playwright on Loading… and timed out the Postgres flow. Co-authored-by: huy.phan9 <huyplb@users.noreply.github.com>
…ript) Cloud VMs cannot reach the developer laptop. Copy the docker-compose foxdb credentials into apps/e2e/.env.example and add a script that reseeds demo_a/demo_b then runs test:e2e:postgres against a local npm run dev. Co-authored-by: huy.phan9 <huyplb@users.noreply.github.com>
…node data Seven types were declared twice — once in lokee-weave.module.ts, once hand-copied into lokeeApi.ts — with nothing checking the copies against each other. Two had already drifted: `source` was widened from a four-value union to `string` on both LokeeVersion and LokeeHistoryEvent, and VersionGraphObject.schemaName was declared on a field no producer ever emits. The contract now lives once in apps/web/src/shared/lokee-wire.ts, the same place permissions.ts and server-beam.ts already serve. apps/web's tsconfig includes both src and packages, so producer, contract and consumer are checked in one pass and drift is a compile error. It does not go in @foxschema/sql: that package is published to npm and scoped to dialect knowledge, while these types carry metadata-DB primary keys, user ids and row counters. Types that genuinely are dialect knowledge — ObjectBlueprint, StoredWeaveObject, ReversalPlan — stay there and are now imported rather than hand-copied. Also: - graph()'s 25-line inline return type is now Promise<VersionGraphDTO>, and the same shape it re-spelled a second time mid-method is VersionGraphObject[]. truncatedObjects moved onto the DTO, deleting two intersections, a redundant Boolean() coercion and a parallel useState. - inspectObject rebuilt the whole object map on every open — one spread per live object, 20,000 on a schema this module budgets for — purely to add a `key` the map key already held. The row literal carries it now. - The node-data interfaces became type aliases. `extends Record<string, unknown>` was not required by React Flow (a type alias satisfies the constraint) and was defeating excess-property checks: a misspelled field in a node payload compiled clean. Verified it is now an error. NodeProps<LokeeVersionNode> types `data`, so five casts are gone and onNodeClick narrows from node.type instead of checking and casting independently. One assertion remains where React Flow's NodeTypes genuinely erases the payload type — at registration, not inside every renderer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main gained #255 (script diff) and #256 (revert connection binding) on the same files this branch refactored. Resolutions: - lokeeApi.ts / lokee-weave.module.ts: kept the consolidated declarations and folded main's additions into the shared contract — `script` / `previousScript` on ObjectInspectResult, and `connection_mismatch` on LokeeRevertErrorCode. The two sides had also drifted on the new fields (backend required, frontend optional); the shared type takes the producer's guarantee. - shared-flow.ts: kept the options-driven historyObjects. main's side had dropped the dialect guard entirely, so the postgres-only assertions ran against every dialect; supplying expectations from postgres.test.ts fixes that properly. - LokeeHistoryPage.ts: the auto-merge left a duplicate objectNamedVisible — main's non-waiting version after the waiting one, which would have won at runtime and silently reverted the fix. Removed it along with the superseded inspectorHasGrowth / inspectorHasSource. - inspectObject: #255 added a second copy of the object-map rebuild for previousScript. objectsAtVersion now carries `key`, so that spread was both redundant and a duplicate-key overwrite. Gates: typecheck clean (web + e2e), 1533 unit tests, 0 ESLint errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_9986886b-b54f-49fc-b75f-9b21f929b614) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-on cleanup to the Lokee Weave feature. No behaviour change intended —
this is a type-surface refactor plus two real defects it exposed.
The problem
Seven types were declared twice: once in
lokee-weave.module.ts, oncehand-copied into
lokeeApi.ts, with nothing checking the copies against eachother. Renaming a field type-checked cleanly on both sides and broke only in
the browser.
Three had already drifted:
sourcewas widened from a four-value union tostringon bothLokeeVersionandLokeeHistoryEvent— the UI could compare against a typoand the compiler stayed quiet.
VersionGraphObject.schemaNamewas declared on a field no producer everemits and no consumer reads.
script/previousScript(added by feat(lokee): table click — column type/constraint subtitles + GitHub script diff #255 while this was in flight) arerequired on the backend and were optional on the frontend.
The fix
One declaration per contract in
apps/web/src/shared/lokee-wire.ts— thedirectory
permissions.tsandserver-beam.tsalready use for exactly this,imported by both tiers. Because
apps/web/tsconfig.jsonincludessrcand../../packages, the repo's primary gate now checks producer, contract andconsumer in one pass: drift is a compile error.
Not in
@foxschema/sql. That package is published to npm and scoped todialect knowledge; these types carry metadata-DB primary keys, user ids and row
counters. Types that genuinely are dialect knowledge —
ObjectBlueprint,StoredWeaveObject,ReversalPlan— stay there and are now imported insteadof hand-copied.
lokeeApi.tsloses 112 lines of type declarations.Two defects this exposed
A per-open Map rebuild.
inspectObjectre-materialised the entire objectmap — one spread per live object, 20,000 on a schema this module explicitly
budgets for — purely to add a
keythe map key already held. The row literalcarries it now. #255 had since added a second copy of the same rebuild for
previousScript; that one also produced a duplicate-key spread oncekeywaspresent, which
tsccaught.Node payloads were unchecked.
extends Record<string, unknown>on the threenode-data types is not required by React Flow — a type alias satisfies the
constraint — and the index signature was defeating excess-property checking, so
a misspelled field in a
satisfiesliteral compiled clean. Converting toaliases and typing renderers as
NodeProps<LokeeVersionNode>removes fivecasts;
onNodeClicknow narrowsnode.datafromnode.typeinstead ofchecking and casting independently (a fourth node type would previously have
been silently mis-cast).
Verified: introducing
changeCoutin a node payload is now a compile error.One assertion remains at
LOKEE_NODE_TYPES, where React Flow'sNodeTypesgenuinely erases the payload type — the honest place for it.
Also
graph()'s 25-line inline return type is nowPromise<VersionGraphDTO>, andthe same shape it re-spelled a second time mid-method is
VersionGraphObject[].truncatedObjectsmoved onto the DTO, deleting twointersections, a redundant
Boolean()coercion and a paralleluseState.DialectFlowOptions(the seam
sqlite.test.tsalready uses) instead of adialectLabel === 'postgres'check inside code shared by 14 dialects.mainhad dropped theguard entirely, so those assertions were running against every dialect.
waitForInspectorLoadednow waits ondata-state="ready"(a new attribute onthe inspector) rather than string-matching "Loading blueprint…". The old wait
also matched
selected.name— a prop set synchronously on click — so itproved selection propagated, not that the fetch landed.
data-object-keyispayload-derived and can't pass early. Unit-tested, since the e2e dependency
would otherwise fail as a timeout rather than an assertion.
dispatchEvent('click')fallback is gone: it bypassed the actionabilitycheck that is the whole point of an e2e click. It now clicks React Flow's Fit
view control and retries for real.
Reviewer notes
This branch merges
main. #255 and #256 landed on the same files mid-flight.The auto-merge left a duplicate
objectNamedVisible— main's non-waitingversion after the waiting one, which would have won at runtime and silently
reverted the fix. Removed, along with the superseded
inspectorHasGrowth/inspectorHasSource. Worth a look at that hunk.Skipped deliberately: deleting the confirmed-dead
windowByTime/windowGraph/blueprintChildCounts/BLUEPRINT_CHILD_TYPES(~190 lines,zero callers). They're exported from a package staged for npm publish, so
removal is a semver decision rather than a cleanup — cheap now at 0.x.
Not verified in a browser. Graph rendering is covered by typecheck and unit
tests only; the Fit-view fallback path in particular only triggers when a node
is off-viewport.
Gates: typecheck clean (web + e2e) ·
npx vitest run→ 1533 passed / 31skipped ·
npx eslint .→ 0 errors.🤖 Generated with Claude Code
Note
Low Risk
Primarily type consolidation and test contracts; runtime behavior is intended unchanged aside from inspector stale-data fix and e2e reliability improvements.
Overview
Consolidates Lokee Weave API types into
apps/web/src/shared/lokee-wire.tsso backend (lokee-weave.module.ts) and frontend (lokeeApi.ts,graphTypes.ts) share one contract—replacing duplicated declarations that had already drifted (sourceasstring, phantomschemaName, optional vs required script fields).VersionGraphDTOnow always includestruncatedObjects; the view reads it from the DTO instead of a separate state.Backend:
graph()returnsVersionGraphDTO;objectsAtVersion/inspectObjectstop rebuilding maps to re-addkeyon every object;toCanonicaltakesStoredWeaveObjectdirectly.React Flow: Node payloads use type aliases instead of
extends Record<string, unknown>; renderers useNodeProps<LokeeVersionNode>/LokeeObjectNode,onNodeClicknarrowsLokeeNode, andLOKEE_NODE_TYPESkeeps a single registration-boundary cast.Inspector UX: Clears stale
datawhen selection changes; exposesdata-stateand payload-deriveddata-object-keyfor tests. E2E: Postgres History expectations move toDialectFlowOptions.historyObjects; inspector waits ondata-state="ready"; off-viewport graph nodes use Fit view + real click instead of synthetic events.Reviewed by Cursor Bugbot for commit c8d0dad. Bugbot is set up for automated code reviews on this repo. Configure here.