Add Visual Identity expression packs and VN casting resolver#2584
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request implements Stages 11A and 11B of the Visual Identity Expression Packs, introducing a VN generated-file import bridge with asset-level provenance (source_context) and a stateless VN role/casting resolver. It also integrates these overrides into the VN Play runtime and resolves various pre-existing frontend TypeScript type errors to ensure a clean baseline check. The reviewer feedback highlights critical improvements: avoiding database schema initialization on every request to prevent SQLite lock contention, securing temporary file creation during ZIP imports against filename collisions, and handling file descriptor cleanup properly to prevent resource leaks if os.fdopen fails.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
PR Summary by QodoAdd Visual Identity expression packs and VN casting resolver
AI Description
Diagram
High-Level Assessment
Files changed (115)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
74 rules 1.
|
ffad4cd to
0d4328e
Compare
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tldw_Server_API/app/core/VN_Play/service.py (2)
906-924: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInconsistent
active_spritescontract between success and fallback paths.On the happy path (line 943)
active_spritesis always set, even as an empty list. On manifest-build failure (lines 917-923) the key is only addedif active_sprites:, so it can be entirely absent from the response when there are no embedded Visual Identity sprites. Clients relying onscene_state.active_spritesalways being an array could break.🐛 Proposed fix
active_sprites = [ sprite for sprite in _list_of_dicts(enriched.get("active_sprite_items")) if _is_embedded_scene_asset(sprite) ] - if active_sprites: - enriched["active_sprites"] = active_sprites + enriched["active_sprites"] = active_sprites return enriched🤖 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 `@tldw_Server_API/app/core/VN_Play/service.py` around lines 906 - 924, The fallback branch in VN_Play service currently returns an inconsistent scene state because `active_sprites` is only added when non-empty. Update the exception path in `service.py` around `self._build_pack_manifest(...)` so `enriched["active_sprites"]` is always set to a list, matching the happy-path contract used elsewhere in the scene state response. Keep the existing filtering logic with `_list_of_dicts(...)` and `_is_embedded_scene_asset(...)`, but assign the result unconditionally before returning `enriched`.
4253-4304: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
directive_error_typemislabeled for Visual Identity directives when the resolver is unavailable.
directive_error_typeis initialized to the (unrelated) VN-asset-manifestmanifest_errorfor every directive, then only overwritten inside theexceptblocks. Whenis_visual_identityis true andvisual_identity_resolver is None, onlydirective_rejection_reasonis set to"visual_identity_resolver_unavailable"—directive_error_typeis left as whatevermanifest_errorhappened to be (e.g. a stale manifest exception class name from an unrelated non-identity directive in the same turn, orNone). The resulting rejected-directive event/warning then carries a misleadingerror_typeunrelated to the actual failure cause.🐛 Proposed fix
resolution = None - directive_error_type = manifest_error - directive_rejection_reason = default_rejection_reason if is_visual_identity: + directive_error_type = None if visual_identity_resolver is None: directive_rejection_reason = "visual_identity_resolver_unavailable" else: try: resolution = resolve_visual_identity_directive( visual_identity_resolver, directive_payload, ) except Exception as exc: logger.exception( "Failed to resolve VN Play Visual Identity directive: session_id={}", session_id, ) directive_error_type = exc.__class__.__name__ directive_rejection_reason = "resolver_error" elif manifest is not None: + directive_error_type = manifest_error + directive_rejection_reason = default_rejection_reason try: resolution = resolve_visual_directive( manifest, directive_payload, seed=f"{seed}:{index}", ) except Exception as exc: logger.exception( "Failed to resolve VN Play visual directive: session_id={}, pack_id={}", session_id, session.vn_asset_pack_id, ) directive_error_type = exc.__class__.__name__ directive_rejection_reason = "resolver_error" + else: + directive_error_type = manifest_error + directive_rejection_reason = default_rejection_reason🤖 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 `@tldw_Server_API/app/core/VN_Play/service.py` around lines 4253 - 4304, The Visual Identity directive rejection path in this loop is leaving directive_error_type tied to manifest_error even when visual_identity_resolver is missing, so the recorded error can be stale or unrelated. Update the handling in the directive processing block around resolve_visual_identity_directive so that when is_visual_identity is true and visual_identity_resolver is None, directive_error_type is explicitly set to a Visual Identity–specific value (and not inherited from manifest_error), alongside directive_rejection_reason = "visual_identity_resolver_unavailable". Ensure the event/warning emitted from this branch reflects the actual Visual Identity resolver failure rather than the VN manifest state.
🤖 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.
Inline comments:
In `@apps/packages/ui/src/components/Common/Playground/Message.tsx`:
- Around line 850-858: `VisualIdentityImage` is receiving `previewUrl` and
`isAnimated` from `props.visualPreviewUrl`/`props.visualIsAnimated` even when
`portraitImage` has fallen back to `moodCharacterAvatar`, `baseCharacterAvatar`,
or `props.modelImage` instead of `visualCharacterAvatar`. Update the `Message`
component’s `portraitImage`/`resolvedModelImage` flow so those props are only
passed when the resolved image is actually the visual identity asset, and
otherwise clear them so the fallback avatar renders correctly.
In
`@apps/packages/ui/src/components/Common/VisualIdentity/ExpressionSlotGrid.tsx`:
- Around line 8-17: `CANONICAL_SLOT_ORDER` in `ExpressionSlotGrid` duplicates
the canonical expression keys already defined in
`VISUAL_IDENTITY_EXPRESSION_OPTIONS`, so update the grid to derive its order
from that shared source instead of maintaining a second hardcoded list. Use the
shared expression options export from `utils/visual-identity-expressions.ts` and
map/extract the canonical keys in `ExpressionSlotGrid.tsx`, keeping the same
order but ensuring any future additions or renames stay in sync automatically.
In
`@apps/packages/ui/src/components/Common/VisualIdentity/VisualIdentityDraftReview.tsx`:
- Around line 80-102: slotFromEntry is doing redundant work by calling
assetToSlot(asset, buildAssetUrl) only to keep the nested asset field and
discard the rest. Refactor the VisualIdentityDraftReview.tsx logic to use a
helper that returns just the asset shape (or otherwise reuse the already
available asset data) so the code is clearer and avoids building an unused full
slot object; keep the change localized around slotFromEntry, assetToSlot, and
buildAssetUrl.
In
`@apps/packages/ui/src/components/Common/VisualIdentity/VisualIdentityImage.tsx`:
- Around line 3-28: The reduced-motion state in usePrefersReducedMotion is
initialized to false and only corrected in useEffect, which causes
VisualIdentityImage to request the animated asset on the first render. Update
usePrefersReducedMotion to derive its initial value synchronously from
window.matchMedia when available, while keeping the effect/listener logic for
updates so the first render can choose previewUrl immediately for reduced-motion
users.
In
`@apps/packages/ui/src/components/Common/VisualIdentity/VisualIdentityPackPanel.tsx`:
- Around line 196-237: Pre-validate the selected ZIP in handleArchiveSelected
before calling fileToUploadData/startVisualIdentityZipImport by checking the
fetched capabilities first. Use the existing capabilities fields
(upload_max_bytes, archive_max_bytes, supported_mime_types) to reject
unsupported MIME types and oversized files up front, set an informative error,
and avoid making the network call when the file is obviously invalid. Keep the
validation close to handleArchiveSelected so the same logic can be reused for
the other upload flow mentioned in the diff.
- Around line 213-228: The polling timeout feedback in VisualIdentityPackPanel
misses non-terminal statuses other than importing, so update the post-loop
status handling after the draft polling in the VisualIdentityPackPanel logic.
Replace the exact latestDraft.status === "importing" check with a condition that
covers any non-terminal status when the loop ends without reaching a terminal
draft state, and keep using setStatusMessage to show the refresh guidance.
Reference the polling block that uses isTerminalDraftStatus,
DRAFT_POLL_ATTEMPTS, and setDraft so the fix applies regardless of whether the
final status is queued, validating, or importing.
In `@apps/packages/ui/src/components/Option/AudioStudio/TimelineEditor.tsx`:
- Around line 209-212: The ref callback in TimelineEditor’s setAudioElementRef
is setting referrerpolicy on an <audio> element, but that attribute is not
applied there. Remove the no-op element?.setAttribute("referrerpolicy",
"no-referrer") from setAudioElementRef and apply the referrer policy at a
document-level <meta name="referrer"> or via the server response header instead,
keeping the audio ref setup unchanged.
In `@apps/packages/ui/src/hooks/__tests__/useVisualIdentityResolver.test.tsx`:
- Around line 10-14: Reset the module-level resolver caches between tests so
each case starts clean. In useVisualIdentityResolver.test.tsx, extend the
existing beforeEach alongside vi.clearAllMocks() to clear the cache Maps used by
useVisualIdentityResolver.ts, so tests don’t accidentally reuse stale results
from prior cases. Locate the cache-handling logic in useVisualIdentityResolver
and make the test setup explicitly reset those singletons before every test.
In `@apps/packages/ui/src/hooks/useVisualIdentityResolver.ts`:
- Around line 273-361: The availability cache bypass in
useVisualIdentityResolver is tied to revision === 0, so after refresh()
increments revision once, the effect will never reuse cached availability for
that component instance again. Update the cache check in the React.useEffect
block to distinguish an initial load from later refreshes without permanently
disabling cache hits, and keep refresh() focused on invalidating the current
cacheKey and triggering a re-fetch rather than changing the cache eligibility
logic.
- Around line 118-225: The cache hit logic in useVisualIdentityResolver is
incorrectly gated by revision === 0, which prevents any later cacheKey from
reusing an already cached resolution after refresh() has been called once.
Update the effect’s cache check so it only depends on
resolutionCache.has(cacheKey) (and the existing null/guard conditions), and keep
refresh() responsible for invalidating the current cache entry and bumping
revision to force a re-run for the same cacheKey.
In `@apps/packages/ui/src/services/scheduled-tasks-control-plane.ts`:
- Line 288: The buildQuery helper is widened too far by using object, which
drops key/value typing for Object.entries. Update buildQuery to use a generic
constraint such as <T extends Record<string, unknown>>(params?: T) so callers
can pass typed request interfaces while preserving type safety inside the
function. Keep the change focused on the buildQuery symbol and any related call
sites that rely on its parameter typing.
In `@backlog/tasks/task-12090` -
Plan-shared-visual-identity-expression-packs-implementation.md:
- Line 4: The task metadata is inconsistent because the status is marked Done
while the Definition of Done checklist remains unchecked. Update the task entry
so the status in the frontmatter and the DOD items in the task body match by
either completing the checklist in the task document or changing the status back
out of Done; use the existing task frontmatter status field and the checklist
section in the task content to locate the mismatch.
In `@backlog/tasks/task-12090.3` -
Plan-VN-visual-identity-bridge-and-resolver-implementation.md:
- Around line 46-48: The FINAL_SUMMARY section has duplicate closing markers,
making the document structure ambiguous. Update the markdown around the
FINAL_SUMMARY boundary in the task document so there is only one
`SECTION:FINAL_SUMMARY:END` marker and the section is closed exactly once.
In `@backlog/tasks/task-12090.4` -
Implement-Stage-11A-VN-visual-identity-generated-file-import-bridge.md:
- Around line 37-54: The task notes are currently outside the
IMPLEMENTATION_NOTES section because the SECTION:IMPLEMENTATION_NOTES block
closes before the verification text. Move the existing task/status content back
inside the IMPLEMENTATION_NOTES markers in this markdown file, keeping the notes
under that section and leaving the rest of the document unchanged.
In `@backlog/tasks/task-12090.5` -
Implement-Stage-11B-VN-visual-identity-role-and-casting-resolver.md:
- Around line 66-72: The implementation notes are duplicated, which will trigger
markdownlint MD024; fold the new Task 10 text into the existing Implementation
Notes block instead of creating a second same-level heading. Update the content
around the existing Implementation Notes section in this task document, keeping
one `## Implementation Notes` heading and appending the follow-up notes there,
or rename the later heading if a separate section is truly needed.
- Around line 52-54: The document contains a duplicated FINAL_SUMMARY closer, so
remove the extra SECTION:FINAL_SUMMARY:END marker and keep only a single closing
marker for the final summary section; locate the repeated markers near the end
of the markdown task doc and clean up the redundant one so the section structure
is unambiguous.
In `@backlog/tasks/task-12098` -
Implement-visual-identity-pack-management-and-draft-review-UI.md:
- Around line 23-27: The document has duplicated section fences in the markdown
sections, so the parser will see nested BEGIN/END markers. Clean up the affected
sections by keeping only one matching begin/end pair per section, especially the
SECTION:DESCRIPTION block and the duplicated SECTION:FINAL_SUMMARY:END markers,
and verify the same fix in the other referenced section. Use the section marker
names to locate and remove the extra fences without changing the section
content.
---
Outside diff comments:
In `@tldw_Server_API/app/core/VN_Play/service.py`:
- Around line 906-924: The fallback branch in VN_Play service currently returns
an inconsistent scene state because `active_sprites` is only added when
non-empty. Update the exception path in `service.py` around
`self._build_pack_manifest(...)` so `enriched["active_sprites"]` is always set
to a list, matching the happy-path contract used elsewhere in the scene state
response. Keep the existing filtering logic with `_list_of_dicts(...)` and
`_is_embedded_scene_asset(...)`, but assign the result unconditionally before
returning `enriched`.
- Around line 4253-4304: The Visual Identity directive rejection path in this
loop is leaving directive_error_type tied to manifest_error even when
visual_identity_resolver is missing, so the recorded error can be stale or
unrelated. Update the handling in the directive processing block around
resolve_visual_identity_directive so that when is_visual_identity is true and
visual_identity_resolver is None, directive_error_type is explicitly set to a
Visual Identity–specific value (and not inherited from manifest_error),
alongside directive_rejection_reason = "visual_identity_resolver_unavailable".
Ensure the event/warning emitted from this branch reflects the actual Visual
Identity resolver failure rather than the VN manifest state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9e738505-37b0-4706-9f78-60b263f9a8e6
⛔ Files ignored due to path filters (11)
Docs/superpowers/plans/2026-06-24-audio-studio-media-tickets-implementation-plan.mdis excluded by!docs/**Docs/superpowers/plans/2026-07-01-openai-realtime-speech-endpoint-implementation-plan.mdis excluded by!docs/**Docs/superpowers/plans/2026-07-01-visual-identity-expression-packs-implementation-plan.mdis excluded by!docs/**Docs/superpowers/plans/2026-07-02-vn-visual-identity-bridge-implementation-plan.mdis excluded by!docs/**Docs/superpowers/plans/2026-07-03-frontend-tsc-baseline-cleanup-plan.mdis excluded by!docs/**Docs/superpowers/specs/2026-06-20-research-source-discovery-chokepoint-design.mdis excluded by!docs/**Docs/superpowers/specs/2026-06-30-skills-uat-quality-gates-design.mdis excluded by!docs/**Docs/superpowers/specs/2026-07-01-openai-realtime-speech-endpoint-design.mdis excluded by!docs/**Docs/superpowers/specs/2026-07-01-visual-identity-expression-packs-design.mdis excluded by!docs/**Docs/superpowers/specs/2026-07-02-vn-visual-identity-bridge-design.mdis excluded by!docs/**apps/packages/ui/node_modules/antdis excluded by!**/node_modules/**
📒 Files selected for processing (113)
apps/packages/ui/src/components/Common/Playground/Message.tsxapps/packages/ui/src/components/Common/Playground/__tests__/visual-identity-message-state.test.tsxapps/packages/ui/src/components/Common/Playground/message-types.tsapps/packages/ui/src/components/Common/Playground/useMessageState.tsapps/packages/ui/src/components/Common/VisualIdentity/ExpressionAssetUploader.tsxapps/packages/ui/src/components/Common/VisualIdentity/ExpressionPicker.tsxapps/packages/ui/src/components/Common/VisualIdentity/ExpressionSlotGrid.tsxapps/packages/ui/src/components/Common/VisualIdentity/VisualIdentityDraftReview.tsxapps/packages/ui/src/components/Common/VisualIdentity/VisualIdentityImage.tsxapps/packages/ui/src/components/Common/VisualIdentity/VisualIdentityPackPanel.tsxapps/packages/ui/src/components/Common/VisualIdentity/VisualIdentityStage.tsxapps/packages/ui/src/components/Common/VisualIdentity/__tests__/ExpressionPicker.test.tsxapps/packages/ui/src/components/Common/VisualIdentity/__tests__/ExpressionSlotGrid.test.tsxapps/packages/ui/src/components/Common/VisualIdentity/__tests__/VisualIdentityDraftReview.test.tsxapps/packages/ui/src/components/Common/VisualIdentity/__tests__/VisualIdentityImage.test.tsxapps/packages/ui/src/components/Common/VisualIdentity/__tests__/VisualIdentityPackPanel.test.tsxapps/packages/ui/src/components/Common/VisualIdentity/__tests__/VisualIdentityStage.test.tsxapps/packages/ui/src/components/Common/VisualIdentity/__tests__/useGeneratedFileImportAction.test.tsapps/packages/ui/src/components/Common/VisualIdentity/useGeneratedFileImportAction.tsapps/packages/ui/src/components/Notes/__tests__/NotesListPanel.stage18.accessibility-selected-state.test.tsxapps/packages/ui/src/components/Notes/__tests__/NotesListPanel.stage46.empty-error-states.test.tsxapps/packages/ui/src/components/Notes/__tests__/NotesSidebar.stage46.list-error-count.test.tsxapps/packages/ui/src/components/Option/AudioStudio/TimelineEditor.tsxapps/packages/ui/src/components/Option/Characters/CharacterEditorForm.tsxapps/packages/ui/src/components/Option/Characters/__tests__/Manager.first-use.test.tsxapps/packages/ui/src/components/Option/Playground/PlaygroundChat.tsxapps/packages/ui/src/components/Option/Playground/PlaygroundCompareCluster.tsxapps/packages/ui/src/components/Option/ResearchWorkspace/__tests__/AddSourceModal.stage1.ingestion.test.tsxapps/packages/ui/src/components/Option/ScheduledTasks/ScheduledTaskAutomationDefinitionEditor.tsxapps/packages/ui/src/components/Option/ScheduledTasks/ScheduledTaskCreatePanel.tsxapps/packages/ui/src/components/Option/Setup/__tests__/AudioInstallerPanel.test.tsxapps/packages/ui/src/components/Option/Skills/Manager.tsxapps/packages/ui/src/components/Option/Skills/__tests__/SkillsWorkspace.test.tsxapps/packages/ui/src/components/PersonaGarden/VisualPackEditor.tsxapps/packages/ui/src/db/dexie/__tests__/audiobook-migration.test.tsapps/packages/ui/src/entries/background.tsapps/packages/ui/src/hooks/__tests__/useVisualIdentityResolver.test.tsxapps/packages/ui/src/hooks/chat/__tests__/useChatActions.character.integration.test.tsxapps/packages/ui/src/hooks/chat/useChatActions.tsapps/packages/ui/src/hooks/chat/useServerChatLoader.tsapps/packages/ui/src/hooks/useMessageOption.tsxapps/packages/ui/src/hooks/useVisualIdentityResolver.tsapps/packages/ui/src/services/__tests__/tldw-api-client.visual-identities.test.tsapps/packages/ui/src/services/scheduled-tasks-control-plane.tsapps/packages/ui/src/services/tldw/TldwApiClient.tsapps/packages/ui/src/services/tldw/domains/index.tsapps/packages/ui/src/services/tldw/domains/visual-identities.tsapps/packages/ui/src/services/tldw/mcp-hub.tsapps/packages/ui/src/services/tldw/voice-cloning.tsapps/packages/ui/src/store/option/types.tsapps/packages/ui/src/types/visual-identities.tsapps/packages/ui/src/utils/__tests__/visual-identity-emote.test.tsapps/packages/ui/src/utils/__tests__/visual-identity-expressions.test.tsapps/packages/ui/src/utils/visual-identity-emote.tsapps/packages/ui/src/utils/visual-identity-expressions.tsbacklog/tasks/task-12082 - Revise-research-discovery-ingest-boundary-design-on-dev.mdbacklog/tasks/task-12088 - Design-OpenAI-compatible-realtime-speech-endpoint.mdbacklog/tasks/task-12089 - Design-shared-visual-identity-expression-packs-for-character-persona-chat.mdbacklog/tasks/task-12090 - Plan-shared-visual-identity-expression-packs-implementation.mdbacklog/tasks/task-12090.1 - Implement-visual-identity-chat-runtime-portraits-picker-and-stage.mdbacklog/tasks/task-12090.2 - Design-VN-visual-identity-import-bridge-and-casting-resolver.mdbacklog/tasks/task-12090.3 - Plan-VN-visual-identity-bridge-and-resolver-implementation.mdbacklog/tasks/task-12090.4 - Implement-Stage-11A-VN-visual-identity-generated-file-import-bridge.mdbacklog/tasks/task-12090.5 - Implement-Stage-11B-VN-visual-identity-role-and-casting-resolver.mdbacklog/tasks/task-12091 - Implement-visual-identity-expression-pack-backend-foundation.mdbacklog/tasks/task-12092 - Implement-visual-identity-asset-storage-validation.mdbacklog/tasks/task-12093 - Implement-visual-identity-ZIP-import-draft-jobs.mdbacklog/tasks/task-12094 - Implement-visual-identity-service-activation-and-resolution.mdbacklog/tasks/task-12095 - Implement-visual-identity-API-schemas-and-endpoints.mdbacklog/tasks/task-12096 - Implement-visual-identity-character-chat-message-metadata-integration.mdbacklog/tasks/task-12097 - Implement-visual-identity-frontend-API-client-and-expression-utilities.mdbacklog/tasks/task-12098 - Implement-visual-identity-pack-management-and-draft-review-UI.mdbacklog/tasks/task-12099 - Clean-up-frontend-TypeScript-baseline-blockers-for-Visual-Identity-branch-finish.mdbacklog/tasks/task-530.14 - Implement-Skills-UAT-and-quality-gates.mdtldw_Server_API/app/api/v1/endpoints/character_chat_sessions.pytldw_Server_API/app/api/v1/endpoints/visual_identities.pytldw_Server_API/app/api/v1/endpoints/vn_play.pytldw_Server_API/app/api/v1/router_groups/core.pytldw_Server_API/app/api/v1/schemas/visual_identity_schemas.pytldw_Server_API/app/core/DB_Management/VisualIdentity_DB.pytldw_Server_API/app/core/DB_Management/db_path_utils.pytldw_Server_API/app/core/VN_Play/assets.pytldw_Server_API/app/core/VN_Play/service.pytldw_Server_API/app/core/Visual_Identities/__init__.pytldw_Server_API/app/core/Visual_Identities/archive_import.pytldw_Server_API/app/core/Visual_Identities/constraints.pytldw_Server_API/app/core/Visual_Identities/expression_slots.pytldw_Server_API/app/core/Visual_Identities/jobs.pytldw_Server_API/app/core/Visual_Identities/service.pytldw_Server_API/app/core/Visual_Identities/source_context.pytldw_Server_API/app/core/Visual_Identities/storage.pytldw_Server_API/app/core/Visual_Identities/vn_bridge.pytldw_Server_API/app/services/startup_content_jobs_pollers.pytldw_Server_API/app/services/startup_tail_finalization.pytldw_Server_API/app/services/visual_identity_jobs_worker.pytldw_Server_API/tests/Character_Chat/test_visual_identity_expression_metadata.pytldw_Server_API/tests/Services/test_lifecycle_worker_catalog.pytldw_Server_API/tests/Services/test_startup_content_jobs_pollers.pytldw_Server_API/tests/Services/test_startup_tail_finalization.pytldw_Server_API/tests/VN_Play/test_vn_play_api.pytldw_Server_API/tests/VN_Play/test_vn_play_assets.pytldw_Server_API/tests/VN_Play/test_vn_play_turns.pytldw_Server_API/tests/Visual_Identities/test_expression_slots.pytldw_Server_API/tests/Visual_Identities/test_visual_identities_api.pytldw_Server_API/tests/Visual_Identities/test_visual_identity_archive_import.pytldw_Server_API/tests/Visual_Identities/test_visual_identity_capabilities.pytldw_Server_API/tests/Visual_Identities/test_visual_identity_db.pytldw_Server_API/tests/Visual_Identities/test_visual_identity_jobs.pytldw_Server_API/tests/Visual_Identities/test_visual_identity_jobs_worker.pytldw_Server_API/tests/Visual_Identities/test_visual_identity_service.pytldw_Server_API/tests/Visual_Identities/test_visual_identity_source_context.pytldw_Server_API/tests/Visual_Identities/test_visual_identity_storage.pytldw_Server_API/tests/Visual_Identities/test_visual_identity_vn_bridge.py
| const visualCharacterAvatar = | ||
| shouldUseCharacterIdentity && typeof props.visualAssetUrl === "string" | ||
| ? props.visualAssetUrl.trim() | ||
| : "" | ||
| const moodCharacterAvatar = shouldUseCharacterIdentity | ||
| ? resolveCharacterMoodImageUrl(props.characterIdentity, resolvedMoodLabel) | ||
| : "" | ||
| const characterAvatar = moodCharacterAvatar || baseCharacterAvatar | ||
| const characterAvatar = | ||
| visualCharacterAvatar || moodCharacterAvatar || baseCharacterAvatar |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
previewUrl/isAnimated mismatched with portraitImage when visual identity asset isn't used.
portraitImage (via resolvedModelImage) can resolve to moodCharacterAvatar, baseCharacterAvatar, or props.modelImage when visualCharacterAvatar is empty — i.e. portraitImage is not necessarily props.visualAssetUrl. But previewUrl/isAnimated passed to VisualIdentityImage are unconditionally sourced from props.visualPreviewUrl/props.visualIsAnimated, which only make sense paired with props.visualAssetUrl.
VisualIdentityImage chooses assetUrl vs previewUrl based on isAnimated and prefers-reduced-motion, then renders an with the provided alt. So if a user prefers reduced motion and
visualIsAnimated happens to be true (stale from the message's original resolution) while portraitImage actually fell back to a mood/base/model avatar, the component will incorrectly swap in the unrelated visualPreviewUrl image instead of the intended static avatar.
Gate previewUrl/isAnimated on whether portraitImage actually is the visual-identity asset.
🐛 Proposed fix
+ const isVisualIdentityPortrait =
+ shouldUseCharacterIdentity &&
+ Boolean(visualCharacterAvatar) &&
+ portraitImage === visualCharacterAvatar
const portraitPanel = shouldShowPortrait ? (
<button
type="button"
onClick={() => setIsAvatarPreviewOpen(true)}
className="relative hidden h-40 w-28 shrink-0 overflow-hidden rounded-2xl border border-border/60 bg-surface/30 shadow-sm transition hover:brightness-110 focus:outline-none focus:ring-2 focus:ring-focus sm:block md:h-52 md:w-36"
aria-label={t("playground:previewCharacterAvatar", {
defaultValue: "Preview character avatar"
}) as string}
>
<VisualIdentityImage
assetUrl={portraitImage}
- previewUrl={props.visualPreviewUrl}
- isAnimated={Boolean(props.visualIsAnimated)}
+ previewUrl={isVisualIdentityPortrait ? props.visualPreviewUrl : undefined}
+ isAnimated={isVisualIdentityPortrait ? Boolean(props.visualIsAnimated) : false}Also applies to: 1928-1939
🤖 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/packages/ui/src/components/Common/Playground/Message.tsx` around lines
850 - 858, `VisualIdentityImage` is receiving `previewUrl` and `isAnimated` from
`props.visualPreviewUrl`/`props.visualIsAnimated` even when `portraitImage` has
fallen back to `moodCharacterAvatar`, `baseCharacterAvatar`, or
`props.modelImage` instead of `visualCharacterAvatar`. Update the `Message`
component’s `portraitImage`/`resolvedModelImage` flow so those props are only
passed when the resolved image is actually the visual identity asset, and
otherwise clear them so the fallback avatar renders correctly.
| const CANONICAL_SLOT_ORDER = [ | ||
| "neutral", | ||
| "happy", | ||
| "excited", | ||
| "sad", | ||
| "angry", | ||
| "thinking", | ||
| "confused", | ||
| "surprised" | ||
| ] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Derive canonical order from the shared expression options instead of duplicating it.
CANONICAL_SLOT_ORDER re-lists the same keys already defined in VISUAL_IDENTITY_EXPRESSION_OPTIONS (utils/visual-identity-expressions.ts). Keeping two independent lists risks them drifting apart if a canonical expression is added/renamed in one place but not the other.
♻️ Proposed fix
-const CANONICAL_SLOT_ORDER = [
- "neutral",
- "happy",
- "excited",
- "sad",
- "angry",
- "thinking",
- "confused",
- "surprised"
-]
+import { VISUAL_IDENTITY_EXPRESSION_OPTIONS } from "`@/utils/visual-identity-expressions`"
+
+const CANONICAL_SLOT_ORDER = VISUAL_IDENTITY_EXPRESSION_OPTIONS.map((option) => option.key)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const CANONICAL_SLOT_ORDER = [ | |
| "neutral", | |
| "happy", | |
| "excited", | |
| "sad", | |
| "angry", | |
| "thinking", | |
| "confused", | |
| "surprised" | |
| ] | |
| import { VISUAL_IDENTITY_EXPRESSION_OPTIONS } from "`@/utils/visual-identity-expressions`" | |
| const CANONICAL_SLOT_ORDER = VISUAL_IDENTITY_EXPRESSION_OPTIONS.map((option) => option.key) |
🤖 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/packages/ui/src/components/Common/VisualIdentity/ExpressionSlotGrid.tsx`
around lines 8 - 17, `CANONICAL_SLOT_ORDER` in `ExpressionSlotGrid` duplicates
the canonical expression keys already defined in
`VISUAL_IDENTITY_EXPRESSION_OPTIONS`, so update the grid to derive its order
from that shared source instead of maintaining a second hardcoded list. Use the
shared expression options export from `utils/visual-identity-expressions.ts` and
map/extract the canonical keys in `ExpressionSlotGrid.tsx`, keeping the same
order but ensuring any future additions or renames stay in sync automatically.
| const slotFromEntry = ( | ||
| slotKey: string, | ||
| fallbackLabel: string, | ||
| canonical: boolean, | ||
| aliases: string[] = [] | ||
| ): ExpressionSlotGridSlot => { | ||
| const entry = isRecord(slotMap[slotKey]) ? slotMap[slotKey] : null | ||
| const expressionKey = stringFromEntry(entry, "expression_key") || slotKey | ||
| const label = | ||
| stringFromEntry(entry, "display_label") || | ||
| fallbackLabel || | ||
| getVisualIdentityExpressionDisplayLabel(expressionKey) || | ||
| expressionKey | ||
| const assetId = assetIdFromEntry(entry) | ||
| const asset = assetId != null ? assetsById.get(assetId) : null | ||
| return { | ||
| key: slotKey, | ||
| label, | ||
| canonical, | ||
| aliases, | ||
| asset: asset ? assetToSlot(asset, buildAssetUrl).asset : null | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Building a full slot object just to discard most of it.
slotFromEntry calls assetToSlot(asset, buildAssetUrl).asset purely to extract the nested asset sub-object, discarding the key/label/canonical fields that assetToSlot also computes. A small helper that returns just the asset shape would avoid the redundant work and clarify intent.
🤖 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/packages/ui/src/components/Common/VisualIdentity/VisualIdentityDraftReview.tsx`
around lines 80 - 102, slotFromEntry is doing redundant work by calling
assetToSlot(asset, buildAssetUrl) only to keep the nested asset field and
discard the rest. Refactor the VisualIdentityDraftReview.tsx logic to use a
helper that returns just the asset shape (or otherwise reuse the already
available asset data) so the code is clearer and avoids building an unused full
slot object; keep the change localized around slotFromEntry, assetToSlot, and
buildAssetUrl.
| const usePrefersReducedMotion = (): boolean => { | ||
| const [prefersReducedMotion, setPrefersReducedMotion] = React.useState(false) | ||
|
|
||
| React.useEffect(() => { | ||
| if (typeof window === "undefined" || typeof window.matchMedia !== "function") { | ||
| return | ||
| } | ||
|
|
||
| const query = window.matchMedia("(prefers-reduced-motion: reduce)") | ||
| setPrefersReducedMotion(query.matches) | ||
|
|
||
| const handleChange = (event: MediaQueryListEvent) => { | ||
| setPrefersReducedMotion(event.matches) | ||
| } | ||
|
|
||
| if (typeof query.addEventListener === "function") { | ||
| query.addEventListener("change", handleChange) | ||
| return () => query.removeEventListener("change", handleChange) | ||
| } | ||
|
|
||
| query.addListener(handleChange) | ||
| return () => query.removeListener(handleChange) | ||
| }, []) | ||
|
|
||
| return prefersReducedMotion | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Reduced-motion preview is applied only after mount, causing a wasted fetch of the animated asset.
prefersReducedMotion starts false and flips via useEffect, so on first render an animated assetUrl is requested even for users who prefer reduced motion, then swapped to previewUrl once the effect runs — defeating the purpose of the preference for the initial load.
⚡ Proposed fix: lazy-initialize from matchMedia
- const [prefersReducedMotion, setPrefersReducedMotion] = React.useState(false)
+ const [prefersReducedMotion, setPrefersReducedMotion] = React.useState(() => {
+ if (typeof window === "undefined" || typeof window.matchMedia !== "function") {
+ return false
+ }
+ return window.matchMedia("(prefers-reduced-motion: reduce)").matches
+ })Also applies to: 49-51
🤖 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/packages/ui/src/components/Common/VisualIdentity/VisualIdentityImage.tsx`
around lines 3 - 28, The reduced-motion state in usePrefersReducedMotion is
initialized to false and only corrected in useEffect, which causes
VisualIdentityImage to request the animated asset on the first render. Update
usePrefersReducedMotion to derive its initial value synchronously from
window.matchMedia when available, while keeping the effect/listener logic for
updates so the first render can choose previewUrl immediately for reduced-motion
users.
| const handleArchiveSelected = async (file: File) => { | ||
| if (typeof client.startVisualIdentityZipImport !== "function") return | ||
| setImporting(true) | ||
| setError(null) | ||
| try { | ||
| const archive = await fileToUploadData(file) | ||
| const started = await client.startVisualIdentityZipImport({ | ||
| archive, | ||
| title: `${actorLabel} expression pack`, | ||
| pack_id: activePack?.id ?? null, | ||
| idempotency_key: makeIdempotencyKey() | ||
| }) | ||
| setStatusMessage(`Import ${started.status}.`) | ||
| if (started.draft_id && typeof client.getVisualIdentityDraft === "function") { | ||
| let latestDraft = await client.getVisualIdentityDraft(started.draft_id) | ||
| if (!isMountedRef.current) return | ||
| setDraft(latestDraft) | ||
| for ( | ||
| let attempt = 0; | ||
| !isTerminalDraftStatus(latestDraft.status) && | ||
| attempt < DRAFT_POLL_ATTEMPTS && | ||
| isMountedRef.current; | ||
| attempt += 1 | ||
| ) { | ||
| await wait(DRAFT_POLL_INTERVAL_MS) | ||
| latestDraft = await client.getVisualIdentityDraft(started.draft_id) | ||
| if (!isMountedRef.current) return | ||
| setDraft(latestDraft) | ||
| if (isTerminalDraftStatus(latestDraft.status)) break | ||
| } | ||
| if (latestDraft.status === "importing" && isMountedRef.current) { | ||
| setStatusMessage("Import is still processing. Refresh the draft to check again.") | ||
| } | ||
| } | ||
| } catch (importError) { | ||
| if (isMountedRef.current) { | ||
| setError(importError instanceof Error ? importError.message : "Failed to import expression ZIP.") | ||
| } | ||
| } finally { | ||
| if (isMountedRef.current) setImporting(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Pre-validate file size/type against fetched capabilities before uploading.
capabilities.upload_max_bytes, archive_max_bytes, and supported_mime_types are already loaded but not used to reject an obviously oversized/unsupported file before making the network call, so users only find out after a full upload round-trip fails server-side.
Also applies to: 263-293
🤖 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/packages/ui/src/components/Common/VisualIdentity/VisualIdentityPackPanel.tsx`
around lines 196 - 237, Pre-validate the selected ZIP in handleArchiveSelected
before calling fileToUploadData/startVisualIdentityZipImport by checking the
fetched capabilities first. Use the existing capabilities fields
(upload_max_bytes, archive_max_bytes, supported_mime_types) to reject
unsupported MIME types and oversized files up front, set an informative error,
and avoid making the network call when the file is obviously invalid. Keep the
validation close to handleArchiveSelected so the same logic can be reused for
the other upload flow mentioned in the diff.
| <!-- SECTION:FINAL_SUMMARY:END --> | ||
|
|
||
| <!-- SECTION:FINAL_SUMMARY:END --> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the duplicate FINAL_SUMMARY:END markers.
The repeated closing tags make the section structure ambiguous and can trip markdown linting/parsing.
♻️ Proposed fix
-<!-- SECTION:FINAL_SUMMARY:END -->
-<!-- SECTION:FINAL_SUMMARY:END -->🤖 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 `@backlog/tasks/task-12090.3` -
Plan-VN-visual-identity-bridge-and-resolver-implementation.md around lines 46 -
48, The FINAL_SUMMARY section has duplicate closing markers, making the document
structure ambiguous. Update the markdown around the FINAL_SUMMARY boundary in
the task document so there is only one `SECTION:FINAL_SUMMARY:END` marker and
the section is closed exactly once.
Source: Linters/SAST tools
| <!-- SECTION:IMPLEMENTATION_NOTES:BEGIN --> | ||
|
|
||
| <!-- SECTION:IMPLEMENTATION_NOTES:END --> | ||
|
|
||
| Task 1 complete: source_context helper added in 4f0da62fae, review feedback fixed in 00b4c40717. Focused pytest passed with 21 tests; source_context.py Bandit scan clean. Spec compliance and code-quality re-review approved. | ||
|
|
||
| Task 2 complete: asset source_context_json DB/schema/create_asset/activation/manifest support added in 7d9a4decc4, review feedback fixed in 6b9b31c57b. Focused pytest passed with 43 tests; VisualIdentity_DB.py Bandit scan clean; raw test-scope Bandit only reports expected pytest B101 asserts. Spec compliance and code-quality re-review approved. | ||
|
|
||
| Task 3 complete: API/schema/idempotency source_context support added in 3d692271f5. Focused API pytest passed with 12 tests; quality review approved with only an advisory invalid-context API hardening test suggestion. Bandit production scope clean; raw test-scope Bandit only reports expected pytest B101 asserts. | ||
|
|
||
| Task 4 complete: VN generated-file provenance bridge and endpoint integration added in c5991c3293, review fixes applied in 3e0fc91643, and final source_feature trust-boundary hardening added in 2b6d62c59c. Focused backend pytest passed with 33 tests; git diff --check clean; Bandit on vn_bridge.py reported zero findings. Spec compliance and code-quality re-review approved. | ||
|
|
||
| Task 5 complete: frontend generated-file import action/types/client contract coverage added in 94f319fec9, with source_context fixture follow-up in cce9082805. Focused frontend Vitest passed with 11 tests for the visual identity client contract and generated-file import helper. Expanded component test run was blocked by the existing local antd dependency resolution issue, but the two reviewed source_context fixture gaps are fixed. Spec compliance and code-quality re-review approved. | ||
|
|
||
| Stage 11A verification: worktree-local `.venv` was absent, so backend verification used the shared project venv at `/Users/macbook-dev/Documents/GitHub/tldw_server2/.venv`. Focused backend pytest passed with 97 tests across source_context, VN bridge, DB, service, and API suites. Focused frontend Vitest passed with 11 tests across the visual identity client contract and generated-file import helper. Bandit Stage 11A backend scope wrote `/tmp/bandit_vn_visual_identity_stage11a.json` with 0 errors and 0 findings. `git diff --check` was clean. Expanded VisualIdentityDraftReview/VisualIdentityPackPanel Vitest remains blocked by the existing local antd dependency resolution issue, not by the source_context fixture changes. | ||
|
|
||
| Final review follow-up: 98ef07b746 fixed three whole-slice review findings. VN imports now always persist derived structural provenance, verify `vn_asset_items.generated_file_id` matches the imported generated-file record, and replay completed VN idempotency responses before live generated-file/VN validation. Red tests failed for the three expected gaps before implementation; focused fix tests passed with 36 tests. Final whole-slice review approved. Fresh final verification passed with 100 backend tests, 11 frontend tests, `git diff --check` clean, and `/tmp/bandit_vn_visual_identity_stage11a_final.json` reporting 0 errors and 0 findings. | ||
| <!-- SECTION:NOTES:END --> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Move the notes back inside the IMPLEMENTATION_NOTES block.
The current markers close that section before the actual task notes, so the recorded verification details are not captured by the structured field.
🤖 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 `@backlog/tasks/task-12090.4` -
Implement-Stage-11A-VN-visual-identity-generated-file-import-bridge.md around
lines 37 - 54, The task notes are currently outside the IMPLEMENTATION_NOTES
section because the SECTION:IMPLEMENTATION_NOTES block closes before the
verification text. Move the existing task/status content back inside the
IMPLEMENTATION_NOTES markers in this markdown file, keeping the notes under that
section and leaving the rest of the document unchanged.
| <!-- SECTION:FINAL_SUMMARY:END --> | ||
|
|
||
| <!-- SECTION:FINAL_SUMMARY:END --> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the extra FINAL_SUMMARY:END markers.
These repeated closers are redundant and make the document structure harder to parse.
♻️ Proposed fix
-<!-- SECTION:FINAL_SUMMARY:END -->
-<!-- SECTION:FINAL_SUMMARY:END -->🤖 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 `@backlog/tasks/task-12090.5` -
Implement-Stage-11B-VN-visual-identity-role-and-casting-resolver.md around lines
52 - 54, The document contains a duplicated FINAL_SUMMARY closer, so remove the
extra SECTION:FINAL_SUMMARY:END marker and keep only a single closing marker for
the final summary section; locate the repeated markers near the end of the
markdown task doc and clean up the redundant one so the section structure is
unambiguous.
Source: Linters/SAST tools
| ## Implementation Notes | ||
|
|
||
| <!-- SECTION:IMPLEMENTATION_NOTES:BEGIN --> | ||
| Task 9 complete. Added frontend resolver request/response fields for VN role/casting override parameters, serialized them through the Visual Identity API client, and included hook role/override options in the resolver cache key and client request. Added focused tests for role override query serialization and override-pack cache separation. Verification: red run failed on the missing query/cache behavior; green run `bunx vitest run src/services/__tests__/tldw-api-client.visual-identities.test.ts src/hooks/__tests__/useVisualIdentityResolver.test.tsx` passed 13 tests. TypeScript diagnostics were attempted with the standard command and hit Node heap OOM, then rerun with `NODE_OPTIONS=--max-old-space-size=8192`; full package diagnostics still fail on existing baseline issues, mainly missing `antd` types, and a filtered rerun showed no diagnostics for the touched Task 9 files. `git diff --check` passed. | ||
| Task 9A complete. Added a non-persistent VN Play Visual Identity casting adapter for sprite directives carrying actor_kind/actor_id, role metadata, expression, and optional override pack/version controls. VN Play now routes those directives through VisualIdentityService.resolve_expression_asset, converts resolved assets into renderable sprite items with Visual Identity content URLs and metadata, and preserves embedded Visual Identity sprites during scene-state enrichment without mutating actor bindings. Added focused asset resolver tests plus a VN Play turn/API regression for active_sprites. Verification: red run failed on missing resolve_visual_identity_directive; green run `python -m pytest tldw_Server_API/tests/VN_Play/test_vn_play_assets.py tldw_Server_API/tests/VN_Play/test_vn_play_api.py::test_session_response_includes_visual_identity_cast_sprite -q` passed 8 tests; compileall on VN_Play/assets.py and VN_Play/service.py passed; Bandit report /tmp/bandit_task12090_5_vn_play_casting_adapter.json has errors [] and results []; git diff --check passed. | ||
| Task 10 final verification complete. Backend focused verification passed: `python -m pytest tldw_Server_API/tests/Visual_Identities/test_visual_identity_source_context.py tldw_Server_API/tests/Visual_Identities/test_visual_identity_vn_bridge.py tldw_Server_API/tests/Visual_Identities/test_visual_identity_db.py tldw_Server_API/tests/Visual_Identities/test_visual_identity_service.py tldw_Server_API/tests/Visual_Identities/test_visual_identities_api.py tldw_Server_API/tests/VN_Play/test_vn_play_assets.py tldw_Server_API/tests/VN_Play/test_vn_play_api.py::test_session_response_includes_visual_identity_cast_sprite -q` passed 124 tests with 15 warnings. Frontend focused verification passed: `bunx vitest run src/services/__tests__/tldw-api-client.visual-identities.test.ts src/components/Common/VisualIdentity/__tests__/useGeneratedFileImportAction.test.ts src/hooks/__tests__/useVisualIdentityResolver.test.tsx` passed 16 tests. Full frontend TypeScript diagnostics still fail on existing package baseline issues, primarily missing `antd` types and unrelated scheduled-task/background/voice-cloning diagnostics; the filtered rerun for touched Stage 11 frontend files produced no diagnostics. Bandit touched backend scope report `/tmp/bandit_vn_visual_identity_stage11.json` has errors [] and results []. Final status review showed only plan/Backlog metadata plus unrelated untracked watchlist template files not touched by this task. | ||
| <!-- SECTION:IMPLEMENTATION_NOTES:END --> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fold this into the earlier Implementation Notes section.
The second ## Implementation Notes heading duplicates the first one and will trip markdownlint MD024; if these are follow-up notes, append them to the existing block or rename the heading.
🧰 Tools
🪛 LanguageTool
[grammar] ~71-~71: Use a hyphen to join words.
Context: ...sed 124 tests with 15 warnings. Frontend focused verification passed: `bunx vites...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.22.1)
[warning] 66-66: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 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 `@backlog/tasks/task-12090.5` -
Implement-Stage-11B-VN-visual-identity-role-and-casting-resolver.md around lines
66 - 72, The implementation notes are duplicated, which will trigger
markdownlint MD024; fold the new Task 10 text into the existing Implementation
Notes block instead of creating a second same-level heading. Update the content
around the existing Implementation Notes section in this task document, keeping
one `## Implementation Notes` heading and appending the follow-up notes there,
or rename the later heading if a separate section is truly needed.
Source: Linters/SAST tools
| <!-- SECTION:DESCRIPTION:BEGIN --> | ||
| <!-- SECTION:DESCRIPTION:BEGIN --> | ||
| <!-- SECTION:DESCRIPTION:END --> | ||
|
|
||
| <!-- SECTION:DESCRIPTION:END --> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the duplicated section fences.
SECTION:DESCRIPTION is nested twice, and SECTION:FINAL_SUMMARY:END appears twice. That will break any parser that relies on these markers; please keep one begin/end pair per section.
Also applies to: 59-71
🤖 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 `@backlog/tasks/task-12098` -
Implement-visual-identity-pack-management-and-draft-review-UI.md around lines 23
- 27, The document has duplicated section fences in the markdown sections, so
the parser will see nested BEGIN/END markers. Clean up the affected sections by
keeping only one matching begin/end pair per section, especially the
SECTION:DESCRIPTION block and the duplicated SECTION:FINAL_SUMMARY:END markers,
and verify the same fix in the other referenced section. Use the section marker
names to locate and remove the extra fences without changing the section
content.
Change summary
Why
Test plan
Summary by cubic
Adds shared Visual Identity expression packs for character/persona chat and VN, with strict backend validation, idempotent import/activation, and a lightweight management UI. Integrates a VN role/casting resolver with override/fallback semantics and renders expression sprites in chat without breaking legacy mood images.
/api/v1/visual-identitiesfor capabilities, slots, packs/drafts/assets, ZIP import jobs, activation, binding resolution, and asset content; persisted viaVisualIdentity_DBwith size/format/frame checks (animated GIF/WebP; AVIF gated) and boundedsource_contextprovenance; hardened idempotency for imports/activations.ExpressionPickerand/emote <expression>; reduced-motion preview handling.VisualIdentityPackPanel(with draft review grid/uploader),ExpressionPicker,VisualIdentityStage,VisualIdentityImage,useGeneratedFileImportAction, resolver hookuseVisualIdentityResolver, expression utilities, and API domainvisual-identities; integrated panels in Character and Persona editors.Written for commit 0d4328e. Summary will update on new commits.