feat: share block-document Ask AI and Deck map helpers across hosts - #805

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".

apps/sqlrooms-cli-ui/src/context/getRunContext.ts (1)

28-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract resolveCliBlockLabel and CLI_BLOCK_CONTEXT_TYPES to a shared module.

Both resolveCliBlockLabel and CLI_BLOCK_CONTEXT_TYPES are duplicated identically in useAssistantContextItems.ts (lines 31, 48-53). If the label resolution logic or supported block types change, both copies must be updated in sync. Extract these to a shared utility (e.g., in statefulBlockArtifactConfigs.ts or a new blockContextUtils.ts) and import from both call sites.

♻️ Proposed extraction

# In a shared file, e.g. apps/sqlrooms-cli-ui/src/blockContextUtils.ts

+import {CLI_AI_BLOCK_TYPES} from './artifactTypeIds';
+import {
+  getStatefulBlockArtifactConfig,
+  isStatefulBlockArtifactType,
+} from './statefulBlockArtifactConfigs';
+
+export const CLI_BLOCK_CONTEXT_TYPES = new Set<string>(CLI_AI_BLOCK_TYPES);
+
+export function resolveCliBlockLabel(blockType: string): string | undefined {
+  if (blockType === 'chart') return 'Chart';
+  return isStatefulBlockArtifactType(blockType)
+    ? getStatefulBlockArtifactConfig(blockType).label
+    : undefined;
+}

Then in both getRunContext.ts and useAssistantContextItems.ts:

-const CLI_BLOCK_CONTEXT_TYPES = new Set<string>(CLI_AI_BLOCK_TYPES);
+import {CLI_BLOCK_CONTEXT_TYPES, resolveCliBlockLabel} from '../blockContextUtils';

-function resolveCliBlockLabel(blockType: string): string | undefined {
-  if (blockType === 'chart') return 'Chart';
-  return isStatefulBlockArtifactType(blockType)
-    ? getStatefulBlockArtifactConfig(blockType).label
-    : undefined;
-}

Also applies to: 87-92

🤖 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/sqlrooms-cli-ui/src/context/getRunContext.ts` at line 28,
`resolveCliBlockLabel` and `CLI_BLOCK_CONTEXT_TYPES` are duplicated in both
`getRunContext` and `useAssistantContextItems`, so extract them into one shared
utility module and import them from both places. Put the shared label-resolution
logic and the `CLI_BLOCK_CONTEXT_TYPES` set in a common file such as
`blockContextUtils` or `statefulBlockArtifactConfigs`, then update
`getRunContext` and `useAssistantContextItems` to use the shared symbols instead
of maintaining separate copies.

packages/mosaic/src/ai/block-document/createBlockDocumentChartTools.ts (1)

79-82: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant type check after prior validation.

existingBlock?.type === 'chart' is redundant — line 72 already throws if existingBlock.type !== 'chart', and line 68 throws if !existingBlock. The optional chaining and type narrowing are unnecessary at this point.

♻️ Simplify selectionGroupId extraction

-        const selectionGroupId =
-          existingBlock?.type === 'chart'
-            ? existingBlock.selectionGroupId
-            : undefined;
+        const selectionGroupId = existingBlock.selectionGroupId;

🤖 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 `@packages/mosaic/src/ai/block-document/createBlockDocumentChartTools.ts`
around lines 79 - 82, Simplify the selectionGroupId logic in
createBlockDocumentChartTools: after the existing validation in the same
function already guarantees existingBlock is present and of type chart, remove
the redundant optional chaining and chart type check when reading
existingBlock.selectionGroupId. Update the assignment to access
existingBlock.selectionGroupId directly so the code reflects the prior guards
and stays easier to read.

apps/sqlrooms-cli-ui/src/ai/createCliBlockDocumentAiTools.ts (1)

181-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove unnecessary type assertion on augmentBlockSummary options

The as Parameters<typeof createListBlockDocumentBlocksTool>[0] & {...} cast is unnecessary. The callback ({block}) => {...} is compatible with the exported BlockDocumentBlockSummaryAugmenter type through contextual typing — TypeScript permits ignoring extra parameters (summary, index). The assertion also narrows the param type to only {block: BlockDocumentBlock}, bypassing the package's exported type and potentially masking future type errors.

♻️ Proposed fix

     augmentBlockSummary: ({block}) => {
       if (
         !getState ||
         block.type !== 'statefulBlock' ||
         block.blockType !== 'map' ||
         !block.blockInstanceId
       ) {
         return undefined;
       }

       const runtimeIssues = getMapBlockRuntimeIssues(
         getState(),
         block.blockInstanceId,
       );
       return runtimeIssues.length > 0 ? {runtimeIssues} : undefined;
     },
-  } as Parameters<typeof createListBlockDocumentBlocksTool>[0] & {
-    augmentBlockSummary: (params: {
-      block: BlockDocumentBlock;
-    }) => Record<string, unknown> | undefined;
-  });
+  });

🤖 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/sqlrooms-cli-ui/src/ai/createCliBlockDocumentAiTools.ts` around lines
181 - 185, The `createCliBlockDocumentAiTools` setup is using an unnecessary `as
Parameters<typeof createListBlockDocumentBlocksTool>[0] & {...}` assertion
around `augmentBlockSummary`, which bypasses the exported
`BlockDocumentBlockSummaryAugmenter` type. Remove the cast and let TypeScript
infer the callback shape directly from
`createListBlockDocumentBlocksTool`/`BlockDocumentBlockSummaryAugmenter`,
keeping the `({ block }) => ...` implementation contextually typed while
preserving the package’s public type contract.

