feat(console): cards, actions, mentions + stripe surface nav + live doc wire - #60
feat(console): cards, actions, mentions + stripe surface nav + live doc wire#60Travis-Gilbert wants to merge 8 commits into
Conversation
Cmd/Ctrl+L (handoff named choice 3, Cursor muscle memory) is a browser accelerator reserved for the address bar, so a browser tab never delivers the keydown to the page and the omnibar cannot capture it. It works in the desktop shell but not in a browser. Add Cmd/Ctrl+K (the command-palette convention browsers do deliver) as an additional Ask trigger, keeping Cmd/Ctrl+L for the desktop target and double-Shift for Search. The field's aria-keyshortcuts advertises all of them. e2e asserts Ctrl+K opens Ask.
…oc wire HANDOFF-CARDS-ACTIONS-MENTIONS K1-K7: - K1 card templates as data (person/task/generic seeds) + one card engine (card.full / card.compact) rendering any kind through the block contract; malformed or missing template degrades to generic with a note, never errors. - K2 mounts: inspector leads with the compact card above the field table; cards.grid descriptor renders an ObjectQuery as virtualized faces at Twenty density; a Cards surface joins the seeded screens. - K3 action sheet, three entries into one sheet (/do composer, todo-block arrow + Alt+Enter on Galley task items, Action verb on inspector and cards); staged context is always visible; auto-suggest adds removable chips; save as rule names IX6. - K4 delegate wire: For me posts the pack to /api/harness/delegate and names the harness refusal; With me stages visible object refs above the composer. - K5 mentions detection engine (crates/commonplace/src/mentions.rs): exact and normalized-exact tiers, span slices back to the original atom text, incremental on ingest and on object create/update, deterministic candidate id as the dedup and dismissal-suppression key. - K6 mentions surface: linked/unlinked counts, passage list with the matched span highlighted, confirm writes the MENTIONED_IN edge with its basis on the edge, dismiss records the negative signal. - K7 gates: template render-model tests, the pack-equals-chips invariant test, the candidate-to-confirmed-edge round trip, and cards/actions/mentions e2e with visual baselines. Follow-up fixes from live review: - Screen navigation moves out of the toolbar dropdown into the leftmost stripe (the stripe surfaces group): surfaces on top, a divider, then the active surface's tool windows, one bar in the original chrome tokens. The toolbar shows the active screen as a quiet breadcrumb. - Documents and code files ride the live object wire so edits persist to the backend; content rides as extra properties (markdown / content) matching the view projection, the console filters client-side so slug/id predicates hold, and the backend is seeded once (idempotent by slug/path). Verify: 40 crate tests, 29 console unit tests, 25 e2e, and the fence, register, contrast, and motion gates all pass.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe console adds template-driven card surfaces, virtualized grids, relation inspection, action-sheet handoff, staged context, incremental mentions, surface-rail navigation, and Ctrl/Cmd+K Ask access. E2E fixtures and backend integrations support the new flows. ChangesConsole cards and domain data
Action handoff
Incremental mentions
Shell navigation and test infrastructure
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CardView
participant ActionSheet
participant DelegateRoute
participant ConsoleHarness
User->>CardView: select card and open action
CardView->>ActionSheet: provide context chips
ActionSheet->>DelegateRoute: submit action pack
DelegateRoute->>ConsoleHarness: forward handoff request
ConsoleHarness-->>ActionSheet: return handoff status
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b690111c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onClose(); | ||
| requestAnimationFrame(() => { | ||
| document | ||
| .querySelector<HTMLTextAreaElement>('[data-thread-composer-input]') | ||
| ?.focus(); |
There was a problem hiding this comment.
When a user chooses With me from the Cards surface, that surface has no ThreadView or composer, so this closes the only visible chip set and the focus query finds no target. The staged refs remain only in Zustand until the user manually navigates to Workspace; with NEXT_PUBLIC_CONSOLE_CHAT_URL unset, ThreadView returns its unavailable state and they cannot be viewed, removed, or sent at all. Navigate to/open a thread surface or keep a visible staging affordance before closing the sheet.
Useful? React with 👍 / 👎.
| const before = snippet.slice(0, snippetStart); | ||
| const match = snippet.slice(snippetStart, snippetEnd); | ||
| const after = snippet.slice(snippetEnd); |
There was a problem hiding this comment.
The mention backend explicitly records snippet_start and snippet_end as Rust character offsets, but JavaScript String.slice consumes UTF-16 code-unit offsets. For any passage with an astral Unicode character before the mention, this highlights the wrong substring (and can split a surrogate pair), even though the recorded span is valid; convert the offsets to JS string indices or slice via code points before rendering.
Useful? React with 👍 / 👎.
| export function UnlinkedMentionsChip({ host, object }: { host: BlockHost; object: ObjectRef }) { | ||
| const { candidates } = useMentionCandidates(host, object.id); | ||
| const unlinked = candidates.filter((entry) => entry.status === 'unlinked').length; |
There was a problem hiding this comment.
Confirming or dismissing a candidate only calls refresh() on the MentionsSection hook instance. The header chip creates a separate useMentionCandidates instance here, while HttpBlockHost subscriptions are a no-op, so its unlinked total remains stale until some unrelated parent rerender. This leaves the card's visible mention count incorrect immediately after the user resolves a mention.
Useful? React with 👍 / 👎.
| // Seed the backend's document fixtures once so the Documents surface has | ||
| // editable, persistent content (the file-editing wire). | ||
| void host.ensureSeedContent(); |
There was a problem hiding this comment.
On a fresh object API, the mounted document views query before this fire-and-forget seeding operation completes. queryLiveDomain returns an empty set with a no-op subscription, so ViewInstanceHost keeps its empty result after the seed creates the documents; the initial Console brief can remain blank until the user remounts the view or reloads. Seed before rendering document queries, or notify/refetch the affected views after creation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR adds the “cards/actions/mentions” round to the console and backend: incremental unlinked-mention detection (with confirm/dismiss), a kind-templated card rendering engine + cards surface/grid, and a unified action sheet that stages visible context and delegates to the harness. It also moves screen navigation into the left stripe and wires document/code editing to persist through the live object seam (with idempotent seed-to-backend bootstrapping), backed by new unit/e2e coverage.
Changes:
- Add Rust mentions engine producing
mention-candidateobjects, plus confirmation writingMENTIONED_INedges; hook evaluation into ingest and object create/update. - Add console card template data model + card renderer (
card.full/card.compact) +cards.grid, plus mentions UI surfaces and inspector integration. - Add action sheet (three entry points), With-me staging into thread, For-me harness delegation route, surface-rail navigation, and expanded e2e fixtures/tests.
Reviewed changes
Copilot reviewed 33 out of 40 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/commonplace/src/mentions.rs | New incremental unlinked-mention detection + edge confirmation and tests |
| crates/commonplace/src/lib.rs | Export mentions module |
| crates/commonplace/src/ingest.rs | Trigger mention evaluation during ingest |
| crates/commonplace/src/block_view.rs | Trigger mention evaluation on create/update; confirm-hook for candidates |
| apps/console/src/views/ThreadView.tsx | Show staged With-me refs above composer |
| apps/console/src/views/registry.tsx | Register card.full / cards.grid view descriptors |
| apps/console/src/views/RecordInspector.tsx | Add compact card header + action entry + cross-kind fetch hinting |
| apps/console/src/views/MentionsSection.tsx | Mentions surface UI (counts, passages, confirm/dismiss) |
| apps/console/src/views/GalleyDocView.tsx | Doc switching state reset + todo action affordance integration |
| apps/console/src/views/CardView.tsx | Card engine rendering + grid virtualization + action entry + mentions embedding |
| apps/console/src/motion/motion-tokens.ts | Motion inventory row for action sheet |
| apps/console/src/lib/workspace-seed.ts | Seed Cards surface + punch-list doc |
| apps/console/src/lib/thread-store.ts | With-me staged refs + include them in outbound message text |
| apps/console/src/lib/shell-store.ts | Action sheet origin/chip model + inspector selection caching + helpers |
| apps/console/src/lib/console-host.ts | Live-wire docs/code + seed-to-backend bootstrap + card-template local pool |
| apps/console/src/lib/console-host.test.ts | Update layout seed expectations (Cards surface) |
| apps/console/src/lib/card-templates.ts | Card template schema/validation + seed templates + resolution logic |
| apps/console/src/lib/card-templates.test.ts | Unit tests for card template parsing/resolution |
| apps/console/src/lib/action-pack.ts | Pack serialization + “pack equals chips” invariant probe |
| apps/console/src/lib/action-pack.test.ts | Unit tests for pack/chips invariant |
| apps/console/src/components/shell/Omnibar.tsx | Add Ctrl/Cmd+K shortcut + updated aria-keyshortcuts |
| apps/console/src/components/shell/MainToolbar.tsx | Remove toolbar surface switcher; add breadcrumb naming active surface |
| apps/console/src/components/shell/IntuiShell.tsx | Add surface-rail nav group + mount ActionSheet |
| apps/console/src/components/shell/icons.tsx | Add new surface-nav icons (Workspace/Cards/Model) |
| apps/console/src/components/shell/ActionSheet.tsx | New action sheet UI (autosuggest, destination, delegate, With-me staging) |
| apps/console/src/components/ConsoleApp.tsx | Intercept /do to open sheet + ensure seed content on startup |
| apps/console/src/app/api/harness/delegate/route.ts | New harness delegate passthrough API route |
| apps/console/playwright.config.ts | Configure chat URL for e2e so composer is live |
| apps/console/e2e/stub-data-api.mjs | Expand stub data API with domain fixtures + mentions + docs/code pools |
| apps/console/e2e/proof-workspace.spec.ts | Update expectations for live composer in e2e |
| apps/console/e2e/omnibar.spec.ts | Add Ctrl+K test + switch surface nav to stripe rail |
| apps/console/e2e/cards.spec.ts | New e2e suite for cards/actions/mentions + baselines |
| .railwayignore | Ignore additional build/test artifacts for Railway deploys |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function packEqualsChips(pack: ActionPack, chips: readonly StagedContextChip[]): boolean { | ||
| if (pack.context.length !== chips.length) return false; | ||
| const key = (entry: PackContextEntry) => | ||
| `${entry.kind}|${entry.label}|${entry.object_id ?? ''}|${entry.text ?? ''}`; | ||
| const packKeys = pack.context.map(key).sort(); | ||
| const chipKeys = chips.map((chip) => key(entryFromChip(chip))).sort(); | ||
| if (packKeys.some((value, index) => value !== chipKeys[index])) return false; | ||
| const firstOrigin = chips.find((chip) => chip.source === 'origin'); | ||
| if (firstOrigin && pack.context.length > 0) { | ||
| return key(pack.context[0]) === key(entryFromChip(firstOrigin)); | ||
| } | ||
| return true; | ||
| } |
| export function ActionSheet({ host }: { host: BlockHost }) { | ||
| const origin = useShellStore((state) => state.actionSheetOrigin); | ||
| const close = useShellStore((state) => state.closeActionSheet); | ||
| if (!origin) return null; | ||
| return <ActionSheetOpen key={origin.chips[0]?.id ?? 'blank'} host={host} onClose={close} />; | ||
| } |
| const [docId, setDocId] = useState<string | undefined>(doc?.id); | ||
| if (doc && doc.id !== docId) { | ||
| setDocId(doc.id); | ||
| setText(typeof doc.properties.markdown === 'string' ? doc.properties.markdown : ''); | ||
| setMode('read'); | ||
| } |
| const staged = get().staged; | ||
| const refLine = staged | ||
| .map((ref) => (ref.objectId ? `@[${ref.label}](${ref.objectId})` : ref.label)) | ||
| .join(' '); | ||
| const text = refLine ? `${rawText}\n${refLine}` : rawText; | ||
| if (staged.length > 0) set({ staged: [] }); |
| } else { | ||
| // An edited atom re-evaluates incrementally; existing | ||
| // candidate ids (any status) stay untouched. | ||
| self.evaluate_mentions_for_atom(&item.id)?; | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (22)
apps/console/src/components/shell/IntuiShell.tsx-161-169 (1)
161-169: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftCommit surface switches atomically or serialize them.
Each click launches multiple unawaited
host.emitoperations. Rapid clicks or a failed receipt can leave multiple—or no—surfaces active, after whichactiveSurfaceIdselects the wrong persisted surface. Route this through one atomic host action, or queue the complete switch and handle failures before accepting another request.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/components/shell/IntuiShell.tsx` around lines 161 - 169, Update switchTo in IntuiShell so changing the active surface is atomic or serialized as one complete operation rather than issuing multiple unawaited host.emit calls. Ensure rapid requests cannot overlap, and handle a failed switch before accepting another request so the persisted state always has exactly the requested surface active.apps/console/e2e/stub-data-api.mjs-254-254 (1)
254-254: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReturn shape metadata matching the selected pools.
Line 254 can now return
person,task,doc, and other types, but Lines 272-274 still describe every result as a record with record-only fields. This violates the query contract and can select an incorrect renderer. Deriveshape.typesandshape.fieldsfrom the query/result.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/e2e/stub-data-api.mjs` at line 254, Update the query result construction around poolFor and the response shape metadata so shape.types and shape.fields are derived from query.types and the selected objects rather than always using record-only metadata. Preserve the existing result data while ensuring person, task, doc, and other pools advertise their correct types and fields for renderer selection.apps/console/e2e/stub-data-api.mjs-307-317 (1)
307-317: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject updates when the target does not exist.
A missing target falls through to the generic
acceptedreceipt even though this stub will never apply the update later. Return a non-success response so broken IDs and lost document edits fail the E2E flow immediately.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/e2e/stub-data-api.mjs` around lines 307 - 317, Update the action.kind === 'update' handling to return a non-success response when allStored().find cannot locate the requested target. Preserve the existing successful response and mutation for found targets, and prevent missing-target updates from falling through to the generic accepted receipt.apps/console/e2e/stub-data-api.mjs-320-327 (1)
320-327: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake seed creates idempotent.
IDs based on
pool.length + 1are deterministic only for the current mutable state. Retrying the same seed request appends another object, changing cardinality and captures. Upsert using a stable logical key or action-provided id.As per PR objectives, backend seeding must be idempotent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/e2e/stub-data-api.mjs` around lines 320 - 327, The create handling for POOLS in the action-processing flow must be idempotent: derive a stable logical key from the action or its provided id, detect an existing pool entry, and update/reuse it instead of appending a duplicate on retries. Preserve the existing response shape and return the stable object id in target_ids.apps/console/src/views/MentionsSection.tsx-134-139 (1)
134-139: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear pending state when
host.emitfails.A rejected update skips Line 137, leaving that candidate disabled indefinitely and producing an unhandled rejection. Use
try/catch/finallyand expose a retryable error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/views/MentionsSection.tsx` around lines 134 - 139, Update setStatus to handle rejected host.emit updates with try/catch/finally: always clear the candidate via setPending(null), expose a retryable error to the user, and prevent an unhandled rejection while preserving refresh only for successful updates.apps/console/src/views/MentionsSection.tsx-59-103 (1)
59-103: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftInvalidate every mounted mention query after updates.
refresh()only updates the hook instance that initiated the action. Confirming in the inspector leaves the separately mounted card chip showing its old unlinked count. Use shared query invalidation or a host subscription.Also applies to: 134-139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/views/MentionsSection.tsx` around lines 59 - 103, Update useMentionCandidates so refresh invalidates every mounted mention query, not just the initiating hook instance. Replace the local generation-only mechanism with the existing shared query invalidation or host subscription mechanism, and ensure all mounted instances reload after mention updates while preserving the current query and retry behavior.crates/commonplace/src/mentions.rs-254-292 (1)
254-292: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftReconcile candidates when an atom is edited.
Re-evaluation only creates missing IDs. If body text removes a mention or moves its span, the existing candidate and captured snippet remain unchanged and can later create an edge from stale evidence. Reconcile unlinked candidates against the current matches and define removal/update behavior for confirmed edges.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/commonplace/src/mentions.rs` around lines 254 - 292, Update evaluate_mentions_for_atom to reconcile existing candidates for the edited atom with the current match_aliases results, removing candidates whose matches disappeared and updating candidates whose spans or captured snippets changed instead of skipping existing IDs. Preserve confirmed mentions/edges according to the project’s established behavior, explicitly preventing stale candidate evidence from producing edges while retaining confirmed edge semantics.apps/console/src/app/api/harness/delegate/route.ts-19-31 (1)
19-31: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the upstream handoff request with a timeout.
A stalled harness can leave this route pending indefinitely, consuming server resources and leaving the sheet stuck in its submitting state. Add an abort timeout and map it to the unavailable response.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/app/api/harness/delegate/route.ts` around lines 19 - 31, Update the upstream fetch in the delegate route to use an AbortController-based timeout, ensuring stalled handoff requests are aborted after the configured limit. Catch the timeout/abort condition and map it to the existing unavailable response while preserving normal upstream response handling.apps/console/src/views/MentionsSection.tsx-43-55 (1)
43-55: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSlice snippets using the backend’s character-offset semantics.
Rust records Unicode character offsets, but JavaScript indexes strings by UTF-16 code units. Emoji or other non-BMP characters before the match shift the highlight. Clamp and slice
Array.from(snippet)instead.Proposed fix
const snippet = typeof props.snippet === 'string' ? props.snippet : ''; + const snippetLength = Array.from(snippet).length; const start = typeof props.snippet_start === 'number' ? props.snippet_start : 0; const end = typeof props.snippet_end === 'number' ? props.snippet_end : 0; ... - snippetStart: Math.max(0, Math.min(start, snippet.length)), - snippetEnd: Math.max(0, Math.min(end, snippet.length)), + snippetStart: Math.max(0, Math.min(start, snippetLength)), + snippetEnd: Math.max(0, Math.min(end, snippetLength)), ... - const before = snippet.slice(0, snippetStart); - const match = snippet.slice(snippetStart, snippetEnd); - const after = snippet.slice(snippetEnd); + const chars = Array.from(snippet); + const before = chars.slice(0, snippetStart).join(''); + const match = chars.slice(snippetStart, snippetEnd).join(''); + const after = chars.slice(snippetEnd).join('');Also applies to: 106-111
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/views/MentionsSection.tsx` around lines 43 - 55, Update the snippet offset handling in the mention mapping logic to interpret snippet_start and snippet_end as Unicode character offsets: convert snippet to Array.from(snippet), clamp both offsets to that array’s length, and slice using the clamped values. Apply the same character-based slicing to the related snippet-highlight logic near the other referenced range.crates/commonplace/src/mentions.rs-58-72 (1)
58-72: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve alias identity in the candidate ID.
alias_slug()is lossy: aliases such asA.BandA Bproduce the same ID. One candidate can therefore overwrite or suppress another, including its dismissal state. Use a collision-resistant encoding or hash of the original alias.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/commonplace/src/mentions.rs` around lines 58 - 72, Update mention_candidate_id and alias_slug so the candidate ID preserves distinct original aliases instead of using the lossy slug alone. Replace the alias component with a collision-resistant encoding or hash derived from the full alias, ensuring aliases such as “A.B” and “A B” produce different deterministic IDs while retaining deduplication and dismissal-state behavior.apps/console/src/views/CardView.tsx-362-380 (1)
362-380: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not wrap mention controls in a card-wide button.
Compact cards contain mention buttons inside this
role="button"wrapper. Their clicks bubble toselectRecord, so expanding, confirming, or dismissing a mention also opens the record; the nested interactive semantics are invalid as well. Use a dedicated Open control or a non-interactive wrapper with explicitly separated actions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/views/CardView.tsx` around lines 362 - 380, The CardGridCell wrapper currently makes the entire card a role="button", causing nested mention controls in RecordCard to trigger selectRecord and creating invalid interactive nesting. Replace the card-wide interactive behavior with a dedicated Open control or a non-interactive container, and ensure mention actions remain isolated while preserving keyboard access to opening the record.apps/console/src/lib/console-host.ts-292-320 (1)
292-320: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve complete and live query semantics.
This wrapper only fetches the first 500 backend objects and replaces the backend subscription with a no-op. Documents beyond that page become unqueryable by slug/path, while
live: trueconsumers never receive updates. Follownext_cursorand reapply the local transform to subscribed/refetched results.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/lib/console-host.ts` around lines 292 - 320, The queryLiveDomain implementation must preserve complete results and live updates instead of limiting reads to the first backend page and returning a no-op subscription. Update queryLiveDomain to follow next_cursor across all http.query pages, reapply predicate filtering, ranking, and pagination to the combined objects, and use the backend subscription/refetch mechanism so live:true consumers receive updates.apps/console/src/lib/console-host.ts-328-358 (1)
328-358: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMove fixture seeding to an atomic backend upsert.
The single-page read-before-create check is not idempotent: an existing slug/path outside the first 200 results is missed, and concurrent console clients can both observe absence and create duplicates. Seed through deterministic IDs or a backend uniqueness/upsert operation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/lib/console-host.ts` around lines 328 - 358, The ensureSeedContent method currently performs a limited read-before-create flow that can miss existing fixtures and race with other clients. Replace the separate document and code-file query/create loops with an atomic backend upsert or deterministic-ID seeding operation, using each document slug and code-file path as the stable identity so repeated and concurrent calls cannot create duplicates.crates/commonplace/src/block_view.rs-961-965 (1)
961-965: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftReconcile stale mention candidates after atom edits.
evaluate_mentions_for_atomonly inserts missing IDs. Removing or moving a mention therefore leaves stale candidates/snippets—and any confirmedMENTIONED_INedge—attached to the edited atom. Re-evaluation should update surviving candidates and retire candidates/edges no longer supported by the text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/commonplace/src/block_view.rs` around lines 961 - 965, Update the edited-atom branch around evaluate_mentions_for_atom so re-evaluation reconciles the atom’s complete mention state rather than only inserting missing candidate IDs. Refresh surviving candidate snippets, remove candidates no longer supported by the current text, and retire any corresponding confirmed MENTIONED_IN edges; preserve existing candidates only when still valid.apps/console/src/views/registry.tsx-114-142 (1)
114-142: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDeclare the
updateaction emitted by card views.Both renderers mount mention controls that emit candidate status updates, but their descriptors omit
update. Add it to bothemitslists so registry capability metadata matches runtime behavior.Proposed fix
- emits: ['select', 'open'], + emits: ['select', 'open', 'update'],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/views/registry.tsx` around lines 114 - 142, Update the emits lists in CARD_FULL and CARDS_GRID to include the update action alongside select and open, so both card view descriptors accurately declare the status-update events emitted by their renderers.apps/console/src/components/shell/ActionSheet.tsx-128-149 (1)
128-149: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPrevent duplicate delegation requests while submission is pending.
Ctrl/Cmd+Enter remains active after
submit.kindbecomessubmitting, allowing repeated non-idempotent POSTs. GuardsubmitSheetwith an in-flight ref or state check.Also applies to: 181-190, 272-280
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/components/shell/ActionSheet.tsx` around lines 128 - 149, The submitSheet callback allows duplicate delegation POSTs while submission is in progress. Add an in-flight ref or check the current submit state at the start of submitSheet, returning immediately when the submission is already submitting, and ensure the guard is cleared when the request completes so later submissions remain possible.apps/console/src/views/GalleyDocView.tsx-107-112 (1)
107-112: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGive each todo selection a collision-free chip ID.
Distinct todos in the same document with identical first 24 characters receive the same ID.
thread-store.stage()then treats the second todo as already staged and silently drops it. Include the list-item index or another unique identifier.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/views/GalleyDocView.tsx` around lines 107 - 112, Update the chip ID construction for todo selections in GalleyDocView so it remains unique for distinct todos within the same document, including when their first 24 characters match. Incorporate the surrounding list-item index or another stable unique identifier while preserving the existing document and text components.apps/console/src/views/RecordInspector.tsx-26-29 (1)
26-29: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not expose the previous record while the new selection loads.
After
selectedRecordIdchanges,recordstill resolves to stalefetcheddata. The body correctly shows loading, but the Action button hands off that previous object.Proposed fix
const record = selectedRecordObject && selectedRecordObject.id === selectedRecordId ? selectedRecordObject - : fetched; + : fetched?.id === selectedRecordId + ? fetched + : null;Also applies to: 99-121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/views/RecordInspector.tsx` around lines 26 - 29, Update the record selection logic in RecordInspector so a changed selectedRecordId cannot fall back to stale fetched data while the new record is loading. Ensure record is undefined or otherwise unavailable until fetched matches the current selection, while preserving selectedRecordObject usage for the matching ID and the existing loading/body behavior.apps/console/src/components/shell/ActionSheet.tsx-263-269 (1)
263-269: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftImplement the advertised rule-saving flow before merging.
The PR objective includes rule saving, but this control explicitly states that the feature is unavailable and provides no action.
Would you like me to help implement or track the missing rules-engine integration?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/components/shell/ActionSheet.tsx` around lines 263 - 269, Implement the rule-saving flow for the ActionSheet control instead of rendering the unavailable disabled message. Replace the data-save-as-rule-unavailable span with an actionable Save as rule control that invokes the existing rule-engine integration or the appropriate save handler, preserving the displayed action and ensuring the IX6 dependency is validated before saving.apps/console/src/components/shell/ActionSheet.tsx-110-126 (1)
110-126: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not silently discard “Mark handled after” for With-me actions.
The checkbox remains selectable, but the With-me branch stages only chip references and drops
followUp. Either carry this intent into the thread flow or disable/reset the option for that destination.Also applies to: 255-262
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/components/shell/ActionSheet.tsx` around lines 110 - 126, The with-me branch in ActionSheet’s destination handling discards the selected followUp intent when calling stageInThread. Update the with-me flow to either propagate followUp into the thread staging/handling path or explicitly disable and reset the option when this destination is selected, ensuring no selectable state is silently lost while preserving the existing focus and close behavior.apps/console/src/components/shell/ActionSheet.tsx-151-179 (1)
151-179: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTrap keyboard focus inside the modal.
aria-modal="true"does not enforce focus containment. Tab navigation can escape into the underlying console and activate controls while the sheet remains open. Use a focus-scope implementation or native modal dialog behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/components/shell/ActionSheet.tsx` around lines 151 - 179, Update the action sheet component around the dialog motion element and its parent overlay to trap keyboard focus within the open modal, using the project’s existing focus-scope implementation or native modal dialog behavior. Preserve Escape handling, backdrop dismissal, and the existing dialog accessibility attributes.apps/console/src/lib/thread-store.ts-85-95 (1)
85-95: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winEscape staged references with the mention protocol’s serializer.
Labels containing
],), backslashes, or newlines produce malformed@[label](objectId)references, so the intended object context may not survive transport. Use one shared round-trip-safe encoder with the mention parser.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/lib/thread-store.ts` around lines 85 - 95, Update send in the thread store to serialize staged reference labels and object IDs through the mention protocol’s shared round-trip-safe encoder before constructing refLine. Reuse the existing encoder paired with the mention parser so characters such as ], ), backslashes, and newlines are escaped consistently; preserve the current staged-clearing and message assembly behavior.
🟡 Minor comments (3)
apps/console/src/views/MentionsSection.tsx-151-154 (1)
151-154: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPluralize the mention count correctly and update its test.
apps/console/src/views/MentionsSection.tsx#L151-L154: renderplacewhen the visible count is one.apps/console/e2e/cards.spec.ts#L223-L224: expect “mentioned in 1 place, 0 unlinked.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/views/MentionsSection.tsx` around lines 151 - 154, Update the mentions summary in MentionsSection.tsx to use singular “place” when the visible, non-dismissed count is one and plural “places” otherwise. Update the corresponding expectation in apps/console/e2e/cards.spec.ts lines 223-224 to expect “mentioned in 1 place, 0 unlinked.”apps/console/src/lib/card-templates.ts-67-69 (1)
67-69: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTreat malformed nested template fields as validation failures.
Invalid subtitle, fact, chip, or gauge entries are silently filtered out, producing an incomplete card without
fallbackNote. Return a parse failure when a supplied array contains an invalid entry so malformed templates consistently degrade to generic.Also applies to: 88-120
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/lib/card-templates.ts` around lines 67 - 69, Update stringArray and the template parsing flow for subtitle, fact, chip, and gauge fields so supplied arrays fail validation when any entry is not a string instead of filtering invalid entries. Propagate the parse failure to the existing generic fallback path, while preserving successful parsing for arrays whose entries are all valid strings.apps/console/src/lib/action-pack.ts-59-68 (1)
59-68: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude
object_typein the exact-context comparison.A pack can change an object's type while still passing
packEqualsChips, violating the stated invariant.Proposed fix
const key = (entry: PackContextEntry) => - `${entry.kind}|${entry.label}|${entry.object_id ?? ''}|${entry.text ?? ''}`; + `${entry.kind}|${entry.label}|${entry.object_id ?? ''}|${entry.object_type ?? ''}|${entry.text ?? ''}`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/lib/action-pack.ts` around lines 59 - 68, Update packEqualsChips and its local key function to include each PackContextEntry’s object_type in the generated comparison key, ensuring both sorted exact-context checks and the first-origin check detect object type changes.
🧹 Nitpick comments (1)
apps/console/e2e/cards.spec.ts (1)
144-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the complete action-pack context exactly.
This test permits reordered entries and duplicate-label substitutions because it checks only length and label membership. Compare the ordered context objects—including IDs/object IDs—to the visible chip model.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/e2e/cards.spec.ts` around lines 144 - 160, Strengthen the no-silent-context assertions in the handoff test by comparing the submitted pack.context to the visible chip model in order, including each chip’s label and ID/object ID fields. Replace the length and membership-only checks around visibleChips and pack.context with an exact ordered deep comparison, preserving the instruction assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Major comments:
In `@apps/console/e2e/stub-data-api.mjs`:
- Line 254: Update the query result construction around poolFor and the response
shape metadata so shape.types and shape.fields are derived from query.types and
the selected objects rather than always using record-only metadata. Preserve the
existing result data while ensuring person, task, doc, and other pools advertise
their correct types and fields for renderer selection.
- Around line 307-317: Update the action.kind === 'update' handling to return a
non-success response when allStored().find cannot locate the requested target.
Preserve the existing successful response and mutation for found targets, and
prevent missing-target updates from falling through to the generic accepted
receipt.
- Around line 320-327: The create handling for POOLS in the action-processing
flow must be idempotent: derive a stable logical key from the action or its
provided id, detect an existing pool entry, and update/reuse it instead of
appending a duplicate on retries. Preserve the existing response shape and
return the stable object id in target_ids.
In `@apps/console/src/app/api/harness/delegate/route.ts`:
- Around line 19-31: Update the upstream fetch in the delegate route to use an
AbortController-based timeout, ensuring stalled handoff requests are aborted
after the configured limit. Catch the timeout/abort condition and map it to the
existing unavailable response while preserving normal upstream response
handling.
In `@apps/console/src/components/shell/ActionSheet.tsx`:
- Around line 128-149: The submitSheet callback allows duplicate delegation
POSTs while submission is in progress. Add an in-flight ref or check the current
submit state at the start of submitSheet, returning immediately when the
submission is already submitting, and ensure the guard is cleared when the
request completes so later submissions remain possible.
- Around line 263-269: Implement the rule-saving flow for the ActionSheet
control instead of rendering the unavailable disabled message. Replace the
data-save-as-rule-unavailable span with an actionable Save as rule control that
invokes the existing rule-engine integration or the appropriate save handler,
preserving the displayed action and ensuring the IX6 dependency is validated
before saving.
- Around line 110-126: The with-me branch in ActionSheet’s destination handling
discards the selected followUp intent when calling stageInThread. Update the
with-me flow to either propagate followUp into the thread staging/handling path
or explicitly disable and reset the option when this destination is selected,
ensuring no selectable state is silently lost while preserving the existing
focus and close behavior.
- Around line 151-179: Update the action sheet component around the dialog
motion element and its parent overlay to trap keyboard focus within the open
modal, using the project’s existing focus-scope implementation or native modal
dialog behavior. Preserve Escape handling, backdrop dismissal, and the existing
dialog accessibility attributes.
In `@apps/console/src/components/shell/IntuiShell.tsx`:
- Around line 161-169: Update switchTo in IntuiShell so changing the active
surface is atomic or serialized as one complete operation rather than issuing
multiple unawaited host.emit calls. Ensure rapid requests cannot overlap, and
handle a failed switch before accepting another request so the persisted state
always has exactly the requested surface active.
In `@apps/console/src/lib/console-host.ts`:
- Around line 292-320: The queryLiveDomain implementation must preserve complete
results and live updates instead of limiting reads to the first backend page and
returning a no-op subscription. Update queryLiveDomain to follow next_cursor
across all http.query pages, reapply predicate filtering, ranking, and
pagination to the combined objects, and use the backend subscription/refetch
mechanism so live:true consumers receive updates.
- Around line 328-358: The ensureSeedContent method currently performs a limited
read-before-create flow that can miss existing fixtures and race with other
clients. Replace the separate document and code-file query/create loops with an
atomic backend upsert or deterministic-ID seeding operation, using each document
slug and code-file path as the stable identity so repeated and concurrent calls
cannot create duplicates.
In `@apps/console/src/lib/thread-store.ts`:
- Around line 85-95: Update send in the thread store to serialize staged
reference labels and object IDs through the mention protocol’s shared
round-trip-safe encoder before constructing refLine. Reuse the existing encoder
paired with the mention parser so characters such as ], ), backslashes, and
newlines are escaped consistently; preserve the current staged-clearing and
message assembly behavior.
In `@apps/console/src/views/CardView.tsx`:
- Around line 362-380: The CardGridCell wrapper currently makes the entire card
a role="button", causing nested mention controls in RecordCard to trigger
selectRecord and creating invalid interactive nesting. Replace the card-wide
interactive behavior with a dedicated Open control or a non-interactive
container, and ensure mention actions remain isolated while preserving keyboard
access to opening the record.
In `@apps/console/src/views/GalleyDocView.tsx`:
- Around line 107-112: Update the chip ID construction for todo selections in
GalleyDocView so it remains unique for distinct todos within the same document,
including when their first 24 characters match. Incorporate the surrounding
list-item index or another stable unique identifier while preserving the
existing document and text components.
In `@apps/console/src/views/MentionsSection.tsx`:
- Around line 134-139: Update setStatus to handle rejected host.emit updates
with try/catch/finally: always clear the candidate via setPending(null), expose
a retryable error to the user, and prevent an unhandled rejection while
preserving refresh only for successful updates.
- Around line 59-103: Update useMentionCandidates so refresh invalidates every
mounted mention query, not just the initiating hook instance. Replace the local
generation-only mechanism with the existing shared query invalidation or host
subscription mechanism, and ensure all mounted instances reload after mention
updates while preserving the current query and retry behavior.
- Around line 43-55: Update the snippet offset handling in the mention mapping
logic to interpret snippet_start and snippet_end as Unicode character offsets:
convert snippet to Array.from(snippet), clamp both offsets to that array’s
length, and slice using the clamped values. Apply the same character-based
slicing to the related snippet-highlight logic near the other referenced range.
In `@apps/console/src/views/RecordInspector.tsx`:
- Around line 26-29: Update the record selection logic in RecordInspector so a
changed selectedRecordId cannot fall back to stale fetched data while the new
record is loading. Ensure record is undefined or otherwise unavailable until
fetched matches the current selection, while preserving selectedRecordObject
usage for the matching ID and the existing loading/body behavior.
In `@apps/console/src/views/registry.tsx`:
- Around line 114-142: Update the emits lists in CARD_FULL and CARDS_GRID to
include the update action alongside select and open, so both card view
descriptors accurately declare the status-update events emitted by their
renderers.
In `@crates/commonplace/src/block_view.rs`:
- Around line 961-965: Update the edited-atom branch around
evaluate_mentions_for_atom so re-evaluation reconciles the atom’s complete
mention state rather than only inserting missing candidate IDs. Refresh
surviving candidate snippets, remove candidates no longer supported by the
current text, and retire any corresponding confirmed MENTIONED_IN edges;
preserve existing candidates only when still valid.
In `@crates/commonplace/src/mentions.rs`:
- Around line 254-292: Update evaluate_mentions_for_atom to reconcile existing
candidates for the edited atom with the current match_aliases results, removing
candidates whose matches disappeared and updating candidates whose spans or
captured snippets changed instead of skipping existing IDs. Preserve confirmed
mentions/edges according to the project’s established behavior, explicitly
preventing stale candidate evidence from producing edges while retaining
confirmed edge semantics.
- Around line 58-72: Update mention_candidate_id and alias_slug so the candidate
ID preserves distinct original aliases instead of using the lossy slug alone.
Replace the alias component with a collision-resistant encoding or hash derived
from the full alias, ensuring aliases such as “A.B” and “A B” produce different
deterministic IDs while retaining deduplication and dismissal-state behavior.
---
Minor comments:
In `@apps/console/src/lib/action-pack.ts`:
- Around line 59-68: Update packEqualsChips and its local key function to
include each PackContextEntry’s object_type in the generated comparison key,
ensuring both sorted exact-context checks and the first-origin check detect
object type changes.
In `@apps/console/src/lib/card-templates.ts`:
- Around line 67-69: Update stringArray and the template parsing flow for
subtitle, fact, chip, and gauge fields so supplied arrays fail validation when
any entry is not a string instead of filtering invalid entries. Propagate the
parse failure to the existing generic fallback path, while preserving successful
parsing for arrays whose entries are all valid strings.
In `@apps/console/src/views/MentionsSection.tsx`:
- Around line 151-154: Update the mentions summary in MentionsSection.tsx to use
singular “place” when the visible, non-dismissed count is one and plural
“places” otherwise. Update the corresponding expectation in
apps/console/e2e/cards.spec.ts lines 223-224 to expect “mentioned in 1 place, 0
unlinked.”
---
Nitpick comments:
In `@apps/console/e2e/cards.spec.ts`:
- Around line 144-160: Strengthen the no-silent-context assertions in the
handoff test by comparing the submitted pack.context to the visible chip model
in order, including each chip’s label and ID/object ID fields. Replace the
length and membership-only checks around visibleChips and pack.context with an
exact ordered deep comparison, preserving the instruction assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b9d7ee97-42c3-4d14-9d98-65d5a8ee92f3
⛔ Files ignored due to path filters (7)
apps/console/e2e/cards.spec.ts-snapshots/action-sheet-darwin.pngis excluded by!**/*.pngapps/console/e2e/cards.spec.ts-snapshots/card-compact-inspector-darwin.pngis excluded by!**/*.pngapps/console/e2e/cards.spec.ts-snapshots/cards-grid-darwin.pngis excluded by!**/*.pngapps/console/e2e/omnibar.spec.ts-snapshots/omnibar-expanded-ask-darwin.pngis excluded by!**/*.pngapps/console/e2e/proof-workspace.spec.ts-snapshots/workspace-1280-dark-darwin.pngis excluded by!**/*.pngapps/console/e2e/proof-workspace.spec.ts-snapshots/workspace-1440-dark-darwin.pngis excluded by!**/*.pngapps/console/e2e/proof-workspace.spec.ts-snapshots/workspace-1440-reduced-motion-darwin.pngis excluded by!**/*.png
📒 Files selected for processing (33)
.railwayignoreapps/console/e2e/cards.spec.tsapps/console/e2e/omnibar.spec.tsapps/console/e2e/proof-workspace.spec.tsapps/console/e2e/stub-data-api.mjsapps/console/playwright.config.tsapps/console/src/app/api/harness/delegate/route.tsapps/console/src/components/ConsoleApp.tsxapps/console/src/components/shell/ActionSheet.tsxapps/console/src/components/shell/IntuiShell.tsxapps/console/src/components/shell/MainToolbar.tsxapps/console/src/components/shell/Omnibar.tsxapps/console/src/components/shell/icons.tsxapps/console/src/lib/action-pack.test.tsapps/console/src/lib/action-pack.tsapps/console/src/lib/card-templates.test.tsapps/console/src/lib/card-templates.tsapps/console/src/lib/console-host.test.tsapps/console/src/lib/console-host.tsapps/console/src/lib/shell-store.tsapps/console/src/lib/thread-store.tsapps/console/src/lib/workspace-seed.tsapps/console/src/motion/motion-tokens.tsapps/console/src/views/CardView.tsxapps/console/src/views/GalleyDocView.tsxapps/console/src/views/MentionsSection.tsxapps/console/src/views/RecordInspector.tsxapps/console/src/views/ThreadView.tsxapps/console/src/views/registry.tsxcrates/commonplace/src/block_view.rscrates/commonplace/src/ingest.rscrates/commonplace/src/lib.rscrates/commonplace/src/mentions.rs
|
@copilot resolve the merge conflicts on this branch. |
…mentions-nav # Conflicts: # apps/console/e2e/stub-data-api.mjs # apps/console/src/lib/console-host.ts # apps/console/src/views/registry.tsx
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Resolved in 9095dca. Three files had conflicts:
All 36 console unit tests pass after the merge. |
Opening or closing a tool window flashed the whole content area to the frame and back (a black frame drop), and the reflow read as broken. Cause: the PanelGroup was keyed on the visible panel set, so every open/close changed the key and remounted the entire well: the editor and thread tore down and rebuilt, and the editor's delayed entrance fade re-fired, leaving the frame showing through for a beat. Key the group on the active surface only (a real screen switch still remounts, which is correct), and give each Panel a stable order derived from the full region lists so react-resizable-panels reconciles a panel appearing or disappearing in place. The editor node now survives the toggle and the content reflows without a teardown. Regression test asserts the editor panel node survives an Alt+9 toggle (reconcile, not remount).
…w surface The surface-rail navigation (replacing the toolbar layout dropdown) merged with the Greenfield Hunk Review surface. Reconcile: - give the review surface its own rail glyph (IconInspector); - point the hunk-review e2e at the stripe surfaces group instead of the removed Layout dropdown; - the rail now carries five surfaces, not four; - give the injected-surface card.full assertion headroom so it does not flake under a cold, parallel-loaded dev server. 28 e2e, 36 unit, and the fence/register/motion/contrast gates pass on the merged tree.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/console/e2e/hunk-review.spec.ts (1)
4-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport
Pagefrom@playwright/test.Instead of using an inline
import()type assertion, you can importPagealongsideexpectandtestfor better readability.♻️ Proposed refactor
-import { expect, test } from '`@playwright/test`'; +import { expect, test, type Page } from '`@playwright/test`'; -async function openReview(page: import('`@playwright/test`').Page) { +async function openReview(page: Page) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/e2e/hunk-review.spec.ts` around lines 4 - 6, Update the Playwright imports in hunk-review.spec.ts to include the Page type alongside expect and test, then replace the inline import('`@playwright/test`').Page annotation in openReview with the imported Page symbol.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/console/e2e/hunk-review.spec.ts`:
- Around line 4-6: Update the Playwright imports in hunk-review.spec.ts to
include the Page type alongside expect and test, then replace the inline
import('`@playwright/test`').Page annotation in openReview with the imported Page
symbol.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 74eef5fe-def1-49c4-adba-b81b5bcf74e6
📒 Files selected for processing (10)
apps/console/e2e/cards.spec.tsapps/console/e2e/hunk-review.spec.tsapps/console/e2e/omnibar.spec.tsapps/console/e2e/stub-data-api.mjsapps/console/src/components/shell/IntuiShell.tsxapps/console/src/components/shell/Omnibar.tsxapps/console/src/lib/console-host.test.tsapps/console/src/lib/console-host.tsapps/console/src/lib/workspace-seed.tsapps/console/src/views/registry.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/console/src/lib/console-host.test.ts
- apps/console/e2e/stub-data-api.mjs
- apps/console/src/views/registry.tsx
- apps/console/src/lib/workspace-seed.ts
- apps/console/src/lib/console-host.ts
- apps/console/e2e/cards.spec.ts
Resolve the omnibar Ctrl/Cmd+K conflict: main #59 and this branch both landed the browser-reliable Ask key, so the logic is identical on both sides. Keep the fuller intent comments, take main's Prettier-conformant condition and accurate aria-label, and take the extra Escape close-assertion in the spec.
Replace the inline import('@playwright/test').Page annotation with the imported
Page symbol (CodeRabbit nitpick).
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1ebf68f15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| query: { | ||
| types: ['person', 'task', 'project', 'org'], | ||
| page: { limit: 400 }, | ||
| live: true, | ||
| } as unknown as JsonValue, |
There was a problem hiding this comment.
Against the live Rust object API, this query returns no relation data: object_ref_from_item only serializes relations requested through traverse or project.relations, while ResolvedCard renders chips exclusively from object.relations. Consequently the Cards surface has no WORKS_AT/IN_PROJECT/etc. chips in production, even though the Playwright stub masks this by returning all fixture relations regardless of the query. Add the template-required outgoing relations to this query's projection (or otherwise request them).
Useful? React with 👍 / 👎.
| ObjectAction::Create { type_ref, props } => { | ||
| let item = item_from_props(&type_ref, props)?; | ||
| let item = self.put_item(item)?; | ||
| // Incremental mention detection (K5): a new text-bearing item | ||
| // is an atom; evaluate it once, never a full rescan. | ||
| self.evaluate_mentions_for_atom(&item.id)?; |
There was a problem hiding this comment.
When a person or other target is created, renamed, or given an alias after text already exists, this evaluates the target itself as an atom; typical targets have no inline body, so evaluate_mentions_for_atom exits without scanning the pre-existing notes. That means candidates are created only when the note happens to be written after its target, and later target metadata changes never discover existing mentions. Re-evaluate relevant existing text atoms when a target's title or aliases change.
Useful? React with 👍 / 👎.
| let edge = EdgeRecord::new( | ||
| format!("mentioned-in:{object_id}:{atom_id}"), | ||
| object_id.as_str(), | ||
| MENTIONED_IN_EDGE, | ||
| atom_id.as_str(), | ||
| basis, | ||
| ) | ||
| .with_confidence(1.0); | ||
| self.store_mut().upsert_edge(edge)?; |
There was a problem hiding this comment.
If one atom contains two aliases for the same object, detection creates two candidates because their IDs include the alias. Confirming each candidate writes this same mentioned-in:{object_id}:{atom_id} edge, so the second confirmation upserts it and replaces the first candidate's recorded basis. This loses confirmed provenance while the UI still represents both candidates as linked; either coalesce candidates per object/atom or retain all confirmed bases on the edge.
Useful? React with 👍 / 👎.
| const existingDocs = await this.http.query({ types: ['doc'], page: { limit: 200 } }); | ||
| const slugs = new Set(existingDocs.objects.map((object) => object.properties.slug)); |
There was a problem hiding this comment.
The claimed slug/path idempotency only considers the first 200 objects. In a production backend with more than 200 documents or code files, an existing seed outside that page is treated as absent and another copy is created; repeated console mounts can keep adding duplicates. Query by the seed key or paginate through all results before issuing creates.
Useful? React with 👍 / 👎.
The visual gate ran on this PR for the first time (it was not attached on the base commit) and surfaced four intended changes plus one gap: - cards: the cards/actions/mentions surface never had linux baselines; add cards-grid, card-compact-inspector, and action-sheet. - omnibar: the expanded island hint changed with the browser-reliable Ask key (#59); refresh omnibar-expanded-ask. - workspace: the nav consolidated into one leftmost stripe and docs now ride the wire (served by the e2e stub), so the brief reflects the stub fixture and the thread renders live; refresh the 1280, 1440, and reduced-motion captures. Baselines are the ubuntu-latest actuals from the failing run artifacts, so they match the renderer that validates them.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
💡 Codex Review
CommonPlace/apps/console/src/views/GalleyDocView.tsx
Lines 149 to 152 in 092f32a
Report failed document saves before leaving edit mode
If the object-wire update is rejected or unreachable, host.emit resolves with an error result rather than throwing, but this fire-and-forget call immediately switches the editor back to read mode with the optimistic text. The user is told nothing, and a reload loses the edit despite the new live-wire persistence behavior; await the receipt and keep edit mode or show a save failure when ok is false.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private async queryLiveDomain(query: ObjectQuery, type: string): Promise<ObjectSet> { | ||
| const all = await this.http.query({ types: [type], page: { limit: 500 } }); | ||
| let objects = all.objects.filter((object) => matchesPredicate(object, query.where)); |
There was a problem hiding this comment.
When the backend has more than 500 documents or code files, this fetch drops every later page before applying the client-side slug/path predicate. As a result, the Documents list omits later documents, and opening a document whose matching slug lies beyond the first 500 yields the empty state even though it exists. Follow next_cursor until exhaustion before filtering and applying the requested page.
Useful? React with 👍 / 👎.
| host.query({ | ||
| types: [MENTION_CANDIDATE_TYPE], | ||
| where: { kind: 'eq', field: 'object_id', value: objectId }, | ||
| page: { limit: 200 }, | ||
| }), |
There was a problem hiding this comment.
For the Rust-backed object seam, object_id is stored in an item's extra map, while predicate evaluation only reads top-level serialized Item fields (crates/commonplace/src/block_view.rs:1548-1550), so this server-side filter returns the first 200 candidates globally and the client filters them afterward. Once the workspace has more than 200 candidates, a target whose candidates fall on later pages incorrectly has no mention section or an incomplete count; paginate all candidate pages or make the backend predicate address projected metadata.
Useful? React with 👍 / 👎.
#62 is the canonical IA-carrying branch, so standardize its visual gate on the repo convention: ubuntu-latest with CI-captured linux baselines (as #60 and #63 use), matching the gates job runner. ubuntu is cheaper, faster, and more deterministic for pixel snapshots. The -linux.png references are harvested from this run's artifact next; darwin baselines stay for local mac dev.
…#62) * fix(console): add Cmd/Ctrl+K as the browser-reliable omnibar Ask key Cmd/Ctrl+L (handoff named choice 3, Cursor muscle memory) is a browser accelerator reserved for the address bar, so a browser tab never delivers the keydown to the page and the omnibar cannot capture it. It works in the desktop shell but not in a browser. Add Cmd/Ctrl+K (the command-palette convention browsers do deliver) as an additional Ask trigger, keeping Cmd/Ctrl+L for the desktop target and double-Shift for Search. The field's aria-keyshortcuts advertises all of them. e2e asserts Ctrl+K opens Ask. * feat(console): cards, actions, mentions + stripe surface nav + live doc wire HANDOFF-CARDS-ACTIONS-MENTIONS K1-K7: - K1 card templates as data (person/task/generic seeds) + one card engine (card.full / card.compact) rendering any kind through the block contract; malformed or missing template degrades to generic with a note, never errors. - K2 mounts: inspector leads with the compact card above the field table; cards.grid descriptor renders an ObjectQuery as virtualized faces at Twenty density; a Cards surface joins the seeded screens. - K3 action sheet, three entries into one sheet (/do composer, todo-block arrow + Alt+Enter on Galley task items, Action verb on inspector and cards); staged context is always visible; auto-suggest adds removable chips; save as rule names IX6. - K4 delegate wire: For me posts the pack to /api/harness/delegate and names the harness refusal; With me stages visible object refs above the composer. - K5 mentions detection engine (crates/commonplace/src/mentions.rs): exact and normalized-exact tiers, span slices back to the original atom text, incremental on ingest and on object create/update, deterministic candidate id as the dedup and dismissal-suppression key. - K6 mentions surface: linked/unlinked counts, passage list with the matched span highlighted, confirm writes the MENTIONED_IN edge with its basis on the edge, dismiss records the negative signal. - K7 gates: template render-model tests, the pack-equals-chips invariant test, the candidate-to-confirmed-edge round trip, and cards/actions/mentions e2e with visual baselines. Follow-up fixes from live review: - Screen navigation moves out of the toolbar dropdown into the leftmost stripe (the stripe surfaces group): surfaces on top, a divider, then the active surface's tool windows, one bar in the original chrome tokens. The toolbar shows the active screen as a quiet breadcrumb. - Documents and code files ride the live object wire so edits persist to the backend; content rides as extra properties (markdown / content) matching the view projection, the console filters client-side so slug/id predicates hold, and the backend is seeded once (idempotent by slug/path). Verify: 40 crate tests, 29 console unit tests, 25 e2e, and the fence, register, contrast, and motion gates all pass. * fix(console): stop the tool-window toggle from remounting the well Opening or closing a tool window flashed the whole content area to the frame and back (a black frame drop), and the reflow read as broken. Cause: the PanelGroup was keyed on the visible panel set, so every open/close changed the key and remounted the entire well: the editor and thread tore down and rebuilt, and the editor's delayed entrance fade re-fired, leaving the frame showing through for a beat. Key the group on the active surface only (a real screen switch still remounts, which is correct), and give each Panel a stable order derived from the full region lists so react-resizable-panels reconciles a panel appearing or disappearing in place. The editor node now survives the toggle and the content reflows without a teardown. Regression test asserts the editor panel node survives an Alt+9 toggle (reconcile, not remount). * fix(console): reconcile the stripe surfaces nav with the merged Review surface The surface-rail navigation (replacing the toolbar layout dropdown) merged with the Greenfield Hunk Review surface. Reconcile: - give the review surface its own rail glyph (IconInspector); - point the hunk-review e2e at the stripe surfaces group instead of the removed Layout dropdown; - the rail now carries five surfaces, not four; - give the injected-surface card.full assertion headroom so it does not flake under a cold, parallel-loaded dev server. 28 e2e, 36 unit, and the fence/register/motion/contrast gates pass on the merged tree. * feat(console): add register-wide coloration * test(console): import Playwright Page type in hunk-review spec Replace the inline import('@playwright/test').Page annotation with the imported Page symbol (CodeRabbit nitpick). * fix(console): run contrast gate with ts stripping * Fix CodeMirror theme mode facet * fix(console): use a plain range in the theme clamp note The chroma clamp note used an en dash (0-0.04); the console constitution bans en and em dashes in UI strings. Write it as 'the safe 0 to 0.04 range.' * docs(console): sync the constitution to the coloration handoff CLAUDE.md and AGENTS.md still mandated Int UI glyphs for chrome and a dark-only 'pinned register exclusively', contradicting HANDOFF-CONSOLE-COLORATION (in force; named choices are requirements). Update the icon ledger row to the sanctioned Noun Project policy (currentColor on the icon ladder, gate:icons, domain tint plus file-kind dots per named choice 7 / T5), note the light register and two-knob theme engine in the material doctrine, and add the icon paint scan to the gate list (gate 5; visual becomes gate 6). Resolves the CodeRabbit 'use the mandated Int UI glyph source' finding: the icons are spec-mandated, so the governing doc was the stale artifact. * feat(console): implement role-aware information architecture * test(console): sync files baseline to CI renderer * fix(console): harden information architecture interactions * chore(console): run the visual gate on ubuntu-latest (repo convention) #62 is the canonical IA-carrying branch, so standardize its visual gate on the repo convention: ubuntu-latest with CI-captured linux baselines (as #60 and #63 use), matching the gates job runner. ubuntu is cheaper, faster, and more deterministic for pixel snapshots. The -linux.png references are harvested from this run's artifact next; darwin baselines stay for local mac dev. * test(console): pin linux visual baselines for the ubuntu gate Gate 6 now runs on ubuntu-latest, so every surface needs a -linux.png reference. Harvested the 14 actuals from the ubuntu CI run (appearance light, cards, information architecture chat/context-graph/files/stripe, hunk review, search field) and pinned them. Darwin baselines stay for local mac dev. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
|
Superseded by #62, now merged to main. #62 integrated this PR's work and carries it into main: cards/actions/mentions, the live doc/code wire ( |
What
HANDOFF-CARDS-ACTIONS-MENTIONS (K1-K7) plus fixes surfaced in live review.
Cards, actions, mentions
card.full/card.compact) rendering any kind through the block contract. A malformed or missing template degrades to the generic card with a note, never an error.cards.gridrenders an ObjectQuery as virtualized faces at Twenty density; a Cards surface joins the seeded screens./doin the composer, the todo-block arrow +Alt+Enteron Galley task items, and the Action verb on inspector and cards. Staged context is always visible; auto-suggest adds removable chips; save-as-rule names IX6./api/harness/delegateand names the harness refusal; With me stages visible object references above the thread composer.crates/commonplace/src/mentions.rs): exact and normalized-exact tiers, the recorded span slices back to the original atom text, incremental on ingest and on object create/update, deterministic candidate id as the dedup and dismissal-suppression key.MENTIONED_INedge carrying its basis, dismiss records the negative signal.Live-review fixes
markdown/content) matching the view projection; the console filters client-side so slug/id predicates hold; the backend is seeded once, idempotent by slug and path.Verify
40 crate tests, 29 console unit tests, 25 e2e, and the fence, register, contrast, and motion gates all pass locally.
Notes
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
/docommands, and document todos (with staged context chips).Accessibility & Performance