Conversation
…ONAL, CONTEXT, SYSTEM) This is phase 1 of the tile type classification feature. Changes include: Schema changes: - Extend MapItemType enum with ORGANIZATIONAL, CONTEXT, SYSTEM values - Remove deprecated BASE type - Add migration (0014) to convert existing BASE tiles: - Favorited tiles + descendants → 'system' - All other BASE tiles → 'context' - USER tiles unchanged Code updates: - Replace all MapItemType.BASE references with MapItemType.CONTEXT (23+ files) - Update error message from BASE_ITEM_MUST_HAVE_PARENT to NON_USER_ITEM_MUST_HAVE_PARENT - Add comprehensive TDD tests for type classification The new types enable type-aware agent behavior: - organizational: Navigation structure (always visible) - context: Reference material (explore on-demand) - system: Executable capability (invoke via hexecute) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace 'module' variable name with 'toolsModule' to avoid Next.js warning - Use nullish coalescing operator instead of logical OR for safer defaults - Add eslint-disable comment for typeof import() pattern (necessary for dynamic import typing) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add documentation for itemType support in MCP tools: - Table showing type values and their semantic meanings - Which tools support itemType (addItem, updateItem, GET tools) - How agents should interpret each type 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Write tests BEFORE implementation (TDD red phase): - TypeSelectorField component tests (rendering, interaction, disabled state) - TileForm integration tests with type selector - useTileState hook tests for itemType state management Tests define expected behavior: - Dropdown with Organizational/Context/System options - Hidden for USER type tiles (root tiles) - itemType included in save callback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add itemType support to the tile creation and editing UI: - Create _TypeSelectorField component with dropdown for type selection - Update useTileState hook to track editItemType state - Integrate TypeSelectorField into TileForm - Update _handleSave to include itemType in save callback - Pass itemType through TileWidget props to TileForm Type options: Organizational, Context (default), System Hidden for USER type tiles (root tiles cannot change type) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove HTMLSelectElement type assertion that ESLint flagged as unnecessary. Use toBeDisabled() matcher instead of checking .disabled property. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add documentation for itemType support in the tile editing UI: - Table showing type values and their UI behavior - Key behaviors for type selector visibility and defaults - Updated responsibilities to include type editing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Document the new MapItemType enum values and their semantic meanings: CLAUDE.md: - Add Tile Types section explaining USER, ORGANIZATIONAL, CONTEXT, SYSTEM - Document agent behavior expectations for each type - Add migration note about BASE → split types UBIQUITOUS.md: - Add comprehensive definitions for each tile type - Explain semantic classification purpose - Document when agents should explore vs invoke tiles 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…chemas and handlers
The itemType field was not being passed from UI dropdown changes through to the API mutation, causing updates to not persist after page reload. Changes: - Add itemType to UpdateMapItemAttrsSchema in parameters.ts - Add toItemTypeEnum converter in mutation-callbacks.ts - Thread itemType through MutationCoordinator and mutation wrappers - Include itemType in TileData adapt function and tile handlers - Update all test files to include mandatory itemType in mock TileData 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Make itemType required in API schema (map-schemas.ts) instead of optional - Remove default fallback in service layer (_item-crud.service.ts) - Add itemType to all callers: agentic.ts, execute-task route, MCP tools - Thread itemType through frontend mutation coordinator chain - Create createTestItem factory helper with sensible defaults for tests - Update 38 test files to use factory instead of direct addItemToMap calls This ensures itemType is always explicitly passed in application code while tests can use the factory's default (CONTEXT) when they don't care about the specific type. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add NonUserMapItemType, NonUserMapItemTypeString, VisibilityString types - Replace hardcoded string unions with proper types across ~15 files - Add structural constraints for tile type hierarchies: - SYSTEM tiles must have SYSTEM structural descendants - CONTEXT tiles must have CONTEXT structural descendants - ORGANIZATIONAL tiles can only be under USER or ORGANIZATIONAL parents - Add batchUpdateItemTypeWithStructuralDescendants SQL query for cascade updates - When updating to SYSTEM/CONTEXT, cascade to structural descendants (1-6) - Skip hexplans (direction 0) and composition children (-1 to -6) - Add comprehensive integration tests for hierarchy validation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…used import - Extend test file ESLint override to disable no-unsafe-* rules - ESLint's TypeScript parser fails to resolve types from test helper factories, causing 320 false positive errors (pnpm typecheck passes) - Remove unused MapItemCreateAttributes import from _mutation-wrappers.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Feature tile types
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughReplaces legacy BASE tile type with semantic itemTypes (USER, ORGANIZATIONAL, CONTEXT, SYSTEM), threads itemType and a typed VisibilityString through DB, services, API, cache, mutations, and UI, adds a SQL migration to reclassify tiles, introduces a Mustache-based agentic templates subsystem and prompt events, and adds test helpers (createTestItem) with broad test updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
rect rgb(250,250,255)
participant User as User (UI)
participant TileWidget as TileWidget
participant UseTileState as useTileState
end
rect rgb(245,245,245)
participant MutationCoordinator as MutationCoordinator
participant Cache as Map Cache
end
rect rgb(240,255,240)
participant TRPC as TRPC Router
participant DB as Database
end
User->>TileWidget: edit title + select itemType
TileWidget->>UseTileState: set editItemType
User->>TileWidget: click Save
TileWidget->>MutationCoordinator: createItemOptimistic(coords, title, itemType)
MutationCoordinator->>Cache: _createOptimisticItem(..., itemType)
Cache-->>User: show optimistic UI (item with itemType)
MutationCoordinator->>TRPC: addItem mutation (payload includes itemType)
TRPC->>TRPC: toMapItemTypeEnum(itemType)
TRPC->>DB: insert map_item (item_type = enum)
DB-->>TRPC: created MapItem (with itemType)
TRPC-->>MutationCoordinator: return MapItemContract (itemType)
MutationCoordinator->>Cache: finalize item with server data (itemType)
Cache-->>User: UI updated (persisted itemType)
sequenceDiagram
autonumber
participant Migration as SQL Migration
participant Favorites as tile_favorites
participant Items as map_items
participant Write as writeQueries
Migration->>Favorites: select favorited map items
Migration->>Items: recursive CTE → compute descendants
Migration->>Write: batch update favorites+descendants → item_type = SYSTEM
Migration->>Write: update remaining BASE → CONTEXT
Migration->>Items: leave USER rows unchanged
Write-->>Migration: report counts / notices
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/lib/domains/mapping/types/__tests__/parameters-negative-directions.test.ts (1)
307-322: Update test descriptions to reflect CONTEXT type.The test descriptions at lines 307 and 324 still reference "BASE items" but the code now uses
MapItemType.CONTEXT. Update the descriptions for accuracy:
- Line 307: "should work with BASE items having composed children paths" → "should work with CONTEXT items having composed children paths"
- Line 324: "should work with BASE items at composition" → "should work with CONTEXT items at composition"
🔎 Suggested description updates
- it("should work with BASE items having composed children paths", () => { + it("should work with CONTEXT items having composed children paths", () => { const params: CreateMapItemParams = { itemType: MapItemType.CONTEXT,- it("should work with BASE items at composition (direction 0)", () => { + it("should work with CONTEXT items at composition (direction 0)", () => { const params: CreateMapItemParams = { itemType: MapItemType.CONTEXT,Also applies to: 324-339
src/lib/domains/mapping/types/__tests__/contracts.test.ts (1)
286-304: Update comments to reflect CONTEXT type.The inline comments at lines 297 and 367 still reference "BASE items" but the code now uses
MapItemType.CONTEXT. Update these comments for accuracy:// parentId - BASE items need a parentshould be:
// parentId - CONTEXT items need a parent🔎 Suggested comment updates
- 999 // parentId - BASE items need a parent + 999 // parentId - CONTEXT items need a parentAlso applies to: 356-378
src/lib/domains/mapping/services/__tests__/item-crud.integration.test.ts (1)
47-47: Update test description to reflect CONTEXT default.The test description mentions "BASE item" but the implementation now defaults to
MapItemType.CONTEXTvia thecreateTestItemhelper. This is correctly asserted at line 99, but the description should be updated for consistency.🔎 Suggested fix
- it("should add a child BASE item to a root USER item", async () => { + it("should add a child CONTEXT item to a root USER item", async () => {src/app/map/Cache/Lifecycle/MutationCoordinator/mutation-coordinator.test.ts (1)
299-316: Update mock responses to use validMapItemTypeenum values.The mock mutation responses use
itemType: "BASE" as const, but"BASE"is not a valid enum value—it's a legacy type that has been split intoORGANIZATIONAL,CONTEXT,SYSTEM, andUSER. The test fixtures correctly useMapItemType.CONTEXT. Update the mocks to either:
- Use
MapItemType.CONTEXTto match the fixtures, or- Use
nullto represent unclassified legacy tiles per the migration noteThis ensures the mocked server responses represent realistic current-state data.
src/lib/domains/mapping/services/__tests__/helpers/item-crud/_item-add-helpers.ts (1)
17-30: Validation mismatch: hardcoded CONTEXT vs. optional itemType parameter.The function accepts an optional
itemTypeparameter (line 14) and uses it when creating the item (line 20), but the validation at line 30 hardcodesMapItemType.CONTEXT. If a caller passes a differentitemType(e.g.,ORGANIZATIONAL), the test will fail unexpectedly.🔎 Proposed fix
- expect(childItemContract.itemType).toBe(MapItemType.CONTEXT); + expect(childItemContract.itemType).toBe(addItemArgs.itemType ?? MapItemType.CONTEXT);src/app/map/Cache/Lifecycle/MutationCoordinator/mutation-coordinator.ts (1)
1254-1269: Usetile.data.itemTypeinstead of hardcodingMapItemType.CONTEXTin_reconstructApiData.The method ignores
tile.data.itemType(which TileData already stores) and always reconstructs items asMapItemType.CONTEXT. Since_reconstructApiDatais used throughout rollback operations (delete, move, copy, visibility updates, etc.), items with other types (e.g.,ORGANIZATIONAL) will be restored with incorrect types, causing data loss.Change line 1264 from:
itemType: MapItemType.CONTEXT,to:
itemType: tile.data.itemType,
🧹 Nitpick comments (21)
drizzle/migrations/0014_migrate_item_types.sql (1)
19-22: Remove unused variabledescendant_ids.The
descendant_idsvariable is declared but never assigned or used. The recursive CTE result is stored directly inall_system_idsinstead.🔎 Proposed fix
DECLARE favorited_map_item_ids INTEGER[]; - descendant_ids INTEGER[]; all_system_ids INTEGER[]; BEGINsrc/app/map/Canvas/Tile/Item/_internals/hooks/__tests__/use-item-state.test.tsx (1)
69-69: Test mock correctly updated with semantic item type.The addition of
itemType: MapItemType.CONTEXTaligns the test fixture with the new TileData schema. UsingCONTEXTis appropriate since these tests focus on drag behavior rather than item-type-specific logic.Optional: Improve readability by placing properties on separate lines
- visibility: Visibility.PRIVATE, itemType: MapItemType.CONTEXT, + visibility: Visibility.PRIVATE, + itemType: MapItemType.CONTEXT,src/lib/domains/mapping/_objects/map-item.ts (1)
64-73: Clarify the distinction betweenref.itemTypeanditemType.Having two fields with similar names (
ref.itemTypeon Line 68 anditemTypeon Line 71) but different purposes creates confusion:
ref.itemTypeis described as a "reference marker"itemTypeis the "semantic tile type"This naming collision makes the codebase harder to understand and maintain. Consider either:
- Renaming
ref.itemTypeto something more distinctive (e.g.,ref.markerorref.category)- Adding a detailed comment explaining why both fields exist and when each should be used
- Consolidating if both aren't truly necessary
src/app/map/Chat/Timeline/Widgets/TileWidget/_internals/form/_TypeSelectorField.tsx (2)
3-3: Import ItemTypeValue from domain layer instead of duplicating.Line 3 defines a local
ItemTypeValuetype that duplicatesNonUserMapItemTypeStringfrom~/lib/domains/mapping/utils. This creates maintenance risk if the types diverge.🔎 Proposed fix to import from domain
+'use client'; + +import type { NonUserMapItemTypeString } from '~/lib/domains/mapping/utils'; + -type ItemTypeValue = 'organizational' | 'context' | 'system'; +type ItemTypeValue = NonUserMapItemTypeString;
24-36: Add label-input association for accessibility.The
<label>(Line 24) and<select>(Line 27) are not properly associated. For better accessibility, add anidto the select element and a matchinghtmlForattribute to the label.🔎 Proposed fix
<div> - <label className="text-xs text-muted-foreground mb-1 block"> + <label htmlFor="tile-type-selector" className="text-xs text-muted-foreground mb-1 block"> Type </label> <select + id="tile-type-selector" value={value} onChange={(e) => {src/app/map/Canvas/Menu/__tests__/items-builder-history.test.ts (1)
6-6: LGTM! Test data properly updated with itemType.The test has been correctly updated to include
itemType: MapItemType.CONTEXTin the mock data, aligning with the new semantic type system.Optional: Line 24 has both
visibilityanditemTypeon the same line. Consider splitting for better readability:- visibility: Visibility.PRIVATE, itemType: MapItemType.CONTEXT, + visibility: Visibility.PRIVATE, + itemType: MapItemType.CONTEXT,Also applies to: 24-24
src/app/map/Cache/Handlers/__tests__/navigation-handler.test.ts (1)
13-13: LGTM! Test fixtures updated to include semantic item type.The addition of
itemType: MapItemType.CONTEXTcorrectly aligns the test data with the new tile schema. UsingCONTEXTis appropriate for general test fixtures representing non-user tiles.Optional: Improve readability by placing properties on separate lines
For better readability, consider formatting multi-property objects with one property per line:
visibility: Visibility.PRIVATE, - itemType: MapItemType.CONTEXT, + itemType: MapItemType.CONTEXT,Currently on lines 70 and 97, both
visibilityanditemTypeappear inline which is valid but slightly harder to scan.Also applies to: 70-70, 97-97
src/app/map/Canvas/Tile/Item/_components/__tests__/item-tile-content-favorites.test.tsx (1)
8-8: LGTM! Test data correctly updated with semantic item type.The test fixture now includes the required
itemTypefield with an appropriate value for the test context.Optional: Format properties on separate lines
Line 57 has two properties on a single line, which reduces readability:
color: "zinc-50", - visibility: Visibility.PRIVATE, itemType: MapItemType.CONTEXT, + visibility: Visibility.PRIVATE, + itemType: MapItemType.CONTEXT,Also applies to: 57-57
src/app/map/Canvas/__tests__/TileContextMenu-composition.test.tsx (1)
8-8: LGTM! Test fixture extended with semantic item type.The mock tile data correctly includes the new
itemTypefield, maintaining consistency with the updated schema.Optional: Format properties on separate lines
Line 31 combines two properties on one line:
color: 'primary', - visibility: Visibility.PRIVATE, itemType: MapItemType.CONTEXT, + visibility: Visibility.PRIVATE, + itemType: MapItemType.CONTEXT,Also applies to: 31-31
src/app/map/Canvas/Tile/Base/__tests__/BaseComponents.test.tsx (1)
8-8: LGTM! Test fixtures updated with semantic item type.Both test cases now include the required
itemTypefield, correctly aligning with the new tile data schema.Optional: Format properties on separate lines
Lines 30 and 67 have two properties on single lines:
color: "zinc-500", - visibility: Visibility.PRIVATE, itemType: MapItemType.CONTEXT, + visibility: Visibility.PRIVATE, + itemType: MapItemType.CONTEXT,And similarly at line 67:
color: "amber-400", - visibility: Visibility.PRIVATE, itemType: MapItemType.CONTEXT, + visibility: Visibility.PRIVATE, + itemType: MapItemType.CONTEXT,Also applies to: 30-30, 67-67
src/app/map/Cache/State/__tests__/reducer.test.ts (1)
20-696: LGTM! Cache reducer tests properly updated with itemType field.The test fixtures now include
itemType: MapItemType.CONTEXT, correctly reflecting the expanded TileData.data shape. The migration is consistent across all test scenarios including region loading, item children, and edge cases.Minor formatting note: Some lines have trailing blank lines after the itemType addition (lines 24, 39, 167, 646). Consider removing these for consistency, though this is purely cosmetic.
src/app/map/Cache/Lifecycle/MutationCoordinator/__tests__/copy-item.test.ts (1)
323-325: Formatting inconsistency.The
visibilityanditemTypeproperties have inconsistent indentation compared to other properties in the object. This appears to be a formatting issue.Suggested fix
color: "#000000", - visibility: Visibility.PRIVATE, - itemType: MapItemType.CONTEXT, + visibility: Visibility.PRIVATE, + itemType: MapItemType.CONTEXT, },src/lib/domains/mapping/infrastructure/map-item/queries/write-queries.ts (1)
268-280: Consider adding a comment explaining the SQL path arithmetic.The
SUBSTRING(... FROM pathLen + 2)andSPLIT_PART(..., ',', 1)logic is non-obvious. The+2accounts for the parent path length plus the comma separator.// Filter: tile itself OR first direction after parent is positive (1-6) pathString === "" ? sql`(${mapItems.path} = '' OR ${mapItems.path} ~ '^[1-6](,.*)?$')` : sql`( ${mapItems.path} = ${pathString} OR ( + -- pathLen + 2 skips parent path + comma to get first child direction LENGTH(${mapItems.path}) > ${sql.raw(String(pathLen))} AND CAST(SPLIT_PART(SUBSTRING(${mapItems.path} FROM ${sql.raw(String(pathLen + 2))}), ',', 1) AS INTEGER) > 0 ) )`src/lib/domains/mapping/services/__tests__/item-deep-copy-negative-directions.test.ts (1)
8-8: Refactoring tocreateTestItemimproves consistency.The migration from direct
addItemToMapcalls to thecreateTestItemhelper centralizes test item creation and provides consistent defaults. The helper defaultsitemTypetoMapItemType.CONTEXT(per the helper implementation in_test-item-factory.ts).Optional: Consider explicit itemType for test clarity
For tests where the itemType might affect deep-copy behavior or assertions, consider explicitly specifying it even when using the default. This makes test intent clearer:
const parentItem = await createTestItem(testEnv, { parentId: rootMap.id, coords: _createTestCoordinates({ userId: setupParams.userId, groupId: setupParams.groupId, path: [Direction.East], }), title: "Parent Item", content: "Parent content", + itemType: MapItemType.CONTEXT, });This is particularly useful for tests verifying type-specific behavior.
Also applies to: 28-37, 40-49, 51-60
src/app/services/mcp/__tests__/mcp-itemtype.test.ts (1)
28-39: Consider extracting repeated type casting to a helper.The type assertion pattern for
itemTypePropertyis repeated across multiple tests. A helper function would reduce duplication and improve maintainability.🔎 Proposed helper extraction
// Add after line 14 function getToolItemTypeProperty(toolName: string) { const tool = mcpTools.find(t => t.name === toolName); return tool?.inputSchema.properties?.itemType as { type?: string; enum?: string[]; description?: string; } | undefined; } function getUpdateToolItemTypeProperty(toolName: string) { const tool = mcpTools.find(t => t.name === toolName); const updates = tool?.inputSchema.properties?.updates as { properties?: { itemType?: { type?: string; enum?: string[]; description?: string } }; }; return updates?.properties?.itemType; }src/app/map/Chat/Timeline/Widgets/TileWidget/useTileState.tsx (1)
32-37: Consider extracting the itemType validation logic to reduce duplication.The same conditional check for valid editable item types appears in both the initial state (lines 33-36) and the sync effect (lines 69-71). Extracting this to a helper would improve maintainability.
🔎 Proposed refactor
+const isEditableItemType = ( + type: ItemTypeValue +): type is 'organizational' | 'context' | 'system' => + type === 'organizational' || type === 'context' || type === 'system'; + +const getEditableItemType = (itemType: ItemTypeValue): 'organizational' | 'context' | 'system' => + isEditableItemType(itemType) ? itemType : 'context'; + // Default to 'context' if itemType is null/undefined -const [editItemType, setEditItemType] = useState<'organizational' | 'context' | 'system'>( - (itemType === 'organizational' || itemType === 'context' || itemType === 'system') - ? itemType - : 'context' -); +const [editItemType, setEditItemType] = useState<'organizational' | 'context' | 'system'>( + getEditableItemType(itemType) +);Then use
getEditableItemType(itemType)in the useEffect as well.Also applies to: 68-73
src/app/map/Chat/Timeline/Widgets/TileWidget/_internals/_handlers.ts (1)
16-17: Consider consolidating theEditableItemTypetype definition.This same type alias is defined in multiple files (
useTileState.tsx,TileForm.tsx, and here). Consider exporting it from a shared location to ensure consistency and reduce duplication.🔎 Proposed approach
Create a shared types file or export from a central location:
// In a shared types file, e.g., ~/app/map/Chat/Timeline/Widgets/TileWidget/types.ts export type EditableItemType = 'organizational' | 'context' | 'system'; export type ItemTypeValue = EditableItemType | 'user' | null;Then import from that location in all three files.
src/lib/domains/mapping/_objects/__tests__/map-item-type-classification.test.ts (1)
53-65: Consider the maintenance implications of asserting exact enum count.While this test ensures BASE is removed, the assertion
expect(enumValues).toHaveLength(4)may cause test failures if new valid types are added in the future. Consider documenting this expectation or using a more flexible approach.🔎 Alternative approach
it("should have exactly 4 enum values", () => { const enumValues = Object.values(MapItemType); - expect(enumValues).toHaveLength(4); + // Explicitly verify the expected types exist; update count if new types are added + const expectedTypes = ["user", "organizational", "context", "system"]; + expect(enumValues).toHaveLength(expectedTypes.length); expect(enumValues).toContain("user"); expect(enumValues).toContain("organizational"); expect(enumValues).toContain("context"); expect(enumValues).toContain("system"); });src/lib/domains/mapping/services/__tests__/helpers/_test-item-factory.ts (1)
76-98: Consider documenting the direction validation expectation.The shorthands
createTestStructuralChildandcreateTestComposedChildsetitemTypedefaults but don't validate that thecoords.pathends with an appropriate direction (1-6 for structural, -1 to -6 for composed). This is likely intentional for test flexibility, but consider adding a brief comment noting that callers are responsible for providing coords with the correct direction.src/app/services/mcp/handlers/tools.ts (1)
325-325: Consider adding runtime validation for itemType in updates.The
updatesobject is cast to includeitemType?: NonUserMapItemTypeString, but unlikeaddItem, there's no validation that the provideditemTypevalue (if present) is one of the valid enum values. While TypeScript types help at compile time, MCP clients send arbitrary JSON. Consider validatingupdates.itemTypeif provided.🔎 Example validation
const validItemTypes = ["organizational", "context", "system"]; if (updates.itemType && !validItemTypes.includes(updates.itemType)) { throw new Error(`Invalid itemType: ${updates.itemType}. Must be one of: ${validItemTypes.join(", ")}`); }src/lib/domains/mapping/services/_item-services/_item-crud.service.ts (1)
539-542: Consider logging when parent is not found during update validation.The silent return when catching an error during parent lookup could mask data integrity issues. If the parent should exist but doesn't, this might indicate orphaned tiles or corrupted data.
🔎 Suggested improvement
} catch { - // Parent not found, skip validation + // Parent not found - log for observability but don't block the update + // This can happen with orphaned tiles or during concurrent operations + console.warn(`Parent not found at ${JSON.stringify(parentCoords)} during itemType update validation for coords ${JSON.stringify(coords)}`); return; }
- Create dedicated templates subsystem at lib/domains/agentic/templates/ - Move template logic from prompt-builder.ts to _prompt-builder.ts - Add _system-template.ts with SYSTEM tile template constants - Add dependencies.json declaring mustache and mapping dependencies - Clean index.ts to only re-export public API (buildPrompt, PromptData) - Make itemType required in PromptData (remove fallback behavior) - Fix cross-domain imports to use public mapping API - Add README.md documenting subsystem responsibilities 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
refactor(agentic): extract templates subsystem with encapsulated API
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/lib/domains/agentic/templates/_system-template.ts (1)
67-87: Consider surfacing blocked steps even when pending steps exist.The template shows three mutually exclusive states:
hexplanBlocked,hexplanComplete, andhexplanPending. However, the state detection logic (in_prompt-builder.tslines 152-177) prioritizeshexplanPendingwhen📋exists, which means blocked (🔴) steps are silently ignored if any pending step remains.This could mask important blockers from the agent. Consider either:
- Adding a warning about blocked steps within the
hexplanPendingblock, or- Making
hexplanBlockedtake precedence overhexplanPendingBased on learnings,
🔴 BLOCKEDindicates tasks that need intervention, so surfacing them promptly seems important.src/lib/domains/agentic/templates/_prompt-builder.ts (1)
152-177: Emoji-based status detection may produce false positives.Using
data.hexPlan.includes('📋')andincludes('🔴')could match emoji appearing in user content, not just status markers. Consider checking for line-start patterns like/^📋/mor using more structured status tracking.Additionally, the state priority means blocked steps are hidden when pending steps exist (as noted in the template review).
🔎 More robust pattern matching
- const hasPendingSteps = data.hexPlan.includes('📋') - const hasBlockedSteps = data.hexPlan.includes('🔴') + const hasPendingSteps = /^📋/m.test(data.hexPlan) + const hasBlockedSteps = /^🔴/m.test(data.hexPlan)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
package.jsonsrc/lib/domains/agentic/dependencies.jsonsrc/lib/domains/agentic/services/task-execution.service.tssrc/lib/domains/agentic/templates/README.mdsrc/lib/domains/agentic/templates/_prompt-builder.tssrc/lib/domains/agentic/templates/_system-template.tssrc/lib/domains/agentic/templates/dependencies.jsonsrc/lib/domains/agentic/templates/index.tssrc/lib/domains/agentic/utils/__tests__/prompt-builder.test.tssrc/lib/domains/agentic/utils/prompt-builder.tssrc/server/api/routers/agentic/agentic.ts
✅ Files skipped from review due to trivial changes (2)
- src/lib/domains/agentic/templates/README.md
- src/lib/domains/agentic/templates/dependencies.json
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use absolute imports with
~/prefix instead of relative imports (./or../) - this is enforced by ESLintno-restricted-importsrule
Files:
src/lib/domains/agentic/templates/_system-template.tssrc/lib/domains/agentic/templates/_prompt-builder.tssrc/lib/domains/agentic/templates/index.tssrc/lib/domains/agentic/utils/__tests__/prompt-builder.test.tssrc/lib/domains/agentic/services/task-execution.service.tssrc/lib/domains/agentic/utils/prompt-builder.tssrc/server/api/routers/agentic/agentic.ts
src/lib/domains/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/lib/domains/**/*.{ts,tsx}: Use direction values: positive 1-6 for subtask children, negative -1 to -6 for context children, and direction 0 for hexplan (execution state and progress tracking)
Use Domain-Driven Design in src/lib/domains/ with clear boundaries between mapping, IAM, and other domains
Files:
src/lib/domains/agentic/templates/_system-template.tssrc/lib/domains/agentic/templates/_prompt-builder.tssrc/lib/domains/agentic/templates/index.tssrc/lib/domains/agentic/utils/__tests__/prompt-builder.test.tssrc/lib/domains/agentic/services/task-execution.service.tssrc/lib/domains/agentic/utils/prompt-builder.ts
src/lib/domains/agentic/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Hexplan tiles (direction-0 children) track execution state using exact emoji-prefixed status markers: 🟡 STARTED (task execution began), ✅ COMPLETED (task finished successfully), 🔴 BLOCKED (task stuck, needs intervention). Steps without prefix are pending (📋 for display only)
Files:
src/lib/domains/agentic/templates/_system-template.tssrc/lib/domains/agentic/templates/_prompt-builder.tssrc/lib/domains/agentic/templates/index.tssrc/lib/domains/agentic/utils/__tests__/prompt-builder.test.tssrc/lib/domains/agentic/services/task-execution.service.tssrc/lib/domains/agentic/utils/prompt-builder.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for tests (not Jest)
Files:
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts
src/server/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use tRPC for type-safe API with server-side caching and optimizations in the backend
Files:
src/server/api/routers/agentic/agentic.ts
🧠 Learnings (4)
📚 Learning: 2026-01-01T14:56:07.704Z
Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T14:56:07.704Z
Learning: Applies to src/lib/domains/agentic/**/*.{ts,tsx} : Hexplan tiles (direction-0 children) track execution state using exact emoji-prefixed status markers: 🟡 STARTED (task execution began), ✅ COMPLETED (task finished successfully), 🔴 BLOCKED (task stuck, needs intervention). Steps without prefix are pending (📋 for display only)
Applied to files:
src/lib/domains/agentic/templates/_system-template.tssrc/lib/domains/agentic/utils/__tests__/prompt-builder.test.tssrc/lib/domains/agentic/services/task-execution.service.tssrc/lib/domains/agentic/utils/prompt-builder.tssrc/server/api/routers/agentic/agentic.ts
📚 Learning: 2026-01-01T14:56:07.704Z
Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T14:56:07.704Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Use Vitest for tests (not Jest)
Applied to files:
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts
📚 Learning: 2026-01-01T14:56:07.704Z
Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T14:56:07.704Z
Learning: Applies to src/lib/domains/**/*.{ts,tsx} : Use direction values: positive 1-6 for subtask children, negative -1 to -6 for context children, and direction 0 for hexplan (execution state and progress tracking)
Applied to files:
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.tssrc/lib/domains/agentic/utils/prompt-builder.tssrc/server/api/routers/agentic/agentic.ts
📚 Learning: 2026-01-01T14:56:07.704Z
Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T14:56:07.704Z
Learning: A tile is either a leaf (does concrete work with no subtask children) or a parent (orchestrates children), never both. Leaf tiles describe WHAT to do and WHY, while parent tiles only orchestrate children
Applied to files:
src/lib/domains/agentic/utils/prompt-builder.ts
🧬 Code graph analysis (3)
src/lib/domains/agentic/templates/_prompt-builder.ts (1)
src/lib/domains/agentic/templates/_system-template.ts (4)
ANCESTOR_INTRO(133-133)SYSTEM_TEMPLATE(46-118)SystemTemplateData(12-40)HEXRUN_INTRO(123-128)
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts (1)
src/lib/domains/agentic/templates/_prompt-builder.ts (2)
PromptData(13-42)buildPrompt(204-213)
src/server/api/routers/agentic/agentic.ts (3)
src/lib/domains/mapping/utils/index.ts (1)
MapItemType(29-29)src/lib/domains/mapping/services/__tests__/helpers/_test-utilities.ts (1)
MapItemType(13-13)src/lib/domains/mapping/types/contracts.ts (1)
MapItemType(30-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Tests - Phase 2 (React Components)
- GitHub Check: Build
- GitHub Check: Type Check
- GitHub Check: Tests - Phase 1 (Main Suite)
- GitHub Check: ESLint
🔇 Additional comments (21)
src/lib/domains/agentic/dependencies.json (1)
13-14: LGTM! Templates subsystem correctly registered.The addition of the templates subsystem aligns with the new itemType-aware prompt generation infrastructure introduced in this PR.
src/lib/domains/agentic/services/task-execution.service.ts (1)
140-142: LGTM! ItemType correctly propagated to prompt builder.The itemType field from the task is correctly passed through to the buildPrompt function, enabling template selection based on tile semantics.
src/lib/domains/agentic/templates/index.ts (1)
1-11: LGTM! Clean public API design.The public entry point follows best practices with clear documentation and proper encapsulation of the internal
_prompt-buildermodule. The absolute import path correctly uses the~/prefix as per coding guidelines.src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts (2)
3-3: LGTM! ItemType properly integrated into test data.The MapItemType import and default value (SYSTEM) correctly establish the foundation for itemType-aware testing throughout the suite.
Also applies to: 15-15
543-571: Error handling implementation is correct and well-tested.The error throwing logic is properly implemented in
_getTemplateByItemType()function, with error messages defined inTEMPLATE_LOOKUP_ERRORS. All three unsupported itemTypes (ORGANIZATIONAL, USER, CONTEXT) correctly throw the expected errors. Tests use Vitest as required and accurately verify the error handling behavior.package.json (1)
92-92: Mustache library is current and has no known vulnerabilities. The package.json already specifies version ^4.2.0, which is the latest stable release as of January 2026. No security advisories are associated with the mustache package.src/server/api/routers/agentic/agentic.ts (4)
6-6: LGTM!Import correctly uses absolute path with
~/prefix as per coding guidelines, andMapItemTypeis properly imported alongside other mapping utilities.
61-62: LGTM!Setting
itemType: MapItemType.SYSTEMfor hexplan tiles is correct. Per the domain conventions, hexplan tiles (direction-0 children) are system-generated execution state trackers, makingSYSTEMthe appropriate item type.
510-512: LGTM!Propagating
itemTypefrom the task context to the prompt builder enables template selection based on tile type. The pattern is consistent with the hexecute endpoint below.
663-665: LGTM!Consistent with the
executeTaskendpoint -itemTypeis correctly propagated to the prompt builder for template selection.src/lib/domains/agentic/templates/_system-template.ts (3)
12-40: LGTM!The
SystemTemplateDatainterface provides a clean contract for template rendering with appropriate flags for conditional sections and pre-rendered content blocks.
88-118: Execution instructions align with domain conventions.The step-by-step instructions correctly reference the emoji status markers (
📋→✅for completion,🔴 BLOCKEDfor failures) and follow the "execute one step, then return" pattern for iterative execution. The parent/leaf tile distinction usingisParentTileis well-implemented.
123-133: LGTM!Static intro constants provide clear context for the HEXRUN execution model and ancestor relationships.
src/lib/domains/agentic/templates/_prompt-builder.ts (5)
7-9: LGTM!Imports correctly use absolute paths with
~/prefix as per coding guidelines.
13-42: LGTM!The
PromptDatainterface provides a clean API for prompt generation, withitemTypeenabling template selection. The separation ofcomposedChildren(context tiles with content) andstructuralChildren(subtask tiles with previews) aligns with the domain model.
46-57: LGTM!XML escaping covers all standard entities, and the
_hasContenthelper provides consistent empty-string handling.
116-139: Exhaustive switch is good practice.The
nevertype check ensures compile-time errors if newMapItemTypevalues are added without handling them here. The explicit error messages for unimplemented types are helpful.
204-212: Minor: Template data preparation is hardcoded to SYSTEM.
_prepareSystemTemplateDatais called unconditionally, but_getTemplateByItemTypevalidates the type first. When USER/CONTEXT templates are implemented, this will need a corresponding data preparation function. This is acceptable for now since only SYSTEM is supported.src/lib/domains/agentic/utils/prompt-builder.ts (3)
11-12: LGTM!Clean re-export from templates subsystem using absolute import path. This maintains backward compatibility while centralizing prompt-building logic.
25-52: LGTM!
generateParentHexplanContentcorrectly uses the emoji status markers per domain conventions:
🟡 STARTEDfor task execution began📋prefix for pending steps (display only)The dual-mode handling (flat leaf tasks for root tiles vs. direct children for intermediate parents) supports the execution tracking model.
58-74: LGTM!
generateLeafHexplanContentcreates a simple execution plan for leaf tiles with the correct🟡 STARTEDmarker and📋pending step. The optional instruction inclusion is a nice touch.
Remove redundant nested ref structure from MapItem attrs. The old
`ref: { itemType: MapItemType; itemId: number }` was premature
abstraction - itemType was always CONTEXT, never stored in DB,
and never read anywhere.
Simplified to `baseItemId: number` which directly represents the
foreign key to BaseItem. The actual BaseItem object remains
accessible via `mapItem.ref` (the RelatedItems property).
Changes:
- Attrs interface: ref → baseItemId
- Constructor: simplified default logic
- Infrastructure: mappers, db.ts, write-queries.ts
- Domain helpers: creation, copy, management services
- Tests: updated all ref constructions
No database migration needed - DB already stored just refItemId.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add PromptGeneratedEvent SSE event type to stream the hexecute prompt - Emit prompt before LLM streaming starts via onPromptBuilt callback - Store prompt in streaming message state and persist after finalization - Create CollapsiblePrompt component with MarkdownRenderer for display - Show prompt in chat messages (collapsed by default, before thinking indicator) - Remove XML escaping from content fields in prompt builder to preserve intentional XML structure in tile content (keeps escaping for attributes) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/map/Chat/Timeline/_components/CollapsiblePrompt.tsx (1)
15-17: Consider dynamic summary text for better UX.The summary text remains static as "Show the prompt" whether the details are open or closed. For improved user experience, consider updating the text to reflect the current state (e.g., "Show the prompt" when collapsed, "Hide the prompt" when expanded).
🔎 Optional refinement with dynamic text
- <summary className="cursor-pointer text-muted-foreground hover:text-foreground transition-colors"> - Show the prompt - </summary> + <summary className="cursor-pointer text-muted-foreground hover:text-foreground transition-colors [&[open]]:before:content-['Hide_the_prompt'] before:content-['Show_the_prompt']"> + </summary>Alternatively, use React state with the
onToggleevent:import { MarkdownRenderer } from '~/app/map/Chat/Timeline/_components/MarkdownRenderer' import { useState } from 'react' interface CollapsiblePromptProps { prompt: string } export function CollapsiblePrompt({ prompt }: CollapsiblePromptProps) { const [isOpen, setIsOpen] = useState(false) return ( <details className="mb-2 text-xs" onToggle={(e) => setIsOpen((e.target as HTMLDetailsElement).open)} > <summary className="cursor-pointer text-muted-foreground hover:text-foreground transition-colors"> {isOpen ? 'Hide the prompt' : 'Show the prompt'} </summary> <div className="mt-2 p-3 bg-muted/50 rounded-md overflow-x-auto max-h-96 overflow-y-auto"> <MarkdownRenderer content={prompt} isSystemMessage={false} /> </div> </details> ) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/app/api/stream/execute-task/route.tssrc/app/map/Chat/Timeline/_components/CollapsiblePrompt.tsxsrc/app/map/Chat/Timeline/_components/MessageActorRenderer.tsxsrc/app/map/Chat/_hooks/_streaming-chat-callbacks.tssrc/app/map/Chat/_hooks/_streaming-utils.tssrc/app/map/Chat/_state/_events/_creators/streaming-creators.tssrc/app/map/Chat/_state/_events/event.creators.tssrc/app/map/Chat/_state/_events/event.types.tssrc/app/map/Chat/_state/_events/index.tssrc/app/map/Chat/_state/_operations/message-operations.tssrc/app/map/Chat/_state/_selectors/streaming-message-handlers.tssrc/lib/domains/agentic/index.tssrc/lib/domains/agentic/services/task-execution.service.tssrc/lib/domains/agentic/templates/_prompt-builder.tssrc/lib/domains/agentic/types/stream.types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/domains/agentic/templates/_prompt-builder.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use absolute imports with
~/prefix instead of relative imports (./or../) - this is enforced by ESLintno-restricted-importsrule
Files:
src/app/map/Chat/_hooks/_streaming-utils.tssrc/app/map/Chat/_state/_events/_creators/streaming-creators.tssrc/app/map/Chat/Timeline/_components/CollapsiblePrompt.tsxsrc/app/map/Chat/_state/_operations/message-operations.tssrc/app/map/Chat/Timeline/_components/MessageActorRenderer.tsxsrc/app/map/Chat/_state/_events/index.tssrc/lib/domains/agentic/types/stream.types.tssrc/lib/domains/agentic/index.tssrc/app/api/stream/execute-task/route.tssrc/lib/domains/agentic/services/task-execution.service.tssrc/app/map/Chat/_hooks/_streaming-chat-callbacks.tssrc/app/map/Chat/_state/_events/event.creators.tssrc/app/map/Chat/_state/_selectors/streaming-message-handlers.tssrc/app/map/Chat/_state/_events/event.types.ts
src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Next.js 15 App Router with progressive enhancement following Static → Progressive → Dynamic component patterns
Files:
src/app/map/Chat/_hooks/_streaming-utils.tssrc/app/map/Chat/_state/_events/_creators/streaming-creators.tssrc/app/map/Chat/Timeline/_components/CollapsiblePrompt.tsxsrc/app/map/Chat/_state/_operations/message-operations.tssrc/app/map/Chat/Timeline/_components/MessageActorRenderer.tsxsrc/app/map/Chat/_state/_events/index.tssrc/app/api/stream/execute-task/route.tssrc/app/map/Chat/_hooks/_streaming-chat-callbacks.tssrc/app/map/Chat/_state/_events/event.creators.tssrc/app/map/Chat/_state/_selectors/streaming-message-handlers.tssrc/app/map/Chat/_state/_events/event.types.ts
src/lib/domains/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/lib/domains/**/*.{ts,tsx}: Use direction values: positive 1-6 for subtask children, negative -1 to -6 for context children, and direction 0 for hexplan (execution state and progress tracking)
Use Domain-Driven Design in src/lib/domains/ with clear boundaries between mapping, IAM, and other domains
Files:
src/lib/domains/agentic/types/stream.types.tssrc/lib/domains/agentic/index.tssrc/lib/domains/agentic/services/task-execution.service.ts
src/lib/domains/agentic/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Hexplan tiles (direction-0 children) track execution state using exact emoji-prefixed status markers: 🟡 STARTED (task execution began), ✅ COMPLETED (task finished successfully), 🔴 BLOCKED (task stuck, needs intervention). Steps without prefix are pending (📋 for display only)
Files:
src/lib/domains/agentic/types/stream.types.tssrc/lib/domains/agentic/index.tssrc/lib/domains/agentic/services/task-execution.service.ts
🧠 Learnings (3)
📚 Learning: 2026-01-01T14:56:07.704Z
Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T14:56:07.704Z
Learning: Applies to src/lib/domains/agentic/**/*.{ts,tsx} : Hexplan tiles (direction-0 children) track execution state using exact emoji-prefixed status markers: 🟡 STARTED (task execution began), ✅ COMPLETED (task finished successfully), 🔴 BLOCKED (task stuck, needs intervention). Steps without prefix are pending (📋 for display only)
Applied to files:
src/lib/domains/agentic/index.tssrc/app/api/stream/execute-task/route.tssrc/lib/domains/agentic/services/task-execution.service.ts
📚 Learning: 2026-01-01T14:56:07.704Z
Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T14:56:07.704Z
Learning: Applies to src/lib/domains/**/*.{ts,tsx} : Use direction values: positive 1-6 for subtask children, negative -1 to -6 for context children, and direction 0 for hexplan (execution state and progress tracking)
Applied to files:
src/lib/domains/agentic/index.tssrc/app/api/stream/execute-task/route.ts
📚 Learning: 2026-01-01T14:56:07.704Z
Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T14:56:07.704Z
Learning: Applies to src/lib/domains/**/*.{ts,tsx} : Use Domain-Driven Design in src/lib/domains/ with clear boundaries between mapping, IAM, and other domains
Applied to files:
src/app/api/stream/execute-task/route.ts
🧬 Code graph analysis (7)
src/app/map/Chat/_state/_events/_creators/streaming-creators.ts (3)
src/app/map/Chat/_state/_events/event.creators.ts (1)
createStreamingMessagePromptEvent(35-35)src/app/map/Chat/_state/_events/index.ts (2)
createStreamingMessagePromptEvent(44-44)ChatEvent(11-11)src/app/map/Chat/_state/_events/event.types.ts (1)
ChatEvent(31-37)
src/app/map/Chat/Timeline/_components/CollapsiblePrompt.tsx (1)
src/app/map/Chat/Timeline/_components/MarkdownRenderer.tsx (1)
MarkdownRenderer(10-21)
src/app/map/Chat/_state/_operations/message-operations.ts (1)
src/app/map/Chat/_state/_events/_creators/streaming-creators.ts (1)
createStreamingMessagePromptEvent(50-58)
src/app/map/Chat/Timeline/_components/MessageActorRenderer.tsx (1)
src/app/map/Chat/Timeline/_components/CollapsiblePrompt.tsx (1)
CollapsiblePrompt(12-23)
src/lib/domains/agentic/services/task-execution.service.ts (1)
src/lib/domains/agentic/index.ts (6)
TaskExecutionCallbacks(92-92)executeTaskStreaming(89-89)TaskExecutionInput(91-91)AgenticService(8-8)StreamChunk(29-29)LLMResponse(28-28)
src/app/map/Chat/_state/_selectors/streaming-message-handlers.ts (2)
src/app/map/Chat/_state/_events/event.types.ts (1)
StreamingMessagePromptPayload(147-150)src/app/map/Chat/_state/_events/index.ts (1)
StreamingMessagePromptPayload(28-28)
src/app/map/Chat/_state/_events/event.types.ts (1)
src/app/map/Chat/_state/_events/index.ts (1)
StreamingMessagePromptPayload(28-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Tests - Phase 1 (Main Suite)
- GitHub Check: Tests - Phase 2 (React Components)
- GitHub Check: Build
- GitHub Check: ESLint
🔇 Additional comments (26)
src/app/map/Chat/Timeline/_components/MessageActorRenderer.tsx (2)
8-8: LGTM!The import follows the coding guidelines by using an absolute import with the
~/prefix.
135-135: Message type correctly includes the optionalpromptproperty.The
Messageinterface insrc/app/map/Chat/_state/_events/event.types.tsdefinesprompt?: string, and the conditional rendering correctly guards access to it.src/app/map/Chat/Timeline/_components/CollapsiblePrompt.tsx (1)
1-23: LGTM!The component implementation is clean and follows best practices:
- Uses absolute imports with
~/prefix per coding guidelines- Leverages native HTML
<details>/<summary>elements for progressive enhancement- Provides good accessibility out of the box
- Integrates seamlessly with the existing
MarkdownRenderersrc/lib/domains/agentic/services/task-execution.service.ts (2)
140-141: LGTM! itemType propagation is correct.The addition of
itemType: task.itemTypeto thebuildPromptcall properly threads semantic item typing through the prompt-building pipeline, consistent with the PR's objectives.
189-217: LGTM! Lifecycle callback implementation is sound.The
TaskExecutionCallbacksinterface and its integration intoexecuteTaskStreamingis well-implemented:
- Properly documented with JSDoc
- Correctly uses optional chaining for safe callback invocation
- Prompt extraction safely handles empty messages array with fallback to empty string
- Timing is appropriate (after prompt building, before streaming)
src/app/map/Chat/_state/_events/index.ts (1)
28-28: LGTM! Event type and creator exports are consistent.The additions of
StreamingMessagePromptPayloadandcreateStreamingMessagePromptEventmaintain consistency with the existing event export patterns.Also applies to: 44-44
src/app/map/Chat/_state/_events/_creators/streaming-creators.ts (1)
46-58: LGTM! Event creator follows established patterns.The
createStreamingMessagePromptEventfunction is correctly implemented:
- Consistent with existing streaming event creators in this file
- Proper JSDoc documentation
- Appropriate payload structure with
streamIdandpromptfields- Uses the same ID generation pattern as other creators
src/app/map/Chat/_hooks/_streaming-utils.ts (2)
16-16: LGTM! Callback signature is appropriate.The
onPromptGeneratedcallback addition toStreamingCallbacksis well-typed and consistent with other callbacks in the interface.
62-64: LGTM! Event handling is correct.The
prompt_generatedevent handling is properly implemented:
- Uses optional chaining for safe callback invocation
- Correctly returns
shouldClose: falseto continue streaming- Follows the pattern of other non-terminal event handlers
src/lib/domains/agentic/index.ts (1)
61-61: LGTM! Public API exports are complete and consistent.The additions properly expose the new types and type guards through the domain's public API:
PromptGeneratedEventalongside other stream event typesisPromptGeneratedEventalongside other event type guardsTaskExecutionCallbacksalongside other task execution typesAlso applies to: 75-75, 92-92
src/app/map/Chat/_state/_operations/message-operations.ts (1)
9-9: LGTM! Clean addition of prompt event.The
setMessagePromptmethod follows the same pattern as other streaming operations and correctly uses the absolute import with~/prefix. The implementation is straightforward and type-safe.Also applies to: 42-44
src/app/map/Chat/_state/_events/event.types.ts (1)
25-25: LGTM! Type definitions are well-structured.The new event type, payload interface, and optional Message.prompt field are correctly defined and documented. The changes integrate cleanly with the existing event type system.
Also applies to: 50-51, 147-150
src/app/api/stream/execute-task/route.ts (4)
38-38: LGTM! Imports follow conventions.The imports correctly use absolute paths with
~/prefix and add necessary types for the new prompt generation flow.Also applies to: 44-45
149-152: LGTM! SSE helper follows existing pattern.The
_emitPromptGeneratedhelper is consistent with other SSE emission functions (_emitTextDelta,_emitDone) and correctly constructs thePromptGeneratedEvent.
214-215: TheaddItemToMapmethod signature correctly accepts theitemTypeparameter.The method in
src/lib/domains/mapping/services/_item-services/_item-crud.service.ts(line 44+) hasitemType: MapItemTypeas a required parameter in its signature. The code at lines 214-215 correctly passesitemType: MapItemType.SYSTEMwhen creating the Hexplan item.
308-313: Remove this comment - the code is correct.The
executeTaskStreamingfunction signature expects 4 parameters:input,agenticService,onChunk, andcallbacks?: TaskExecutionCallbacks. The code correctly passesonPromptBuiltas a property of the fourth parameter (callbacksobject), not the third parameter. The fourth parameter is properly typed asTaskExecutionCallbacks, which hasonPromptBuilt?: (prompt: string) => voiddefined in the interface.Likely an incorrect or invalid review comment.
src/app/map/Chat/_state/_events/event.creators.ts (1)
35-35: LGTM! Export addition is consistent.The
createStreamingMessagePromptEventre-export follows the existing pattern and correctly uses absolute imports.src/app/map/Chat/_hooks/_streaming-chat-callbacks.ts (1)
40-42: LGTM! Callback implementation is correct.The
onPromptGeneratedcallback correctly uses thegetStreamId()helper to avoid stale closures and follows the same pattern as other callbacks in this function. The implementation properly bridges the streaming event to the chat state.src/lib/domains/agentic/types/stream.types.ts (3)
32-40: LGTM! Clean event interface definition.The
PromptGeneratedEventinterface follows the established discriminated union pattern and is well-documented. The comment clarifying "full XML prompt" provides helpful context for consumers.
158-166: LGTM! Proper union type extension.The
PromptGeneratedEventis correctly integrated into the discriminated union, maintaining type safety for event handling.
185-190: LGTM! Type guard follows established pattern.The type guard implementation is consistent with other guards in the file and properly enables type-safe event handling.
src/app/map/Chat/_state/_selectors/streaming-message-handlers.ts (5)
1-8: LGTM! Proper absolute import usage.The import follows the coding guideline requirement for absolute imports with
~/prefix.
13-20: LGTM! Consistent state extension.The optional
promptfield follows the same pattern as themodelfield and is appropriately documented.
64-74: LGTM! Consistent event handling pattern.The prompt event handler follows the established defensive pattern used by other streaming event handlers, with proper payload validation and state updates.
87-96: LGTM! Prompt properly included in finalized messages.The prompt is correctly threaded through from streaming state to the finalized message using optional chaining.
114-124: LGTM! Prompt properly included in streaming messages.The prompt is consistently included in in-progress streaming messages, maintaining parity with finalized message structure.
Tests now correctly expect that only XML attributes are escaped, while content bodies preserve raw text for LLM processing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use absolute imports with
~/prefix instead of relative imports (./or../) - this is enforced by ESLintno-restricted-importsrule
Files:
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts
src/lib/domains/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/lib/domains/**/*.{ts,tsx}: Use direction values: positive 1-6 for subtask children, negative -1 to -6 for context children, and direction 0 for hexplan (execution state and progress tracking)
Use Domain-Driven Design in src/lib/domains/ with clear boundaries between mapping, IAM, and other domains
Files:
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts
src/lib/domains/agentic/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Hexplan tiles (direction-0 children) track execution state using exact emoji-prefixed status markers: 🟡 STARTED (task execution began), ✅ COMPLETED (task finished successfully), 🔴 BLOCKED (task stuck, needs intervention). Steps without prefix are pending (📋 for display only)
Files:
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for tests (not Jest)
Files:
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts
🧠 Learnings (3)
📚 Learning: 2026-01-01T14:56:07.704Z
Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T14:56:07.704Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Use Vitest for tests (not Jest)
Applied to files:
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts
📚 Learning: 2026-01-01T14:56:07.704Z
Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T14:56:07.704Z
Learning: Applies to src/lib/domains/agentic/**/*.{ts,tsx} : Hexplan tiles (direction-0 children) track execution state using exact emoji-prefixed status markers: 🟡 STARTED (task execution began), ✅ COMPLETED (task finished successfully), 🔴 BLOCKED (task stuck, needs intervention). Steps without prefix are pending (📋 for display only)
Applied to files:
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts
📚 Learning: 2026-01-01T14:56:07.704Z
Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T14:56:07.704Z
Learning: Applies to src/lib/domains/**/*.{ts,tsx} : Use direction values: positive 1-6 for subtask children, negative -1 to -6 for context children, and direction 0 for hexplan (execution state and progress tracking)
Applied to files:
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts
🧬 Code graph analysis (1)
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts (2)
src/lib/domains/agentic/templates/_prompt-builder.ts (2)
PromptData(13-42)buildPrompt(204-213)src/lib/domains/agentic/utils/prompt-builder.ts (2)
PromptData(12-12)buildPrompt(12-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: ESLint
- GitHub Check: Build
- GitHub Check: Tests - Phase 1 (Main Suite)
- GitHub Check: Tests - Phase 2 (React Components)
🔇 Additional comments (3)
src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts (3)
3-3: LGTM!The import uses the required absolute path with
~/prefix, consistent with the project's coding guidelines.
7-18: LGTM!The test helper with the default
itemType: MapItemType.SYSTEMproperly aligns with the updatedPromptDatainterface and provides a sensible default for test scenarios.
189-191: Implementation correctly handles XML escaping as tested.Verification confirms the
buildPromptimplementation properly implements the escaping policy: XML attributes are escaped via_escapeXML()while content bodies preserve raw text for LLM processing. This behavior is consistently applied across all sections (composed children contexts, subtask previews, task content, and hexplan) and matches all test expectations exactly.
| // ==================== UNSUPPORTED ITEM TYPES ==================== | ||
| describe('buildPrompt error handling for unsupported itemTypes', () => { | ||
| it('should throw error for ORGANIZATIONAL itemType', () => { | ||
| const data = createTestData({ | ||
| task: { title: 'Test', content: 'Content', coords: 'userId,0:1' }, | ||
| itemType: MapItemType.ORGANIZATIONAL | ||
| }) | ||
|
|
||
| expect(() => buildPrompt(data)).toThrow('ORGANIZATIONAL tiles cannot be executed') | ||
| }) | ||
|
|
||
| it('should throw error for USER itemType', () => { | ||
| const data = createTestData({ | ||
| task: { title: 'Test', content: 'Content', coords: 'userId,0:1' }, | ||
| itemType: MapItemType.USER | ||
| }) | ||
|
|
||
| expect(() => buildPrompt(data)).toThrow('USER tile templates not yet implemented') | ||
| }) | ||
|
|
||
| it('should throw error for CONTEXT itemType', () => { | ||
| const data = createTestData({ | ||
| task: { title: 'Test', content: 'Content', coords: 'userId,0:1' }, | ||
| itemType: MapItemType.CONTEXT | ||
| }) | ||
|
|
||
| expect(() => buildPrompt(data)).toThrow('CONTEXT tile templates not yet implemented') | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the buildPrompt implementation and error handling
rg -n 'ORGANIZATIONAL tiles cannot be executed|USER tile templates not yet implemented|CONTEXT tile templates not yet implemented' src/lib/domains/agentic/ -A 2 -B 2Repository: Diplow/hexframe
Length of output: 2089
🏁 Script executed:
# Look for buildPrompt implementation
fd prompt-builder.ts src/lib/domains/agentic/ | grep -v testRepository: Diplow/hexframe
Length of output: 159
🏁 Script executed:
# Search for MapItemType enum definition
rg -n 'enum MapItemType|type MapItemType' src/lib/ -A 15Repository: Diplow/hexframe
Length of output: 4053
🏁 Script executed:
# Find where these errors are thrown in the implementation
rg -n 'throw.*ORGANIZATIONAL|throw.*USER|throw.*CONTEXT' src/lib/domains/agentic/ -B 3 -A 1Repository: Diplow/hexframe
Length of output: 1671
🏁 Script executed:
# Verify MapItemType enum definition to understand CONTEXT itemType
cat -n src/lib/domains/mapping/_objects/map-item.ts | sed -n '31,45p'Repository: Diplow/hexframe
Length of output: 534
🏁 Script executed:
# Check how error throwing is implemented in buildPrompt
rg -n 'organizationalNotSupported|userNotImplemented|contextNotImplemented' src/lib/domains/agentic/ -B 2 -A 1Repository: Diplow/hexframe
Length of output: 1676
Fix ORGANIZATIONAL error message test to verify complete error string.
The ORGANIZATIONAL test on line 555 only checks for a partial error message. The implementation throws 'ORGANIZATIONAL tiles cannot be executed - they are structural groupings only', but the test expects 'ORGANIZATIONAL tiles cannot be executed'. While the partial match passes with Vitest's .toThrow(), the test is incomplete and doesn't verify the full error message.
Update line 555 to:
expect(() => buildPrompt(data)).toThrow('ORGANIZATIONAL tiles cannot be executed - they are structural groupings only')The USER and CONTEXT tests correctly match their implementation error messages.
Note: MapItemType.CONTEXT (the enum value) does not conflict with the coding guideline's "CONTEXT" for context children with negative directions—they are separate concepts used in different contexts.
🤖 Prompt for AI Agents
In src/lib/domains/agentic/utils/__tests__/prompt-builder.test.ts around lines
547 to 575, update the ORGANIZATIONAL error-message assertion to check the full
error string thrown by buildPrompt; replace the current partial expectation on
line 555 with one that expects "ORGANIZATIONAL tiles cannot be executed - they
are structural groupings only" so the test verifies the complete error message.
1 participant
Summary by CodeRabbit
New Features
Data Migration
Documentation
Tests & Tools
✏️ Tip: You can customize this high-level summary in your review settings.