packages/documents/src/createListBlockDocumentBlocksTool.ts (1)

31-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add tsdoc to exported BlockDocumentBlockSummaryAugmenter

As per coding guidelines, exported types in @sqlrooms/* packages should have tsdoc. BlockDocumentBlockSummaryAugmenter is a new public export from @sqlrooms/documents but lacks a tsdoc annotation.

As per coding guidelines: "Add tsdoc to exported types and functions in @sqlrooms/* packages."

🤖 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 `@packages/documents/src/createListBlockDocumentBlocksTool.ts` around lines 31
- 36, Add tsdoc for the exported BlockDocumentBlockSummaryAugmenter type in
createListBlockDocumentBlocksTool.ts. Document that it is a callback used to
augment a BlockDocumentBlockSummary, and describe the params block, summary, and
index so the public API in `@sqlrooms/documents` follows the exported-type tsdoc
guideline.

Source: Coding guidelines

packages/pivot/src/PivotBlock.tsx (2)

25-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add unit tests for the new PivotBlock component

PivotBlock is a new public export with non-trivial logic (useEffect calling ensurePivot, conditional fallback rendering). No test file was found in the provided changes.

As per coding guidelines: "Add or update tests when changing behavior or public API, when practical."

🤖 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 `@packages/pivot/src/PivotBlock.tsx` around lines 25 - 63, Add unit tests for
the new PivotBlock public export to cover its behavior. Verify PivotBlock calls
useBaseRoomStore’s ensurePivot inside the useEffect with the resolved pivotId
and the expected payload (title/defaultTitle, source, config), and that it falls
back to the missing pivot id message when both pivotId and blockId are absent.
Also assert it renders PivotView with the resolved pivotId when present, using
the PivotBlock and PivotBlockProps symbols to locate the component.

Source: Coding guidelines

---

25-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add tsdoc to exported PivotBlockProps and PivotBlock

As per coding guidelines, exported types and functions in @sqlrooms/* packages should have tsdoc. Both PivotBlockProps (line 25) and PivotBlock (line 32) are new public exports from @sqlrooms/pivot but lack tsdoc annotations.

As per coding guidelines: "Add tsdoc to exported types and functions in @sqlrooms/* packages."

🤖 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 `@packages/pivot/src/PivotBlock.tsx` around lines 25 - 63, Add tsdoc comments
for the new public exports in PivotBlock.tsx: document PivotBlockProps and
PivotBlock so they comply with the `@sqlrooms` package guidelines. Place concise
tsdoc directly above the PivotBlockProps type and the PivotBlock component
declaration, describing their purpose and key props/behavior without changing
runtime logic.

Source: Coding guidelines

packages/ai-core/src/components/BlockAiPromptPopover.tsx (1)

19-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add tsdoc to the exported type/function.

Neither BlockAiPromptPopoverProps nor BlockAiPromptPopover has tsdoc, but both are re-exported as public API from @sqlrooms/ai-core's index.

As per coding guidelines, "Add tsdoc to exported types and functions in @sqlrooms/* packages."

📝 Example tsdoc addition

+/**
+ * Props for {`@link` BlockAiPromptPopover}.
+ */
 export type BlockAiPromptPopoverProps = {
   trigger: ReactNode;
   onSubmit: (prompt: string) => void;
   placeholder?: string;
   label?: string;
   isRunning?: boolean;
   disabled?: boolean;
   open?: boolean;
   onOpenChange?: (open: boolean) => void;
 };

