Design Workspace: local-first projects and dedicated Design mode - #79
Design Workspace: local-first projects and dedicated Design mode#79sambitcreate wants to merge 68 commits into
Conversation
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Two things need attention before merging: a startup-ordering hazard in the recovery wiring that can abort app launch after a crash, and a question about whether model-generated revisions ever join an existing artboard lineage (the History surface). Details inline and below.
Reviewed changes
- Durable project store (
design-projects.json, CAS revisions, lazy legacy-chat migration) plus the project library UI with cascade-preview delete and ordinary chat deletion routed through project confirmation. - Canvas persistence: React Flow arrangement, device/flow viewport, and active revisions debounce-persisted through
designer:updateProject; reference images in a content-addressed0600store. - Deterministic offline ZIP export core (fixed order/timestamps/modes, portable-path and credential gates) with real-browser offline acceptance.
- Continue-in-workspace handoff: journaled coordinator state machine with proof-based rollback, effect ledger, managed-worktree default, and startup reconciliation.
- Local design-system context: attach/refresh/detach with per-source freshness proof and a path-free model-context projection.
- Durable comments and bounded direct edits (prototype origin → new immutable revision; connected origin → reviewed Designer Action).
- Durable multi-file Designer Actions with a journaled apply/rollback/undo transaction state machine.
- Contained Vite/Next preview transport: proof-bound loopback HTTP proxy, HMR WebSocket proxy, service-worker capability transport, script detection, and process-group teardown.
- Startup wiring and fail-closed availability reporting for the new stores; onboarding bento tile and archived plans/docs.
Note: five substantive new files (design-workspace.tsx, design-comments-panel.tsx, design-handoff-recovery.tsx, renderer/shared/design-projects.ts, design-comments.ts) were dropped from the extracted diff as "(binary file or no changes)"; they were reviewed from the working tree instead.
⚠️ Model-generated revisions never join an existing durable lineage
With a saved project, durableArtifactGroups (in renderer/components/design-workspace.tsx, around lines 98–134) builds artboard groups strictly from each project node's artifactMediaIds. A model-generated artifact with a new mediaId — even one sharing the title of an existing artboard, which the shipped MVP contract defined as "another revision of an artboard" — is unclaimed and becomes a brand-new group, and buildDurableCanvas (lines ~1570–1632) then persists it as a new node with a new lineage: id. Only the direct-edit path appends to an existing lineage (design-direct-edit-service.ts artifactMediaIds: [...ids, mediaId]).
The consequence: after the first project save, repeated "revise this artboard" turns spawn duplicate-titled artboards instead of versions, and the History tab (a headline feature) can never accumulate model-generated revisions — its per-lineage history will stay single-revision except for direct edits. The pre-durable renderer grouped same-title artifacts into one artboard with a version selector, so this is a behavior change for existing users.
Technical details
# Model-generated revisions and durable lineage
## Affected sites
- renderer/components/design-workspace.tsx:98-134 — `durableArtifactGroups` claims only `node.artifactMediaIds`; every other entry (any new model artifact, same title or not) becomes its own group with id `design-artboard:<artifact.id>`.
- renderer/components/design-workspace.tsx:1570-1632 — `buildDurableCanvas` persists each group as a node; a group with no prior node gets `lineage:<first artifact id>` (a fresh lineage).
- main/services/design-direct-edit-service.ts:250-266 — the only path that extends `artifactMediaIds` (and thus the only path that can produce multi-revision History today).
## Required outcome
- A model revision of a selected artboard must end up in that artboard's lineage (or the product contract must explicitly change to "every prompt creates a new artboard" and the History empty-state/UI copy updated accordingly).
## Suggested approach (options for the human)
- Have the design-mode `render_artifact` accept an explicit target lineage/revision anchor (main-validated against the selected target), or
- In the renderer, link a live artifact that matches the currently selected artboard's lineage to that node before `buildDurableCanvas` runs (still never title-based merging).
## Open questions for the human
- Is always-new-artboard the intended durable behavior with a follow-on plan, or a gap? The `design-project-persistence.test.tsx` pins the current new-node behavior, and the migration ADR only justifies title-agnostic grouping for legacy chats — neither addresses the post-project model-revision flow.ℹ️ Nitpicks
- The renderer mints a fresh random
gestureIdper direct-edit invocation, so the deterministic proposal/undo identity documented indesign-comments-direct-edits.mdonly holds if the caller replays the exact same gesture id; a retry after an ambiguous failure creates a second revision instead of converging. Recovery discards the stranded pending row at startup, so this is cosmetic today — but the docs overstate the idempotency guarantee.
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in the incremental delta — both commits since the last review verify clean. The review stays non-approving only because the previously flagged items (the startup-ordering hazard at
main/index.tsand the durable-lineage question) remain open; none of these commits touch that wiring.
Reviewed changes
- Deferred the cyclic
llmClientsingleton dependency behind a call-time method adapter in the workspace application service composition, with a source-shape regression test and a papercuts note documenting the incident. - Consolidated the merged
app:checkForUpdatesIPC registration inmain/index.ts(single registration, sender-boundary check preserved,app:getUpdateState/app:restartToUpdateuntouched).
Both changes verified against the working tree: the adapter matches the Pick<typeof llmClient, "cancelWorkspaceAndSettle"> deps type and covers the service's only call sites (workspace-application-service.ts:214,255), the lazy binding access is safe under ESM cycle evaluation, and the updater source-pin tests plus the full workspace-application-service.test.ts suite pass locally.
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No critical issues in this incremental delta — one minor suggestion inline on the new test-stability commit.
Reviewed changes
- Added DOM round-trip assertions to the source-designer E2E spec: after apply the test waits for
exact-childto read "Saved" and after undo for it to read "Save", so the file → Vite HMR → DOM round trip is observed before the next page listener is installed (papercuts note added for the CI flake).
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found — the single test-stability commit since the prior review is sound, targets a real CI-load flake root cause, and passes stably.
Reviewed changes
The delta reviewed is one commit (f6b84fd6, 1 file, +3): pinning the supervisor's accounting clock in the "seals an uncooperative tree only after bounded cancellation grace" test.
- Added
now: () => 0to theSubagentSupervisortest options (subagent-supervisor.test.ts:2200). The flake mechanism checks out against the source:startedAt = this.now()at construction and the admission-time checkfloor(treeDeadlineMs - (now() - startedAt))previously ran against wallperformance.now(), so under aggregate CI load the 100 ms tree budget could decay to ≤ 0 before admission and divert the run into the pre-admission expiry throw — bypassing the abort-then-grace settlement the test asserts. Pinning the clock keeps the budget constant while the real tree timer (100 ms) and cancellation grace (10 ms) still exercise wall time; the second-execute/tree deadline elapsed/rejection rides thetreeExpiredflag the first run's timer sets. Consistent with the file's existing fake-clock pattern (lines 433, 462, 2120), and the test passes 3/3 stable runs at this head.
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found in this delta — the single styling commit since the prior review verifies clean, and the change actually resolves a real contrast gap on the mode menu's highlight surface.
Reviewed changes
- De-pilled the mode picker trigger: the filled
bg-controlcapsule (rounded-[22px], hover/open backgrounds) became a quiet 2x bold text row with only a chevron affordance (rounded-[10px] border-0 bg-transparent px-1 shadow-none), and the menu rows now flip their label and description totext-accent-foregroundviagroup-data-[highlighted]when the Radix item sits on the accent highlight background. - Re-pinned the appearance source-shape test to the new classes and highlight treatment, rejecting the old pill classes.
The highlight fix adopts the exact idiom already used by DropdownMenuCheckboxItem sublabels (ui.tsx:1251) and the chat-sidebar organizer rows, so the mode menu now stays legible on the accent surface instead of keeping text-primary/text-secondary over bg-accent. The trigger keeps its focus-visible:ring-2 ring-focus-ring treatment (non-text control), no residual references to the removed classes exist anywhere, and appearance.test.ts passes 13/13 at this head.
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found in this delta — the single test-stability commit is sound, targets a real
--fail-on-flaky-testshazard, and weakens no assertion.
Reviewed changes
- Granted the source-preview integration test its own deadline:
test.slow()plus a rationale comment at the top of the "local React preview binds the exact nested element to its JSX range" Playwright test (tests/generative-ui/source-designer.spec.ts). The test boots a real Vite dev server throughsourceDesignPreviewServiceand verifies apply plus undo across three full navigations with a bounded retry loop, a sibling loopback probe, and filesystem fixture setup — legitimately able to exceed the suite'stimeout: 30_000on a shared runner. Because the suite runs under--fail-on-flaky-testswith CIretries: 1, a slow-but-valid first attempt previously failed the whole run;test.slow()triples only this test's deadline to 90 s.
Verified against source: the config claim matches playwright.generative-ui.config.ts (timeout: 30_000), the call sits inside the test body above the first await (the documented idiom), and no assertion or expectation was loosened — the change is deadline-only.
DeepSeek Flash ｜ 𝕏
|
@pullfrog Please review the current exact head 80827f4. This update makes Design artifact publication and recovery crash-safe, tightens published/live source authority, preserves optimistic/detached preview continuity, and closes lifecycle ambiguity edge cases. All local focused and full suites are green. |
|
Run failed. View the logs →
|
There was a problem hiding this comment.
Important
Two real gaps in the new durable-publication delta: a terminal (suppressed) publication is surfaced to the renderer as an in-progress reconciliation that can never converge, and cancelled/failed design turns leave their generated artifact permanently unreachable while the durable transcript still advertises it. The CAS machinery itself is sound — no rollback, double-append, or corruption path found — but the failure UX contradicts the PR's "actionable reconciliation retry" claim.
Reviewed changes
- Added a main-owned artifact publication protocol: staged Generative UI records carry validated
{ projectId, lineageId }ownership (deterministic lineage/node IDs), transition candidate → eligible (only after a successfully completed terminal turn, before the assistant-message append) → published (only after the append is durable), with semantic compare-and-swap so a revision advances only from its exact active base and a crash between stores replays idempotently without rolling a lineage back. Failed/cancelled/interrupted candidates are suppressed and can never mutate project history. - Added startup reconciliation (
designGeneratedRevisionService.reconcileAtStartup, ordered before generic interrupted-artifact recovery): eligible rows publish only with exact full-descriptor proof in a durable assistant message; rows without that proof are suppressed; direct-edit and direct-edit-revert pending rows are handled in a separate loop. - Added
DesignArtifactRecoveryService: journal-proven repair (only bytes of the exact owning generation, never a same-title neighbor) that either recovers a new revision, removes a missing/damaged non-active history entry, removes a broken artboard, or finalizes an interrupted eligible publication, all under artifact-store snapshot guards linearized against staging/commit and project CAS. - Hardened every Design read surface (preview srcdoc, export, code view, direct edit, comments, context, handoff, health) to accept only exact published project-owned sources;
committedRecoverySourceFornever crosses IPC; a live optimistic lane (DesignLivePreviewAuthority, per-document/chat/media, suspend/resume/revoke) covers only the in-flight candidate. - Made
DataStore.withSerializedSnapshothold the writer queue for cross-store repair CAS; addedreconcileDeletePublication,DesignProjectPublicationUncertainError, immutable-artboard-guard on generic canvas updates, and startup prune of unreferenced reference assets. - Renderer (80827f4): a send-time persistence barrier (serialized canvas save + captured target snapshot feeds the main preflight), optimistic claims with a reconciliation notice + Retry, per-media generated-source hydration with retryable per-revision errors, provisional-position migration to durable artboard identity by media ID, a sidebar artifact-recovery flow (inspect/recover/regenerate) with truthful "Open to regenerate" actions, and explicit suspend/resume of detached design-preview authority.
Findings are inline.
ℹ️ Nitpicks
- Sending while the 350 ms canvas debounce persist is in flight can fail once with "This Design Project changed in another window": the send barrier serializes behind the in-flight save but returns the pre-flush local snapshot whose revision lags main, so the preflight comparison rejects a healthy single-window send. Fail-closed and self-heals on the next click; the copy is misleading in that window.
DeepSeek Flash ｜ 𝕏
|
Reviewed the exact head What verified clean
Why approval is withheld I independently traced and confirmed the residual blind spot that review 5095762484's thread (still open at Task list (6/6 completed)
|
There was a problem hiding this comment.
ℹ️ No critical issues in this delta — both prior findings are cleanly resolved, and one informational observation on a residual terminal-suppression blind spot (the retry/detached paths) follows inline.
Reviewed changes
- Added the Keep draft / Discard stop flow: an explicit user stop with partial Design output and an interactive desktop owner now prompts before terminal persistence, and only an exact "Keep draft" answer crosses the eligibility barrier (
design-cancel-draftquestionnaire kind, structurally pinned by the shared parser so the renderer cannot reword the decision). - Reversed the failed/discarded-turn lifecycle from suppress-and-commit to exact-delete:
decideDesignGenerationPublicationnow discards the staged rows (discardPendingunder exact chat + generation + media ownership, one blind retry) before the assistant message is appended without the HTML descriptors, so cancelled turns no longer orphan committed bytes, advertise dead cards, or consume the per-chat artifact budget. - Classified publication failures into terminal
"suppressed"vs durable"retryable"and threaded the outcome throughchat:error/ChatError, the detached terminal-sync projections, and the Aiden Remote error mapping, with distinct, truthful user copy for each. - Made reconciliation actionable:
designer:openProjectnow runsreconcilePersistedChatunder the append mutation lane when the chat is idle (re-checking detached generation ownership), reports durable retry eligibility in the open result, and seeds the inline reconciliation notice on route entry before the composer paints; the Retry button genuinely republishes through that lane. - Serialized recovery, terminal adoption, and generation admission with a renderer-side
DesignOperationFence, blocked sends while a reconciliation notice is armed or a detached publication is unsettled, and cleared optimistic state on suppression so Retry is never offered for an already-terminal outcome. - Preserved detached terminal projections (
published/retryable) until the adopting pane acknowledges after refreshing durable project authority, and stopped cached sidebar snapshots from exposing the composer before the authoritative open recovery completes. - Filtered Design transcript cards to project-owned media, dropped the "preview remains available" claim when no optimistic preview exists, excluded Design-owned rows from generic HTML recovery, and rebuilt the queued-send canvas snapshot after the earlier debounce save (closing the prior debounce-persist nitpick).
Verified at this head: the focused publication/recovery/shared suites pass (188 tests across test:design-recovery, the generated-revision/publication/artifact-store/live-authority/terminal-sync/ipc-stream/ask-user-question/design-workspace sets); the 6 test:generative-ui failures are confined to source-design-preview tests that need the vendored react-grab-primitives.js host library, which this environment lacks — none of the commit's files are involved.
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
Important
The new delta resolves both findings from the 80827f4 review cleanly, but it ships with a broken source-shape assertion: one added regex in design-project-sidebar.test.tsx demands layout copy that lives in the sidebar, so the suite fails 1/27 at this head and test:sidebar (chained into pretest) turns CI red. Details inline.
Reviewed changes
- Added the Keep draft / Discard stop flow: an explicit user stop with partial Design output now asks the owning desktop renderer (
design-cancel-draftquestionnaire, answerable only via the two option buttons; navigation, Close, custom typing, and Skip are hidden for it). Keep crosses the same eligibility barrier as completed output; Discard exact-deletes the staged rows and omits the artifact descriptors from the durable assistant message. Dismissed prompts, failed turns, headless clients, and non-user cancellations take the discard path without UI, and cancellation escalation (window close, deletion, shutdown) detaches the pending question sopersistAssistantcan never hang. - Made terminal publication honest end-to-end:
chat:errornow carriesdesignPublication: "retryable" | "suppressed"throughtypes.ts, the IPC stream,chat-terminal-sync, and the Aiden Remote mapping. Suppressed output is terminal — the renderer clears its optimistic state, never arms Retry, and shows "conflicted with newer project history … generate again" — while only genuinely eligible rows arm the reconciliation notice whose Retry now re-runs open-project reconciliation and converges. - Made reconciliation recoverable and re-entrant:
designer:openProjectruns under the project mutation lane, re-checks chat busy state, invokesreconcilePersistedChat, and returnsDesignProjectOpenResultV1withdesignPublication: "retryable"when eligible rows remain after the attempt.DesignGeneratedRevisionServicenow discards interrupted uncommitted candidate/suppressed rows under exact generation + publication CAS, suppresses committed anomalies, never touches live or recovery-prefixed generations (viaisGenerationActive/isChatGenerationActivefrom the live-preview authority), and isolates per-chat cleanup failures at startup. - Hardened the artifact-store contract:
discardPendinggained anexpectedDesignPublicationCAS, genericrecover()andreconcilePersistednever advertise Design-owned candidates, and the transcript plan (htmlArtifactTranscriptPlan) filters design media not claimed by the open project so suppressed/conflicted rows leave no dead card. - Fixed the send-vs-debounce flush false rejection: the send barrier now rebuilds the latest canvas only after acquiring the barrier (
nodesRef), andDesignOperationFenceserializes reconciliation, generation admission, and detached terminal publication in the pane. - Preserved detached terminal handoffs:
chat-terminal-syncretainspublished/retryableprojections until the Design conversation adopts them (acknowledgeDetachedDesignPublication), and the pane's route-open seeding and detached-adoption effects keep the composer blocked until main reports the publication resolved.
ℹ️ The retryable "preview available" claim is presence-based, not read-backed
While suppression is now honest, the retryable banner copy is only as truthful as the artifact count that arms it. Once a turn is terminal and its rows are committed-but-eligible, every fresh stored read fails the published-only gate (isUsablePublishedDesignSource) with "That Design revision is damaged", and the live-preview authority is revoked. The claim "The generated preview remains available while you retry" therefore holds only while the already-mounted preview iframe stays alive; any remount or fresh fetch during the retryable window — most concretely the detached-adoption path, which seeds the notice with a non-empty artifact set after the authority is gone — renders a dead node with that error while the banner keeps promising the preview, until the Retry republishes the row and the error self-clears. Consider deriving previewAvailable from the same read capability the preview itself needs, or wording the notice for the no-read case ("Retry to finish restoring project history").
Note: 1 inline comment(s) dropped because they did not anchor to lines inside the PR diff:
renderer/components/design-project-sidebar.test.tsx:104-107(RIGHT) — file has no textual diff (binary, pure rename, or mode change)
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
ℹ️ This delta itself is clean — the route-recovery test pin lands and the sidebar suite passes 8/8, resolving the only failing test my previous review flagged. Approval is withheld because the open thread on
renderer/main/chat-pane.tsx:963(terminal suppression has no channel on the retry or detached paths) is untouched by this delta and still leaves a genuinely non-convergent Retry for suppressions discovered during a Retry or after reload.
Reviewed changes
- Fixed the source-shape test my previous review flagged:
design-project-sidebar.test.tsxnow asserts whatDesignProjectRouteactually does —setDesignPublication(openResult.designPublication), theopenedRouteIdentity !== projectOrLegacyChatIdre-entry guard,designPublication={designPublication}, andonDesignPublicationResolved={() => setDesignPublication(undefined)}— replacing the assertion that demanded sidebar-only copy insidechat-layout.tsx. Sidebar assertions (const project = openResult.project, the retryable toast in the export flow) are unchanged.
Verified at this head: test:sidebar-style run of the file passes 8/8 (it failed 1/8 at 5c003d0), and each new pattern matches the implementation (chat-layout.tsx:389, 424, 437-438). Test-only delta; no behavioral surface.
DeepSeek Flash ｜ 𝕏
|
@pullfrog Please review exact head 8735cfc. This closes the valid terminal-suppression thread from the prior review: retry-time and detached semantic conflicts now propagate as terminal suppressed outcomes, clear Retry/optimism, preserve newer project state, and unblock the composer. The preview copy is also conservative and no longer infers read capability from artifact presence. Focused regression set: 106/106; type checks, lint, and diff hygiene clean. |
There was a problem hiding this comment.
✅ No new issues found — the single commit since the prior review (
8735cfc5, 9 files) closes the last open terminal-suppression thread: semantic conflicts that surface during a Retry or after a route-open reconcile now propagate as terminal"suppressed"outcomes, and detached suppressed turns retain their main-owned explanation until the adopting pane consumes it. Verified clean at this head: the generated-revision (15/15), chat-terminal-sync (20/20), and project-persistence (18/18) suites, fulltsc --noEmit, eslint on the changed files, andgit diff-tree --check.
Reviewed changes
- Made reconciliation report its own suppression:
reconcileRecordsnow returns whether any eligible rows were suppressed by a semantic CAS conflict during the attempt, andreconcilePersistedChatreturnsdesignPublication: "suppressed"when suppression happened and nothing eligible remains — sodesigner:openProjectand its Retry lane can finally surface the terminal outcome they themselves produce instead of arming an endless Retry banner (design-generated-revision-service.ts:200,259). - Retained detached suppressed terminals: a suppressed
chat:errornow keeps its projection (with the main-ownedterminalErrorcopy retained inDetachedLifecycleProjection) until the owning pane adopts it, mirroring the existing retryable retention so a returning renderer can show why the revision was not added (chat-terminal-sync.ts:704). - Consolidated renderer adoption into
adoptSuppressedDesignPublication, applied consistently on the route-open suppressed result, the retained detached suppressed projection (exactterminalError, then acknowledgement), the reconciliation Retry result, the live-streamchat:error, and the post-completion openProject check — each cleared the reconciliation notice/optimistic artifacts before showing the honest terminal error, with the newer-project revision guard applied before adoption where anopenProjectsnapshot is in hand (chat-pane.tsx:307,715,814,999,1513,1635). - Made the reconciliation notice conservative: the
projectReconciliationHasPreviewprop and the "The generated preview remains available while you retry" copy are gone — no read capability is inferred from artifact presence anymore; the notice only says "Retry to finish restoring project history." (design-workspace.tsx:2372,3808). - Pinned each behavior with real tests: the main-side attempt-scoped suppression test (a second reconcile reports
{}, so suppression is projected once by the observer), the detached suppressed-retention/acknowledgement test, and the source-shape assertions that order route-open suppression adoption ahead of retryable seeding.
One pre-existing session-scoped residual remains outside this delta's reach (already accepted in earlier rounds): suppression discovered only after a full app restart has no live channel to surface its copy — a niche crash-plus-cross-window-conflict corner, since startup reconciliation publishes any row whose descriptor is durable and only suppresses proof-less candidates that were never advertised.
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
ℹ️ The delta cleanly closes the last open Pullfrog thread (terminal suppression on the retry and detached paths) — one sentence-level copy nitpick below.
Reviewed changes
- Reported terminal suppression from foreground reconciliation:
reconcileRecords/reconcilePersistedChatnow surface a semantic conflict suppressed during the attempt asdesignPublication: "suppressed"throughdesigner:openProject, and the route-open layout effect, the Retry handler, the generation-terminalopenProjectrefresh, and the detached-adoption effect all consume it into an honest terminal error — clearing the retryable banner, optimistic artifacts, and preview handles — instead of re-arming Retry on a banner that can never converge. - Retained detached suppressed turns:
chat-terminal-sync.tskeeps the"suppressed"projection with the main-owned terminal copy (terminalError) until the returning pane adopts and acknowledges, anddisplayedStreamingArtifactsdrops the dead artifact card for suppressed projections. - Removed the presence-based "preview remains available while you retry" claim from
ProjectReconciliationNotice(dropping thepreviewAvailableprop entirely), retiring the informational nitpick from the prior review. - Added genuine regression pins: a service test proving suppression is projected only by the attempt that observes the conflict (rows drain eligible → suppressed, active media untouched, second reconcile returns
{}), a terminal-sync test asserting the suppressed projection retains exact terminal truth until acknowledged, and source-shape assertions pinning the suppressed-before-retryable adoption ordering on every renderer path.
Verified against the working tree and current sources: the widened "retryable" | "suppressed" union is handled by every consumer (chat-layout route state, all ChatPane adoption sites, the sidebar export flow falls through benignly against the fresh project), retryable correctly dominates suppressed in the service when rows genuinely remain, and the renderer's layout-effect adoption cannot re-arm a retryable notice after a suppression. Focused suites at this head: design-generated-revision-service + chat-terminal-sync 35/35, design-project-persistence 18/18. Full type-check could not be re-run here — this environment's npm ci fails at the react-grab/primitives vendoring step and @xyflow/react is absent — so type-safety rests on the source-shape pins and the author's clean type-check claim.
ℹ️ Nitpicks
- The reconciliation notice now always appends "Retry to finish restoring project history." after
message(design-workspace.tsx:3831), so the most common retryable copy renders as "The Design revision is saved and waiting to be added to project history. Retry to finish restoring project history." — the doubled "project history" reads redundantly. Consider wording the trailing sentence so it does not repeat a term the message already ends with (e.g. "Retry to finish adding it."), while keeping the truthful terminal copy for the suppressed case unchanged.
DeepSeek Flash ｜ 𝕏
|
Confirmed: Verified against this exact head:
No new issues. The delta since the last Pullfrog review ( Task list (5/5 completed)
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
The delta since the prior Pullfrog review is one commit (4bda6779 fix(design): clarify recovery guidance, 2 files, +2/−2):
- Reworded the reconciliation-notice trailing sentence from "Retry to finish restoring project history." to "Retry to finish adding it." (
renderer/components/design-workspace.tsx:3831), adopting the exact suggestion from the prior review and eliminating the doubled "project history" in the common retryable copy. - Re-pinned the
design-project-persistencesource-shape assertion to the new wording, which now genuinely rejects the old copy.
The fix is sound. I verified it reads coherently against every message that flows into the notice (the retryable route-open, detached, and terminal-refresh variants), confirmed the suppressed publication uses a separate honest terminal copy (llm-client.ts:3423) that never reaches this notice, and found zero residual references to the old sentence anywhere in the repo.
DeepSeek Flash ｜ 𝕏
1 participant
Summary
Design storage contract
Edge-case review
Verification