+/**
+ * A popover-based prompt input for triggering block-scoped Ask AI actions.
+ */
 export function BlockAiPromptPopover({

🤖 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 `@packages/ai-core/src/components/BlockAiPromptPopover.tsx` around lines 19 -
39, Add TSDoc comments to the public API symbols BlockAiPromptPopoverProps and
BlockAiPromptPopover in BlockAiPromptPopover.tsx. Document the purpose of the
props type and the component, including the main behavior and key props like
trigger, onSubmit, open, and onOpenChange, so the re-exported `@sqlrooms/ai-core`
API follows the package guideline for exported types/functions.

Source: Coding guidelines

packages/deck/src/createOrUpdateDeckMapBlock.ts (1)

1-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

DECK_MAP_DASHBOARD_PANEL_TYPE is imported but only re-exported, never used in the function body.

The symbol is imported at line 4 and re-exported at line 254, but createOrUpdateDeckMapBlock never references it. Since index.ts already exports DECK_MAP_DASHBOARD_PANEL_TYPE from ./dashboardConfig (line 46), this re-export is redundant for package consumers. Consider removing both the import and the re-export to reduce confusion.

♻️ Proposed cleanup

 import {
-  DECK_MAP_DASHBOARD_PANEL_TYPE,
   type DeckMapDashboardPanelConfig,
 } from './dashboardConfig';

And remove the re-export at the end:

-export {DECK_MAP_DASHBOARD_PANEL_TYPE};

Also applies to: 254-254

🤖 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 `@packages/deck/src/createOrUpdateDeckMapBlock.ts` around lines 1 - 10,
`DECK_MAP_DASHBOARD_PANEL_TYPE` is only being re-exported and never used inside
`createOrUpdateDeckMapBlock`, so remove the unused import from the module header
and delete the matching export at the end of `createOrUpdateDeckMapBlock` to
avoid redundant re-exporting. Keep the function’s existing behavior intact; the
symbol is already exported through `index.ts` via `dashboardConfig`, so no
additional package-level export is needed here.

packages/deck/__tests__/createOrUpdateDeckMapBlock.test.ts (1)

56-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding tests for missingMapBlockBehavior: 'create' and SQL-only source validation.

The three tests cover the main create, update, and throw paths. Two important branches in createOrUpdateDeckMapBlock are untested:

1. missingMapBlockBehavior: 'create' — generates a fresh mapId via createMapId() even when params.mapId is provided but no block exists (FSQ path).
2. SQL-only dataset source validation (lines 172-181) — throws when updating with sqlQuery sources and no tableName.

Adding these would improve confidence in the shared orchestration before both CLI and FSQ consume it.

🤖 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 `@packages/deck/__tests__/createOrUpdateDeckMapBlock.test.ts` around lines 56 -
107, Add tests for the uncovered branches in createOrUpdateDeckMapBlock: verify
that when missingMapBlockBehavior is set to create and the provided mapId does
not exist, the function uses createMapId() to create a new block instead of
throwing, and verify the SQL-only source validation path by asserting that an
update with sqlQuery sources and no tableName fails as expected. Use the
existing createOrUpdateDeckMapBlock test setup, especially createHost,
createMapId, and the mapId/blockId behavior, to keep the new cases consistent
with the current create/update/throw coverage.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

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".

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".

packages/documents/__tests__/startBlockScopedChat.test.ts (2)

153-188: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider asserting setPrompt and revealAssistant are not called in the blocking path.

The test correctly verifies toastError and that startAnalysisWhenReady is not called, but doesn't assert that setPrompt and revealAssistant were also skipped. Adding those negative assertions would fully lock the early-return contract.

🤖 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 `@packages/documents/__tests__/startBlockScopedChat.test.ts` around lines 153 -
188, The blocking-path test for startBlockScopedChat should also verify the
early return skips UI updates. In the existing "blocks when a matching session
is already running" case, add negative assertions that setPrompt and
revealAssistant are not called, alongside the current toastError and
startAnalysisWhenReady checks, to fully cover the contract in
startBlockScopedChat.

---

111-151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider asserting revealAssistant and setSessionDraftContextItemIds in the reuse path.

The test verifies the key side effects (switchSession, setPrompt, startAnalysisWhenReady) but doesn't assert that revealAssistant was called or that setSessionDraftContextItemIds was not called (since the context item id is already present in the draft). Adding these assertions would fully lock the reuse-path contract against regressions.

🤖 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 `@packages/documents/__tests__/startBlockScopedChat.test.ts` around lines 111 -
151, The reuse-path test for startBlockScopedChat should also cover the missing
side effects. In the existing “reuses a finished session” case, assert that
revealAssistant is called for the reused session and that
setSessionDraftContextItemIds is not called because the draft already contains
the context item. Use the existing startBlockScopedChat setup and the actions
object methods to lock down this contract alongside switchSession, setPrompt,
and startAnalysisWhenReady.

apps/sqlrooms-cli-ui/src/ai/createCliBlockDocumentAgentTool.ts (1)

608-616: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use STATEFUL_TARGET_BLOCK_TYPES constant instead of inline array in the early guard.

The early guard at line 610 uses ['dashboard', 'html-app', 'map'].includes(...) while validateTargetBlock uses the STATEFUL_TARGET_BLOCK_TYPES Set defined at line 328. If a new stateful block type is added to the Set but not the inline array, the early guard won't catch it, allowing ensureBlockDocument/setCurrentBlockDocument side effects before validateTargetBlock throws.

♻️ Proposed fix

         if (
           targetBlock &&
-          ['dashboard', 'html-app', 'map'].includes(targetBlock.blockType) &&
+          STATEFUL_TARGET_BLOCK_TYPES.has(targetBlock.blockType) &&
           !targetBlock.blockInstanceId
         ) {

🤖 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/sqlrooms-cli-ui/src/ai/createCliBlockDocumentAgentTool.ts` around lines
608 - 616, The early missing-blockInstanceId guard in
createCliBlockDocumentAgentTool should use the same STATEFUL_TARGET_BLOCK_TYPES
source as validateTargetBlock instead of an inline ['dashboard', 'html-app',
'map'] array. Update the guard to reference the STATEFUL_TARGET_BLOCK_TYPES Set
so any newly added stateful block type is caught consistently before
ensureBlockDocument or setCurrentBlockDocument can run.

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".

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".

packages/documents/__tests__/startBlockScopedChat.test.ts (1)

1-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Solid test structure covering the main session paths.

The ESM mock setup, createActions helper with Map-backed draft context, and the four test cases map well to the implementation's primary control flow: creation, invalid artifact rejection, finished-session reuse, and running-session blocking. The finished-session test correctly asserts that setSessionDraftContextItemIds is not called when the context item id is already present.

Two error paths in the implementation are not covered:

1. createArtifactScopedSession returns undefined → toast error "Cannot start an AI chat for this block"
2. startAnalysisWhenReady returns false → toast error "Could not start the AI chat"

Consider adding tests for these branches when practical.

🤖 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 `@packages/documents/__tests__/startBlockScopedChat.test.ts` around lines 1 -
37, The current `startBlockScopedChat` test suite covers the happy path and a
few session states, but it misses the two toast-error branches in the
`startBlockScopedChat` flow. Add tests using `createActions` for the cases where
`createArtifactScopedSession` returns `undefined` and where
`startAnalysisWhenReady` resolves to `false`, and assert that `toast.error` is
called with the corresponding messages. Use the existing `startBlockScopedChat`
import and the `toastError` mock to verify these fallback paths.

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".

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".

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".

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".

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".