feat(studio): Add centralized asset manifest error handling #87#88
feat(studio): Add centralized asset manifest error handling #87#88zenchantlive wants to merge 2 commits intomainfrom
Conversation
Story of Collaboration: User requested Part 4 of Issue #87: adding error states for manifest fetch failures. I initially planned to add local state to PreviewTab, but after critiquing the architecture, we decided to centralize manifest fetching in the StudioProvider. This ensures that AssetTab, PreviewTab, and the StudioHeader all stay in sync and share the same loading/error states. Decisions Made: - Lifted assetManifest, manifestLoading, and manifestError states to StudioProvider. - Centralized fetchAssets API call in the provider to avoid redundant requests. - Added explicit Error and Empty state UI to PreviewTab with retry capability. - Documented the architecture shift in ADR-027. Challenges: - Handling the race condition where multiple tabs might try to fetch assets on mount—resolved by centralizing ownership in the Provider. - Coordinating asset counts between the Header and the Assets tab—resolved by using the shared manifest state.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
🤖 Hi @zenchantlive, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
📝 WalkthroughWalkthroughCentralizes asset-manifest state in StudioProvider, exposes fetchAssets and manifest state via StudioContext, updates AssetsTab/PreviewTab to consume it, and adds robust iframe script-load tracking/error reporting (generateScriptTagsWithErrorHandling, ScriptLoadInfo) plus assorted new skill documentation files and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Studio UI
participant Provider as StudioProvider (Context)
participant API as Asset API
participant AssetsTab as AssetsTab / PreviewTab
UI->>Provider: mount / useStudio()
Provider->>API: fetchAssets(gameId)
API-->>Provider: assetManifest
Provider-->>AssetsTab: update context (assetManifest, manifestLoading=false)
AssetsTab-->>UI: render counts / show assets or empty message
sequenceDiagram
participant Preview as PreviewFrame (iframe)
participant Parent as Studio UI (parent window)
participant Libraries as Script loader (in iframe)
Preview->>Libraries: insert scripts (with onload/onerror)
Libraries-->>Preview: onload / onerror (per-script)
Libraries-->>Parent: postMessage { type: "script-error", script }
Parent->>Parent: map to ErrorInfo(kind: 'script', script: ...)
Parent-->>UI: show script error overlay, suppress user code execution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
🤖 I'm sorry @zenchantlive, but I was unable to process your request. Please see the logs for more details. |
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Summary of ChangesHello @zenchantlive, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors how asset manifests are managed and displayed within the Studio. By centralizing the asset manifest state and fetching logic in the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR addresses part 4 of Issue #87 by implementing centralized asset manifest error handling in the Hatch Studios preview system. The change lifts asset manifest fetching from individual components (PreviewTab, AssetsTab) to the StudioProvider, eliminating redundant API calls and ensuring consistent state management across the UI.
Changes:
- Centralized asset manifest state management in StudioProvider
- Added explicit error and empty state UI to PreviewTab with retry capability
- Enhanced script loading error detection in PreviewFrame
- Added comprehensive debug logging to asset-loader
- Documented the architectural decision in ADR-027
Reviewed changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/studio/StudioProvider.tsx | Added centralized asset manifest state (assetManifest, manifestLoading, manifestError) and fetchAssets callback |
| src/lib/studio/context.ts | Extended StudioContextValue interface with asset manifest fields |
| src/components/studio/tabs/PreviewTab.tsx | Refactored to use shared state, added error/empty UI with retry |
| src/components/studio/tabs/AssetsTab.tsx | Optimized to use shared manifest instead of fetching independently |
| src/components/studio/PreviewFrame.tsx | Enhanced with script load error detection and handling |
| src/lib/studio/preview-libraries.ts | Added comprehensive script loading tracking and error handling functions |
| src/lib/studio/asset-loader.ts | Added optional debug logging throughout asset loading pipeline |
| src/memory/adr/027-centralized-asset-manifest-handling.md | New ADR documenting the architectural decision and trade-offs |
| src/memory/system_patterns.md | Updated with centralized asset manifest pattern |
| src/memory/active_state.md | Documented current session work |
| docs/rca/issue-87.md | Comprehensive root cause analysis for the black screen issue |
| src/tests/regression/critical_suite.test.ts | New critical regression test suite |
| src/lib/plan-parser.test.ts | Added edge case and failure path tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||||||||
…ling #87 Story of Collaboration: We implemented a comprehensive suite of fixes for Issue #87 to make the Hatch Studios preview environment more resilient and transparent. This involved adding script loading detection to the iframe, implementing verbose debug logging for the asset loader, refactoring script metadata, and centralizing the asset manifest state. Decisions Made: - [Part 1] Added script load tracking to PreviewFrame.tsx to catch CDN failures before user code executes. - [Part 2] Implemented a verbose 'debug' mode in asset-loader.ts to provide granular visibility into the asset resolution lifecycle. - [Part 3] Refactored preview-libraries.ts to use detailed metadata for script status tracking and automated check-scripts. - [Part 4] Centralized the asset manifest in StudioProvider to eliminate redundant API calls across separate tabs and ensure UI consistency. - Added explicit Error and Empty state UI to PreviewTab with retry functionality. - Documented the architectural shift in ADR-027. Challenges: - Coordinating script load signals from a sandboxed iframe back to the parent UI—resolved via postMessage and a global __scriptStatus tracker. - Managing manifest fetching across multiple tabs to avoid race conditions—resolved by lifting ownership to the primary StudioProvider context.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/lib/studio/preview-libraries.ts (1)
139-139: Minor: extra leading space in string.There's an extra space before "Smoke effects".
📝 Suggested fix
- ' Smoke effects', + 'Smoke effects',src/components/studio/PreviewFrame.tsx (1)
120-120: Remove debug console.log statement.This appears to be leftover debug code. The comment suggests uncertainty about its purpose.
🧹 Suggested fix
- console.log(isPlaying); // Use isPlaying to satisfy lint if needed, or remove from props if truly unusedIf
isPlayingis truly unused, consider removing it from the props interface or prefixing with underscore to indicate intentional non-use:export function PreviewFrame({ files, assetManifest, gameId, - isPlaying, + isPlaying: _isPlaying, // Reserved for future play/pause functionality onReady,src/components/studio/tabs/PreviewTab.tsx (1)
21-141: Add line-by-line reasoning comments for new manifest UI logic.
The new manifest state usage and UI blocks (Lines 21-141) lack the required per-line reasoning comments. Please annotate the added blocks (state destructuring, loading/error/empty banners, and registry toggle) with brief intent comments.✍️ Example (apply consistently to the new logic)
- const { + // Pull shared manifest state so preview stays aligned with provider. + const { files, isPlaying, previewKey, requestErrorFix, game, assetManifest, manifestLoading, manifestError, fetchAssets } = useStudio(); @@ - {/* Manifest error state */} + {/* Manifest error state */} + // Surface manifest fetch failures and offer a retry action. {manifestError && ( @@ - {/* Empty manifest info state (successful fetch but no assets) */} + {/* Empty manifest info state (successful fetch but no assets) */} + // Explain empty asset registry to reduce user confusion. {!manifestError && !manifestLoading && assetManifest && Object.keys(assetManifest.assets).length === 0 && (src/components/studio/tabs/AssetsTab.tsx (1)
10-29: Add line-by-line reasoning comments for the new state/ctx wiring.The TS/TSX guidelines require inline, line-by-line reasoning comments for new logic; these additions currently lack them.
As per coding guidelines, please add line-by-line comments explaining reasoning.
src/components/studio/StudioProvider.tsx (1)
10-66: Add line-by-line reasoning comments for the new manifest state.The TS/TSX guidelines require inline reasoning comments for new logic; these additions currently lack them.
As per coding guidelines, please add line-by-line comments explaining reasoning.
🤖 Fix all issues with AI agents
In @.claude/skills/dispatching-parallel-agents/SKILL.md:
- Around line 34-43: Replace the bold labels "**Use when:**" and "**Don't use
when:**" with proper Markdown headings (e.g., "## Use when" and "## Don't use
when") so markdownlint doesn't flag emphasis-as-heading; keep the following
bullet lists unchanged under each new heading and ensure spacing (blank line)
before each heading; update the occurrences in SKILL.md where the exact strings
"**Use when:**" and "**Don't use when:**" appear.
In @.claude/skills/finishing-a-development-branch/SKILL.md:
- Line 106: Update the documentation so Option 2 (Create PR) consistently
instructs to keep the worktree instead of cleaning it up: replace the “Then:
Cleanup worktree (Step 5)” wording under Option 2 (referenced as the phrase
"Cleanup worktree (Step 5)" and the heading "Option 2: Create PR") with a clear
"Keep the worktree" note (or similar) and adjust the Quick Reference table,
Common Mistakes, and Red Flags sections that currently conflict so all mention
that Option 2 keeps the worktree to allow addressing PR feedback.
In @.claude/skills/receiving-code-review/SKILL.md:
- Around line 16-25: All fenced code blocks in SKILL.md are missing language
identifiers and trigger markdownlint; update every triple-backtick fence to
include a language token (e.g., text or markdown). Locate the unlabeled fences
by searching for the exact snippets shown in the diff such as the block starting
with "WHEN receiving code review feedback:", the block beginning "IF any item is
unclear:", the block with "your human partner: \"Fix 1-6\"", and the block
starting "BEFORE implementing: 1. Check: Technically correct for THIS
codebase?", and add consistent language tags (e.g., ```text or ```markdown) to
each corresponding opening fence across the other reported ranges (42-48, 51-57,
68-84, 90-96, 102-111, 134-144, 153-160, 179-182, 185-188, 191-194, 197-201) so
markdownlint passes.
In @.claude/skills/subagent-driven-development/implementer-prompt.md:
- Around line 5-78: The markdown fenced code block under "Task tool
(general-purpose):" (the prompt: | block) is missing a language identifier which
triggers markdownlint MD040; update the opening fence to include a language
(e.g., change ``` to ```text) so the block becomes a labelled code fence and
keep the closing ``` as-is; locate the block by the "Task tool
(general-purpose):" heading and the prompt: | field and apply the single-line
fence change.
In @.claude/skills/systematic-debugging/find-polluter.sh:
- Around line 21-23: The script currently ignores the TEST_PATTERN argument:
replace the hardcoded glob in the find invocation that sets TEST_FILES with use
of the TEST_PATTERN variable (or fall back to the existing "*.test.ts" and
"*.test.tsx" defaults) so the find command respects the provided TEST_PATTERN;
update the TEST_FILES assignment (and any related TOTAL calculation) to use
TEST_PATTERN (or a computed pattern variable) instead of the fixed patterns so
the script's documented usage works as expected.
- Line 29: The loop "for TEST_FILE in $TEST_FILES" splits on IFS and breaks on
filenames with spaces; replace it with a safe read loop that preserves
whitespace by using a while read -r pattern (e.g.: replace the for line with a
construct that reads from TEST_FILES via a here-string or pipe: while IFS= read
-r TEST_FILE; do ... done <<<"$TEST_FILES" so TEST_FILE and TEST_FILES are used
unchanged), or if TEST_FILES is NUL-separated use while IFS= read -r -d ''
TEST_FILE; do ... done <<<"$TEST_FILES"; update the loop surrounding code
accordingly.
In @.claude/skills/systematic-debugging/root-cause-tracing.md:
- Around line 35-37: The fenced code block containing "Error: git init failed in
/Users/jesse/project/packages/core" is missing a language identifier; update the
triple-backtick fence to specify a language (use text) so the block becomes
```text ... ``` to satisfy markdownlint and improve rendering.
In @.claude/skills/using-git-worktrees/SKILL.md:
- Around line 57-61: The current ignore check tests both ".worktrees" and
"worktrees" which can pass incorrectly for the wrong path; instead, after you
determine the actual directory to use (the variable referenced as CHOSEN_DIR in
your script), run a single git check-ignore -q against "$CHOSEN_DIR" only,
replacing the existing dual-check with this single check and ensuring it
executes after directory selection so the check validates the specific chosen
path (.worktrees or worktrees) rather than both.
In @.claude/skills/writing-skills/examples/CLAUDE_MD_TESTING.md:
- Around line 148-150: The nested bullets under the numbered item "3. **Pressure
test** - Add time/sunk cost/authority" are under‑indented and fail MD007; fix by
indenting each nested dash line (the lines starting with "- Does agent still
check under pressure?" and "- Document when compliance breaks down") by 4 spaces
so they are properly nested under the "3." list item and re-run markdownlint to
confirm MD007 is satisfied.
In @.claude/skills/writing-skills/render-graphs.js:
- Around line 110-118: Summary: The platform-specific check using
execSync('which dot') fails on Windows; replace it with a cross-platform probe.
Update the check around execSync('which dot') to detect process.platform ===
'win32' and use the Windows equivalent ('where dot') or, better, attempt to run
the 'dot' binary directly (e.g., execSync or spawnSync 'dot -V') and catch
errors; then log the same install guidance and call process.exit(1) on failure.
Ensure you update the same block that currently calls execSync('which dot') and
the surrounding try/catch so the check works on both Unix and Windows.
In @.claude/skills/writing-skills/SKILL.md:
- Line 598: The document uses the term "TodoWrite" with no definition; update
SKILL.md to either add a brief definition of TodoWrite (what it is: a
command/skill/tool and expected usage) near its first occurrence and/or replace
the bare reference with a hyperlink to the TodoWrite documentation or README,
and include a short usage example or the command signature so readers know how
to invoke it; specifically modify the section around the "TodoWrite" reference
and add an entry to the glossary or a footnote that defines TodoWrite and points
to its official docs or repository.
In @.claude/skills/writing-skills/testing-skills-with-subagents.md:
- Line 327: Edit the checklist item in the markdown where the text reads "- [ ]
Updated description ith violation symptoms" and correct the typo "ith" to "with"
so the line becomes "- [ ] Updated description with violation symptoms"; locate
this string in .claude/skills/writing-skills/testing-skills-with-subagents.md
and update the checkbox line accordingly.
In @.openhands/skills/unit-regression-test-agent/SKILL.md:
- Around line 158-160: The example test call uses Bun's wrong parameter order
and an invalid option; update the example in SKILL.md to use Bun's correct
signature by making the test callback the second argument and the options object
(with a supported field like timeout, retry, or repeats) the third
argument—e.g., replace the incorrect test('login flow', { critical: true }, ()
=> { ... }) with test('login flow', () => { ... }, { timeout: 5000 }) or
similar, and ensure any other examples use the same order and only supported
TestOptions fields.
In `@CLAUDE.md`:
- Around line 81-86: Fix typos and capitalization in the "Skills and Agents"
section and the table row: correct the table header "| Github Querey |
**Airweave** |" to "Github Query" or appropriate casing, fix "skills.md"
wording/capitalization to "skills.md" or "Skills.md" consistently, change "an
script" to "a script", "reccomend" to "recommend", "parralel" to "parallel",
"prrompts" to "prompts", and "subagent driven development and dispatching
paralel agent skills" to "subagent-driven development and dispatching parallel
agent skills"; ensure the heading "Skills and Agents" uses proper capitalization
and punctuation consistently throughout.
In `@src/lib/studio/context.ts`:
- Around line 48-52: Add brief inline reasoning comments for each new manifest
field in the context definition: annotate assetManifest with its purpose (stores
parsed AssetManifest or null when not loaded), manifestLoading to indicate
active fetch state (true while fetchAssets is running), manifestError to capture
a user- or network-facing error message or null, and fetchAssets to describe its
responsibility (async action to load/update assetManifest and set loading/error
states). Place each comment directly above or to the right of the corresponding
symbol (assetManifest, manifestLoading, manifestError, fetchAssets) in the
context type so they are visible line-by-line.
In `@src/memory/adr/027-centralized-asset-manifest-handling.md`:
- Around line 76-79: In the "References" section replace the absolute file://
Windows paths pointing to Implementation Plan and StudioProvider.tsx with
repo-relative paths (or remove if not checked in); locate the references to
"Implementation Plan" and "StudioProvider.tsx" in
src/memory/adr/027-centralized-asset-manifest-handling.md and change the links
to their repository-relative locations (e.g., docs/implementation_plan.md or
src/components/studio/StudioProvider.tsx) so they work for collaborators and CI.
In `@src/memory/system_patterns.md`:
- Around line 461-469: Update the markdown list under the "Client-Side Image
Processing" heading so nested list items use 2-space indentation instead of
4-space indentation to satisfy markdownlint MD007; specifically adjust the
nested bullets for "Why:", its subpoints, "Example:" and "Gotcha:" (which
reference src/lib/image-processing.ts and the blendSeams function) to use
two-space indents so the list is properly rendered and passes the linter.
In `@src/tests/regression/critical_suite.test.ts`:
- Around line 12-34: Add line-by-line comments to the test named "successfully
parses a standard asset plan markdown" to explain setup, reasoning, and
invariants: annotate the markdown fixture and why it represents composite mode;
note the parsePlan call (parsePlan(markdown, { mode: 'composite', projectId:
'reg-test-1' })) and that it should produce individual ParsedAsset entries for
variants; comment each assertion (expect(assets).toBeDefined(),
expect(assets.length).toBe(5), and the three find(...) assertions) explaining
what specific invariant it verifies (count of assets, variant expansion into
separate entries, and expected type mapping for background/ui-element) and why
those invariants matter for the critical regression suite; keep comments concise
and adjacent to the corresponding lines so future readers can quickly understand
the test intent and the role of assets array/indices.
🧹 Nitpick comments (22)
.claude/skills/using-superpowers/SKILL.md (2)
1-4: Consider shortening the YAML description.The description field in the YAML front matter is quite long (149 characters). YAML front matter descriptions typically should be concise. Consider shortening to focus on the essential purpose.
📝 Suggested shorter description
--- name: using-superpowers -description: Use when starting any conversation - establishes how to find and use skills, requiring Skill tool invocation before ANY response including clarifying questions +description: Establishes skill discovery and mandatory invocation before any response ---
87-87: Add trailing newline at end of file.Markdown files conventionally end with a newline character. While not functionally required, this follows standard text file conventions and prevents some tools from showing warnings.
.claude/skills/systematic-debugging/SKILL.md (2)
18-20: Optional: Add language specifier to code block.The fenced code block for "The Iron Law" could specify a language for better syntax highlighting, though the current plain text rendering is acceptable for emphasis.
♻️ Optional improvement
-``` +```text NO FIXES WITHOUT ROOT CAUSE INVESTIGATION FIRST</details> --- `77-87`: **Optional: Add language specifier to code block.** The multi-component system instrumentation example could use `text` or `bash` as a language specifier per markdown linting best practices. <details> <summary>♻️ Optional improvement</summary> ```diff - ``` + ```text For EACH component boundary:.claude/skills/test-driven-development/SKILL.md (2)
33-35: Optional: Add language specifier to code block.Per markdown best practices, the "Iron Law" code block should specify a language (e.g.,
text).♻️ Optional improvement
-``` +```text NO PRODUCTION CODE WITHOUT A FAILING TEST FIRST</details> --- `366-368`: **Optional: Add language specifier to code block.** The final rule code block should specify a language for consistency. <details> <summary>♻️ Optional improvement</summary> ```diff -``` +```text Production code → test exists and failed first Otherwise → not TDD</details> </blockquote></details> <details> <summary>src/lib/plan-parser.test.ts (1)</summary><blockquote> `123-167`: **Add line-by-line reasoning comments for the new tests.** The new test block doesn’t include the required per-line reasoning comments for `src/**/*.ts` files. Please annotate the added lines to match the repository rule. As per coding guidelines, ... </blockquote></details> <details> <summary>.claude/skills/systematic-debugging/find-polluter.sh (1)</summary><blockquote> `32-37`: **Consider failing fast when pollution pre-exists.** If pollution already exists before running any tests, the script enters an infinite skip loop without cleaning up or failing. This could mask issues where the environment wasn't clean to begin with. Consider either: 1. Failing immediately if pollution exists at script start, or 2. Cleaning up the pollution before starting the bisection <details> <summary>💡 Optional: Add upfront check</summary> ```diff echo "Found $TOTAL test files" echo "" +# Fail fast if pollution already exists +if [ -e "$POLLUTION_CHECK" ]; then + echo "❌ Pollution already exists before running any tests!" + echo " Please clean up '$POLLUTION_CHECK' and re-run." + exit 1 +fi + COUNT=0 for TEST_FILE in $TEST_FILES; do.claude/skills/verification-before-completion/SKILL.md (1)
18-21: Add language specifiers to fenced code blocks for better rendering.Multiple code blocks lack language specifiers (lines 18, 26, 79, 85, 91, 97, 103). While these are pseudo-code examples, adding
textorplaintextwill satisfy linting and improve rendering consistency.💡 Example fix
-``` +```text NO COMPLETION CLAIMS WITHOUT FRESH VERIFICATION EVIDENCEApply similar changes to other unlabeled code blocks. </details> </blockquote></details> <details> <summary>.claude/skills/test-driven-development/testing-anti-patterns.md (1)</summary><blockquote> `15-19`: **Add language specifiers to pseudo-code blocks.** Several code blocks (lines 15, 53, 104, 153, 212, 231, 243) lack language specifiers. Use `text` or `plaintext` for the rule/gate function blocks to satisfy linting. <details> <summary>💡 Example fix</summary> ```diff -``` +```text 1. NEVER test mock behavior 2. NEVER add test-only methods to production classes 3. NEVER mock without understanding dependencies</details> </blockquote></details> <details> <summary>.claude/skills/using-git-worktrees/SKILL.md (1)</summary><blockquote> `42-49`: **Add language specifiers to unlabeled code blocks.** Code blocks at lines 42, 138, and 180 lack language specifiers. Use `text` for the prompt/output examples to satisfy linting. Also applies to: 138-142, 180-192 </blockquote></details> <details> <summary>src/lib/studio/asset-loader.ts (1)</summary><blockquote> `57-65`: **Debug helper implementation looks good.** The `debugLog` helper is well-designed with proper gating behind the debug flag, consistent stage-based logging format, and safe handling of optional data parameters. However, per coding guidelines for `src/**/*.{ts,tsx}`, line-by-line comments explaining reasoning should be added. Consider adding brief comments explaining the purpose of this helper. <details> <summary>📝 Optional: Add explanatory comments</summary> ```diff + // Debug logging helper - outputs structured logs only when debug mode is enabled + // Stage parameter categorizes the log (init, resolve, load, etc.) for filtering function debugLog(stage, message, data) { if (!ASSET_CONFIG.debug) return; if (data !== undefined) { console.log('[ASSETS:DEBUG] [' + stage + '] ' + message, data); } else { console.log('[ASSETS:DEBUG] [' + stage + '] ' + message); } }src/tests/regression/critical_suite.test.ts (1)
44-47: Consider defensive assertions for clearer test failures.Accessing
assets[0].mobility.typedirectly will throw aTypeErrorifmobilityis undefined, resulting in an unclear test failure. Consider adding intermediate assertions or using optional chaining with explicit checks.💡 Suggested improvement for clearer test failures
expect(assets.length).toBe(2) - expect(assets[0].mobility.type).toBe('moveable') - expect(assets[1].mobility.directions).toBe(4) + // Verify mobility metadata is parsed correctly + expect(assets[0].mobility).toBeDefined() + expect(assets[0].mobility.type).toBe('moveable') + expect(assets[1].mobility).toBeDefined() + expect(assets[1].mobility.directions).toBe(4).claude/skills/brainstorming/SKILL.md (1)
19-19: Minor grammar fix: hyphenate "multiple-choice".As per the static analysis hint, "multiple choice" should be hyphenated when used as a compound adjective.
📝 Suggested fix
-- Prefer multiple choice questions when possible, but open-ended is fine too +- Prefer multiple-choice questions when possible, but open-ended is fine too.claude/skills/systematic-debugging/condition-based-waiting-example.ts (1)
26-43: Add inline comments explaining the polling logic.Per coding guidelines for
**/*.{ts,tsx}andsrc/**/*.{js,jsx,ts,tsx}, line-by-line comments explaining reasoning should be added. The implementation is correct, but the code would benefit from brief inline comments.📝 Suggested comments
export function waitForEvent( threadManager: ThreadManager, threadId: string, eventType: LaceEventType, timeoutMs = 5000 ): Promise<LaceEvent> { return new Promise((resolve, reject) => { const startTime = Date.now(); const check = () => { + // Query current events from thread manager const events = threadManager.getEvents(threadId); + // Look for first event matching the target type const event = events.find((e) => e.type === eventType); if (event) { + // Found matching event - resolve immediately resolve(event); } else if (Date.now() - startTime > timeoutMs) { + // Timeout exceeded - reject with descriptive error reject(new Error(`Timeout waiting for ${eventType} event after ${timeoutMs}ms`)); } else { + // Not found yet and time remains - schedule next poll setTimeout(check, 10); // Poll every 10ms for efficiency } }; + // Start polling immediately check(); }); }src/lib/studio/preview-libraries.ts (1)
413-417: Consider using a parameterized origin instead of hardcoded'*'.
generateScriptTagsWithErrorHandlingaccepts aparentOriginparameter butgenerateLibraryCheckScripthardcodes'*'for postMessage. For consistency and security, consider adding an optionalparentOriginparameter here as well.♻️ Suggested fix
-export function generateLibraryCheckScript(): string { +export function generateLibraryCheckScript(parentOrigin: string = '*'): string { // ... - }, '*'); + }, '${parentOrigin}'); // ... and similarly for the second postMessage - }, '*'); + }, '${parentOrigin}');.claude/skills/personas/skill.md (1)
150-196: Add a language identifier to the fenced block.
MD040 flags the code fence without a language. Considermarkdownhere.♻️ Proposed fix
-```markdown +```markdown # Persona Roster (Persistent) ... -```` +```.claude/skills/requesting-code-review/SKILL.md (1)
51-75: Specify a language for the example fence.
MD040 flags the unlabeled fence. Usetextormarkdownhere.♻️ Proposed fix
-``` +```text [Just completed Task 2: Add verification function] ... [Continue to Task 3] -``` +```.claude/skills/subagent-driven-development/spec-reviewer-prompt.md (1)
7-61: Add a language identifier to the fenced block.
This resolves MD040 for the template block.♻️ Proposed fix
-``` +```text Task tool (general-purpose): description: "Review spec compliance for Task N" prompt: | You are reviewing whether an implementation matches its specification. ... -``` +```.claude/skills/requesting-code-review/code-reviewer.md (1)
112-146: Add a language tag to the example output fence.
MD040 is triggered by the unlabeled code block.♻️ Proposed fix
-``` +```text ### Strengths - Clean database schema with proper migrations (db.ts:15-42) ... **Reasoning:** Core implementation is solid with good architecture and tests. Important issues (help text, date validation) are easily fixed and don't affect core functionality. -``` +```.claude/skills/subagent-driven-development/code-quality-reviewer-prompt.md (1)
9-18: Add a language identifier to the fenced block.
This satisfies MD040 for the template snippet.♻️ Proposed fix
-``` +```text Task tool (superpowers:code-reviewer): Use template at requesting-code-review/code-reviewer.md ... DESCRIPTION: [task summary] -``` +```src/components/studio/StudioProvider.tsx (1)
97-126: Guard against stale manifest responses on rapid game switches/unmounts.A slower, earlier request can overwrite newer state. Consider a fetch-id guard (or abort) to ensure only the latest request wins.
♻️ Proposed guard using a fetch-id ref
@@ - const activityIdRef = useRef(0); + const activityIdRef = useRef(0); + const manifestFetchIdRef = useRef(0); @@ - const fetchAssets = useCallback(async () => { + const fetchAssets = useCallback(async () => { if (!game?.id) return; + const fetchId = ++manifestFetchIdRef.current; setManifestLoading(true); setManifestError(null); @@ - const manifest = await response.json(); - setAssetManifest(manifest); + const manifest = await response.json(); + if (fetchId !== manifestFetchIdRef.current) return; + setAssetManifest(manifest); } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Failed to fetch asset manifest'; console.warn('[StudioProvider] Failed to fetch asset manifest:', error); - setManifestError(errorMessage); + if (fetchId !== manifestFetchIdRef.current) return; + setManifestError(errorMessage); } finally { - setManifestLoading(false); + if (fetchId === manifestFetchIdRef.current) { + setManifestLoading(false); + } } }, [game?.id]);
| **Use when:** | ||
| - 3+ test files failing with different root causes | ||
| - Multiple subsystems broken independently | ||
| - Each problem can be understood without context from others | ||
| - No shared state between investigations | ||
|
|
||
| **Don't use when:** | ||
| - Failures are related (fix one might fix others) | ||
| - Need to understand full system state | ||
| - Agents would interfere with each other |
There was a problem hiding this comment.
Use headings instead of bold labels for sections.
markdownlint flags emphasis used as headings. Convert these to proper headings.
🔧 Suggested fix
-**Use when:**
+### Use when
@@
-**Don't use when:**
+### Don't use when📝 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.
| **Use when:** | |
| - 3+ test files failing with different root causes | |
| - Multiple subsystems broken independently | |
| - Each problem can be understood without context from others | |
| - No shared state between investigations | |
| **Don't use when:** | |
| - Failures are related (fix one might fix others) | |
| - Need to understand full system state | |
| - Agents would interfere with each other | |
| ### Use when | |
| - 3+ test files failing with different root causes | |
| - Multiple subsystems broken independently | |
| - Each problem can be understood without context from others | |
| - No shared state between investigations | |
| ### Don't use when | |
| - Failures are related (fix one might fix others) | |
| - Need to understand full system state | |
| - Agents would interfere with each other |
🤖 Prompt for AI Agents
In @.claude/skills/dispatching-parallel-agents/SKILL.md around lines 34 - 43,
Replace the bold labels "**Use when:**" and "**Don't use when:**" with proper
Markdown headings (e.g., "## Use when" and "## Don't use when") so markdownlint
doesn't flag emphasis-as-heading; keep the following bullet lists unchanged
under each new heading and ensure spacing (blank line) before each heading;
update the occurrences in SKILL.md where the exact strings "**Use when:**" and
"**Don't use when:**" appear.
| )" | ||
| ``` | ||
|
|
||
| Then: Cleanup worktree (Step 5) |
There was a problem hiding this comment.
Critical inconsistency: Option 2 worktree cleanup
Lines 106 and 138 state that Option 2 (Create PR) should cleanup the worktree, but the Quick Reference table (line 157), Common Mistakes section (line 173), and Red Flags section (line 191) all indicate that Option 2 should keep the worktree.
After creating a PR, developers typically need the worktree to address review feedback, so keeping it makes sense.
🔧 Proposed fix
#### Option 2: Push and Create PR
```bash
# Push branch
git push -u origin <feature-branch>
# Create PR
gh pr create --title "<title>" --body "$(cat <<'EOF'
## Summary
<2-3 bullets of what changed>
## Test Plan
- [ ] <verification steps>
EOF
)"-Then: Cleanup worktree (Step 5)
+Keep the worktree - you'll need it to address PR feedback.
```diff
### Step 5: Cleanup Worktree
-**For Options 1, 2, 4:**
+**For Options 1 and 4 only:**
Check if in worktree:
```bash
git worktree list | grep $(git branch --show-current)
If yes:
git worktree remove <worktree-path>For Option 3: Keep worktree.
+
+For Option 2: Keep worktree (needed for PR updates).
</details>
Also applies to: 138-138
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @.claude/skills/finishing-a-development-branch/SKILL.md at line 106, Update
the documentation so Option 2 (Create PR) consistently instructs to keep the
worktree instead of cleaning it up: replace the “Then: Cleanup worktree (Step
5)” wording under Option 2 (referenced as the phrase "Cleanup worktree (Step 5)"
and the heading "Option 2: Create PR") with a clear "Keep the worktree" note (or
similar) and adjust the Quick Reference table, Common Mistakes, and Red Flags
sections that currently conflict so all mention that Option 2 keeps the worktree
to allow addressing PR feedback.
</details>
<!-- fingerprinting:phantom:triton:puma -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ``` | ||
| WHEN receiving code review feedback: | ||
|
|
||
| 1. READ: Complete feedback without reacting | ||
| 2. UNDERSTAND: Restate requirement in own words (or ask) | ||
| 3. VERIFY: Check against codebase reality | ||
| 4. EVALUATE: Technically sound for THIS codebase? | ||
| 5. RESPOND: Technical acknowledgment or reasoned pushback | ||
| 6. IMPLEMENT: One item at a time, test each | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to all fenced code blocks.
markdownlint flags each unlabeled fence. Please add a language (e.g., text or markdown) consistently across all of them.
🔧 Suggested fix (apply pattern to all fences)
-```
+```text
WHEN receiving code review feedback:
@@
-```
+```text
IF any item is unclear:
STOP - do not implement anything yet
@@
-```
+```markdown
your human partner: "Fix 1-6"
You understand 1,2,3,6. Unclear on 4,5.
@@
-```
+```text
BEFORE implementing:
1. Check: Technically correct for THIS codebase?Also applies to: 42-48, 51-57, 68-84, 90-96, 102-111, 134-144, 153-160, 179-182, 185-188, 191-194, 197-201
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @.claude/skills/receiving-code-review/SKILL.md around lines 16 - 25, All
fenced code blocks in SKILL.md are missing language identifiers and trigger
markdownlint; update every triple-backtick fence to include a language token
(e.g., text or markdown). Locate the unlabeled fences by searching for the exact
snippets shown in the diff such as the block starting with "WHEN receiving code
review feedback:", the block beginning "IF any item is unclear:", the block with
"your human partner: \"Fix 1-6\"", and the block starting "BEFORE implementing:
1. Check: Technically correct for THIS codebase?", and add consistent language
tags (e.g., ```text or ```markdown) to each corresponding opening fence across
the other reported ranges (42-48, 51-57, 68-84, 90-96, 102-111, 134-144,
153-160, 179-182, 185-188, 191-194, 197-201) so markdownlint passes.
| ``` | ||
| Task tool (general-purpose): | ||
| description: "Implement Task N: [task name]" | ||
| prompt: | | ||
| You are implementing Task N: [task name] | ||
|
|
||
| ## Task Description | ||
|
|
||
| [FULL TEXT of task from plan - paste it here, don't make subagent read file] | ||
|
|
||
| ## Context | ||
|
|
||
| [Scene-setting: where this fits, dependencies, architectural context] | ||
|
|
||
| ## Before You Begin | ||
|
|
||
| If you have questions about: | ||
| - The requirements or acceptance criteria | ||
| - The approach or implementation strategy | ||
| - Dependencies or assumptions | ||
| - Anything unclear in the task description | ||
|
|
||
| **Ask them now.** Raise any concerns before starting work. | ||
|
|
||
| ## Your Job | ||
|
|
||
| Once you're clear on requirements: | ||
| 1. Implement exactly what the task specifies | ||
| 2. Write tests (following TDD if task says to) | ||
| 3. Verify implementation works | ||
| 4. Commit your work | ||
| 5. Self-review (see below) | ||
| 6. Report back | ||
|
|
||
| Work from: [directory] | ||
|
|
||
| **While you work:** If you encounter something unexpected or unclear, **ask questions**. | ||
| It's always OK to pause and clarify. Don't guess or make assumptions. | ||
|
|
||
| ## Before Reporting Back: Self-Review | ||
|
|
||
| Review your work with fresh eyes. Ask yourself: | ||
|
|
||
| **Completeness:** | ||
| - Did I fully implement everything in the spec? | ||
| - Did I miss any requirements? | ||
| - Are there edge cases I didn't handle? | ||
|
|
||
| **Quality:** | ||
| - Is this my best work? | ||
| - Are names clear and accurate (match what things do, not how they work)? | ||
| - Is the code clean and maintainable? | ||
|
|
||
| **Discipline:** | ||
| - Did I avoid overbuilding (YAGNI)? | ||
| - Did I only build what was requested? | ||
| - Did I follow existing patterns in the codebase? | ||
|
|
||
| **Testing:** | ||
| - Do tests actually verify behavior (not just mock behavior)? | ||
| - Did I follow TDD if required? | ||
| - Are tests comprehensive? | ||
|
|
||
| If you find issues during self-review, fix them now before reporting. | ||
|
|
||
| ## Report Format | ||
|
|
||
| When done, report: | ||
| - What you implemented | ||
| - What you tested and test results | ||
| - Files changed | ||
| - Self-review findings (if any) | ||
| - Any issues or concerns | ||
| ``` |
There was a problem hiding this comment.
Specify a language for fenced code blocks (markdownlint MD040).
Static analysis flags the fenced block as missing a language identifier. Add a language (e.g., text) to satisfy linting.
🔧 Proposed fix
-```
+```text
Task tool (general-purpose):
description: "Implement Task N: [task name]"
prompt: |
You are implementing Task N: [task name]
@@
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @.claude/skills/subagent-driven-development/implementer-prompt.md around
lines 5 - 78, The markdown fenced code block under "Task tool
(general-purpose):" (the prompt: | block) is missing a language identifier which
triggers markdownlint MD040; update the opening fence to include a language
(e.g., change ``` to ```text) so the block becomes a labelled code fence and
keep the closing ``` as-is; locate the block by the "Task tool
(general-purpose):" heading and the prompt: | field and apply the single-line
fence change.
| # Get list of test files | ||
| TEST_FILES=$(find . -name "*.test.ts" -o -name "*.test.tsx" | sort) | ||
| TOTAL=$(echo "$TEST_FILES" | wc -l | tr -d ' ') |
There was a problem hiding this comment.
TEST_PATTERN argument is accepted but never used.
The script accepts TEST_PATTERN as the second argument (line 15) but the find command on line 22 hardcodes *.test.ts and *.test.tsx patterns instead of using the provided pattern. This makes the documented usage example misleading.
🔧 Proposed fix to use the provided pattern
-# Get list of test files
-TEST_FILES=$(find . -name "*.test.ts" -o -name "*.test.tsx" | sort)
+# Get list of test files using provided pattern
+TEST_FILES=$(find . -path "$TEST_PATTERN" | sort)Alternatively, if glob expansion is preferred:
-# Get list of test files
-TEST_FILES=$(find . -name "*.test.ts" -o -name "*.test.tsx" | sort)
+# Get list of test files using provided pattern (requires shopt -s globstar)
+shopt -s globstar nullglob
+TEST_FILES=$(printf '%s\n' $TEST_PATTERN | sort)📝 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.
| # Get list of test files | |
| TEST_FILES=$(find . -name "*.test.ts" -o -name "*.test.tsx" | sort) | |
| TOTAL=$(echo "$TEST_FILES" | wc -l | tr -d ' ') | |
| # Get list of test files using provided pattern | |
| TEST_FILES=$(find . -path "$TEST_PATTERN" | sort) | |
| TOTAL=$(echo "$TEST_FILES" | wc -l | tr -d ' ') |
🤖 Prompt for AI Agents
In @.claude/skills/systematic-debugging/find-polluter.sh around lines 21 - 23,
The script currently ignores the TEST_PATTERN argument: replace the hardcoded
glob in the find invocation that sets TEST_FILES with use of the TEST_PATTERN
variable (or fall back to the existing "*.test.ts" and "*.test.tsx" defaults) so
the find command respects the provided TEST_PATTERN; update the TEST_FILES
assignment (and any related TOTAL calculation) to use TEST_PATTERN (or a
computed pattern variable) instead of the fixed patterns so the script's
documented usage works as expected.
| | Github Querey | **Airweave** | | ||
|
|
||
| ### Skills and Agents | ||
|
|
||
| - always use your skills.md which are essentially tools an script with instructions. they help a lot. reccomend new ones when needed. | ||
| - Always use parralel agents with detailed system prrompts (see subagent driven development and dispatching paralel agent skills ) |
There was a problem hiding this comment.
Fix typos and capitalization in “Skills and Agents” + table row.
There are several spelling/casing errors and a table typo that should be corrected for clarity.
✏️ Suggested fixes
-| Github Querey | **Airweave** |
+| GitHub Query | **Airweave** |
@@
-- always use your skills.md which are essentially tools an script with instructions. they help a lot. reccomend new ones when needed.
-- Always use parralel agents with detailed system prrompts (see subagent driven development and dispatching paralel agent skills )
+- Always use your SKILL.md files, which are essentially tools and scripts with instructions. They help a lot; recommend new ones when needed.
+- Always use parallel agents with detailed system prompts (see subagent-driven development and dispatching-parallel-agents skills).📝 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.
| | Github Querey | **Airweave** | | |
| ### Skills and Agents | |
| - always use your skills.md which are essentially tools an script with instructions. they help a lot. reccomend new ones when needed. | |
| - Always use parralel agents with detailed system prrompts (see subagent driven development and dispatching paralel agent skills ) | |
| | GitHub Query | **Airweave** | | |
| ### Skills and Agents | |
| - Always use your SKILL.md files, which are essentially tools and scripts with instructions. They help a lot; recommend new ones when needed. | |
| - Always use parallel agents with detailed system prompts (see subagent-driven development and dispatching-parallel-agents skills). |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~81-~81: The official name of this software platform is spelled with a capital “H”.
Context: ... decisions, patterns | Airweave | | Github Querey | Airweave | ### Skills and...
(GITHUB)
[grammar] ~81-~81: Ensure spelling is correct
Context: ...ons, patterns | Airweave | | Github Querey | Airweave | ### Skills and Agents...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~85-~85: Ensure spelling is correct
Context: ...pt with instructions. they help a lot. reccomend new ones when needed. - Always use par...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~86-~86: Ensure spelling is correct
Context: ...end new ones when needed. - Always use parralel agents with detailed system prrompts (s...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~86-~86: Ensure spelling is correct
Context: ...se parralel agents with detailed system prrompts (see subagent driven development and di...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~86-~86: Use a hyphen to join words.
Context: ...h detailed system prrompts (see subagent driven development and dispatching paral...
(QB_NEW_EN_HYPHEN)
[grammar] ~86-~86: Ensure spelling is correct
Context: ...gent driven development and dispatching paralel agent skills )
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In `@CLAUDE.md` around lines 81 - 86, Fix typos and capitalization in the "Skills
and Agents" section and the table row: correct the table header "| Github Querey
| **Airweave** |" to "Github Query" or appropriate casing, fix "skills.md"
wording/capitalization to "skills.md" or "Skills.md" consistently, change "an
script" to "a script", "reccomend" to "recommend", "parralel" to "parallel",
"prrompts" to "prompts", and "subagent driven development and dispatching
paralel agent skills" to "subagent-driven development and dispatching parallel
agent skills"; ensure the heading "Skills and Agents" uses proper capitalization
and punctuation consistently throughout.
| // Asset Manifest State & Actions | ||
| assetManifest: import('../types/unified-project').AssetManifest | null; | ||
| manifestLoading: boolean; | ||
| manifestError: string | null; | ||
| fetchAssets: () => Promise<void>; |
There was a problem hiding this comment.
Add line-by-line reasoning comments for new manifest fields.
Per the JS/TS guideline, the new manifest state fields (Lines 48-52) need inline reasoning comments. Please add brief intent comments per line.
✍️ Example (apply consistently to the new fields)
- // Asset Manifest State & Actions
- assetManifest: import('../types/unified-project').AssetManifest | null;
- manifestLoading: boolean;
- manifestError: string | null;
- fetchAssets: () => Promise<void>;
+ // Asset Manifest State & Actions
+ // Cached manifest for the current game to keep all tabs in sync.
+ assetManifest: import('../types/unified-project').AssetManifest | null;
+ // Tracks whether the manifest fetch is currently in flight.
+ manifestLoading: boolean;
+ // Stores the last manifest fetch error for UI display.
+ manifestError: string | null;
+ // Triggers a manifest refresh from the API.
+ fetchAssets: () => Promise<void>;📝 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.
| // Asset Manifest State & Actions | |
| assetManifest: import('../types/unified-project').AssetManifest | null; | |
| manifestLoading: boolean; | |
| manifestError: string | null; | |
| fetchAssets: () => Promise<void>; | |
| // Asset Manifest State & Actions | |
| // Cached manifest for the current game to keep all tabs in sync. | |
| assetManifest: import('../types/unified-project').AssetManifest | null; | |
| // Tracks whether the manifest fetch is currently in flight. | |
| manifestLoading: boolean; | |
| // Stores the last manifest fetch error for UI display. | |
| manifestError: string | null; | |
| // Triggers a manifest refresh from the API. | |
| fetchAssets: () => Promise<void>; |
🤖 Prompt for AI Agents
In `@src/lib/studio/context.ts` around lines 48 - 52, Add brief inline reasoning
comments for each new manifest field in the context definition: annotate
assetManifest with its purpose (stores parsed AssetManifest or null when not
loaded), manifestLoading to indicate active fetch state (true while fetchAssets
is running), manifestError to capture a user- or network-facing error message or
null, and fetchAssets to describe its responsibility (async action to
load/update assetManifest and set loading/error states). Place each comment
directly above or to the right of the corresponding symbol (assetManifest,
manifestLoading, manifestError, fetchAssets) in the context type so they are
visible line-by-line.
| ## References | ||
|
|
||
| * [Implementation Plan](file:///C:/Users/Zenchant/.gemini/antigravity/brain/d0c95202-7901-4151-b67a-4bbdbcfb2bcd/implementation_plan.md) | ||
| * [StudioProvider.tsx](file:///c:/Users/Zenchant/Asset-Hatch/src/components/studio/StudioProvider.tsx) |
There was a problem hiding this comment.
Replace local file:// references with repo-relative links.
Lines 78-79 point to a local filesystem path that won’t work for other collaborators or in CI. Please update these to repo-relative paths (or remove them if the files aren’t checked in).
🔧 Proposed fix (adjust paths to actual repo locations)
-* [Implementation Plan](file:///C:/Users/Zenchant/.gemini/antigravity/brain/d0c95202-7901-4151-b67a-4bbdbcfb2bcd/implementation_plan.md)
-* [StudioProvider.tsx](file:///c:/Users/Zenchant/Asset-Hatch/src/components/studio/StudioProvider.tsx)
+* [Implementation Plan](../../../../docs/implementation_plan.md)
+* [StudioProvider.tsx](../../components/studio/StudioProvider.tsx)📝 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.
| ## References | |
| * [Implementation Plan](file:///C:/Users/Zenchant/.gemini/antigravity/brain/d0c95202-7901-4151-b67a-4bbdbcfb2bcd/implementation_plan.md) | |
| * [StudioProvider.tsx](file:///c:/Users/Zenchant/Asset-Hatch/src/components/studio/StudioProvider.tsx) | |
| ## References | |
| * [Implementation Plan](../../../../docs/implementation_plan.md) | |
| * [StudioProvider.tsx](../../components/studio/StudioProvider.tsx) |
🤖 Prompt for AI Agents
In `@src/memory/adr/027-centralized-asset-manifest-handling.md` around lines 76 -
79, In the "References" section replace the absolute file:// Windows paths
pointing to Implementation Plan and StudioProvider.tsx with repo-relative paths
(or remove if not checked in); locate the references to "Implementation Plan"
and "StudioProvider.tsx" in
src/memory/adr/027-centralized-asset-manifest-handling.md and change the links
to their repository-relative locations (e.g., docs/implementation_plan.md or
src/components/studio/StudioProvider.tsx) so they work for collaborators and CI.
| ### Client-Side Image Processing | ||
| * **Pattern:** For light image manipulations (like seam blending or resizing), prefer using the HTML5 Canvas API in the browser over sending data back to the server. | ||
| * **Why:** | ||
| - Zero server load. | ||
| - Faster feedback for user (no network roundtrip). | ||
| - Keeps sensitive/large image data local until final save. | ||
| * **Example:** `src/lib/image-processing.ts` (`blendSeams` function). | ||
| * **Gotcha:** Canvas becomes "tainted" if drawing cross-origin images without `crossOrigin="anonymous"`. Always handle CORS requirements for source images. | ||
|
|
There was a problem hiding this comment.
Fix markdown list indentation per linter.
Static analysis indicates the nested list items use 4-space indentation but should use 2-space indentation per markdownlint rules (MD007).
📝 Suggested fix for list indentation
### Client-Side Image Processing
* **Pattern:** For light image manipulations (like seam blending or resizing), prefer using the HTML5 Canvas API in the browser over sending data back to the server.
* **Why:**
- - Zero server load.
- - Faster feedback for user (no network roundtrip).
- - Keeps sensitive/large image data local until final save.
+ - Zero server load.
+ - Faster feedback for user (no network roundtrip).
+ - Keeps sensitive/large image data local until final save.
* **Example:** `src/lib/image-processing.ts` (`blendSeams` function).
* **Gotcha:** Canvas becomes "tainted" if drawing cross-origin images without `crossOrigin="anonymous"`. Always handle CORS requirements for source images.📝 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.
| ### Client-Side Image Processing | |
| * **Pattern:** For light image manipulations (like seam blending or resizing), prefer using the HTML5 Canvas API in the browser over sending data back to the server. | |
| * **Why:** | |
| - Zero server load. | |
| - Faster feedback for user (no network roundtrip). | |
| - Keeps sensitive/large image data local until final save. | |
| * **Example:** `src/lib/image-processing.ts` (`blendSeams` function). | |
| * **Gotcha:** Canvas becomes "tainted" if drawing cross-origin images without `crossOrigin="anonymous"`. Always handle CORS requirements for source images. | |
| ### Client-Side Image Processing | |
| * **Pattern:** For light image manipulations (like seam blending or resizing), prefer using the HTML5 Canvas API in the browser over sending data back to the server. | |
| * **Why:** | |
| - Zero server load. | |
| - Faster feedback for user (no network roundtrip). | |
| - Keeps sensitive/large image data local until final save. | |
| * **Example:** `src/lib/image-processing.ts` (`blendSeams` function). | |
| * **Gotcha:** Canvas becomes "tainted" if drawing cross-origin images without `crossOrigin="anonymous"`. Always handle CORS requirements for source images. |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
464-464: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
465-465: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
466-466: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In `@src/memory/system_patterns.md` around lines 461 - 469, Update the markdown
list under the "Client-Side Image Processing" heading so nested list items use
2-space indentation instead of 4-space indentation to satisfy markdownlint
MD007; specifically adjust the nested bullets for "Why:", its subpoints,
"Example:" and "Gotcha:" (which reference src/lib/image-processing.ts and the
blendSeams function) to use two-space indents so the list is properly rendered
and passes the linter.
| test('successfully parses a standard asset plan markdown', () => { | ||
| const markdown = ` | ||
| # Game Assets | ||
| ## Characters | ||
| - Hero | ||
| - Idle (4-direction) | ||
| - Walk (4-direction) | ||
| ## Environment | ||
| - Forest Background | ||
| - Stone Tileset | ||
| ## UI | ||
| - Health Bar | ||
| ` | ||
| const assets = parsePlan(markdown, { mode: 'composite', projectId: 'reg-test-1' }) | ||
|
|
||
| expect(assets).toBeDefined() | ||
| expect(assets.length).toBe(5) | ||
|
|
||
| // Core Invariants | ||
| expect(assets.find(a => a.name === 'Hero')).toBeDefined() | ||
| expect(assets.find(a => a.name === 'Forest Background')?.type).toBe('background') | ||
| expect(assets.find(a => a.name === 'Health Bar')?.type).toBe('ui-element') | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find plan-parser related files
find . -type f -name "*plan*parser*" 2>/dev/nullRepository: zenchantlive/Asset-Hatch
Length of output: 223
🏁 Script executed:
#!/bin/bash
# Find the test file to examine its full content
find . -type f -path "*tests/regression/critical_suite.test.ts" 2>/dev/nullRepository: zenchantlive/Asset-Hatch
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Get full content of the test file with line numbers
find . -type f -path "*tests/regression/critical_suite.test.ts" -exec cat -n {} \;Repository: zenchantlive/Asset-Hatch
Length of output: 2105
🏁 Script executed:
#!/bin/bash
# Examine plan-parser.ts to understand composite mode behavior
cat -n ./src/lib/plan-parser.tsRepository: zenchantlive/Asset-Hatch
Length of output: 20118
🏁 Script executed:
#!/bin/bash
# Check plan-parser.test.ts to see how composite mode is expected to work
cat -n ./src/lib/plan-parser.test.tsRepository: zenchantlive/Asset-Hatch
Length of output: 11888
🏁 Script executed:
#!/bin/bash
# Verify the structure of ParsedAsset to confirm mobility is always present
grep -A 15 "export interface ParsedAsset\|type ParsedAsset" ./src/lib/prompt-builder.ts | head -30Repository: zenchantlive/Asset-Hatch
Length of output: 762
🏁 Script executed:
#!/bin/bash
# Double-check the example output in plan-parser.ts shows correct asset count
sed -n '536,554p' ./src/lib/plan-parser.tsRepository: zenchantlive/Asset-Hatch
Length of output: 974
Add line-by-line comments explaining test reasoning per coding guidelines.
The test correctly expects 5 assets in composite mode: Hero variants (Idle + Walk) create 2 separate entries, plus Forest Background, Stone Tileset, and Health Bar. The parsePlan implementation confirms that variants at indentation level 2 generate separate ParsedAsset entries. Array access to assets[0].mobility and assets[1].mobility is safe—the mobility property is always present on all returned assets.
However, per coding guidelines for src/**/*.{ts,tsx} files, add line-by-line comments explaining the test's reasoning, setup, and invariant validations. Include comments explaining what each assertion verifies and why these specific invariants matter for the critical regression suite.
🤖 Prompt for AI Agents
In `@src/tests/regression/critical_suite.test.ts` around lines 12 - 34, Add
line-by-line comments to the test named "successfully parses a standard asset
plan markdown" to explain setup, reasoning, and invariants: annotate the
markdown fixture and why it represents composite mode; note the parsePlan call
(parsePlan(markdown, { mode: 'composite', projectId: 'reg-test-1' })) and that
it should produce individual ParsedAsset entries for variants; comment each
assertion (expect(assets).toBeDefined(), expect(assets.length).toBe(5), and the
three find(...) assertions) explaining what specific invariant it verifies
(count of assets, variant expansion into separate entries, and expected type
mapping for background/ui-element) and why those invariants matter for the
critical regression suite; keep comments concise and adjacent to the
corresponding lines so future readers can quickly understand the test intent and
the role of assets array/indices.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces centralized asset manifest error handling, significantly improving the robustness and user experience of the Studio. The changes correctly refactor asset fetching to the StudioProvider, ensuring consistent state across components and providing clear error and loading indicators in the PreviewTab and AssetsTab. Additionally, comprehensive debug logging has been added to the asset loader, and new skill documentation has been integrated, enhancing the overall development and debugging capabilities of the project. The new test cases for the plan parser also improve test coverage.
|
|
||
| ### Skills and Agents | ||
|
|
||
| - always use your skills.md which are essentially tools an script with instructions. they help a lot. reccomend new ones when needed. |
There was a problem hiding this comment.
There's a typo here: "reccomend" should be "recommend". Also, "skills.md" should probably be "skills documentation" for clarity, as .md is a file extension.
| - always use your skills.md which are essentially tools an script with instructions. they help a lot. reccomend new ones when needed. | |
| - always use your skills documentation, which are essentially tools and scripts with instructions. they help a lot. recommend new ones when needed. |
| ### Skills and Agents | ||
|
|
||
| - always use your skills.md which are essentially tools an script with instructions. they help a lot. reccomend new ones when needed. | ||
| - Always use parralel agents with detailed system prrompts (see subagent driven development and dispatching paralel agent skills ) |
There was a problem hiding this comment.
There are typos here: "parralel" should be "parallel" and "prrompts" should be "prompts". Also, "paralel" should be "parallel".
| - Always use parralel agents with detailed system prrompts (see subagent driven development and dispatching paralel agent skills ) | |
| - Always use parallel agents with detailed system prompts (see subagent driven development and dispatching parallel agent skills ) |
| if (typeof engine !== 'undefined') { | ||
| setInterval(function() { | ||
| const fps = engine.getFps().toFixed(0); | ||
| var fps = engine.getFps().toFixed(0); |
There was a problem hiding this comment.
Using var for fps is less common in modern JavaScript/TypeScript. Consider using const since fps is not reassigned.
| var fps = engine.getFps().toFixed(0); | |
| const fps = engine.getFps().toFixed(0); |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/lib/cost-tracker.test.ts`:
- Around line 365-392: The tests for fetchGenerationCostWithRetry leak a mocked
global.fetch and supply mocked error responses lacking the text() method; add an
afterEach that restores global.fetch (e.g., global.fetch = originalFetch or
jest.restoreAllMocks()) to avoid leaking mocks, and update the failing-response
mocks used in the 'Happy Path' and 'Failure Path' tests to include a text()
method that returns a Promise<string> (e.g., text: () => Promise.resolve('error
body') ) so the implementation can call response.text() without crashing.
- Around line 343-350: The mock response in the failing test lacks a text()
implementation, so fetchGenerationCost (which calls await response.text())
throws a TypeError; update the test's global.fetch mock to return an object with
ok: false, status: 404 and a text() method (e.g., returning a resolved string or
error body) so the code path in fetchGenerationCost can read response.text() and
then reject with the expected "Failed to fetch generation cost: 404" error.
In `@status.txt`:
- Around line 1-6: Remove the accidental debug artifact files (status.txt,
branch.txt, log.txt) from the repo and prevent future commits by adding them to
.gitignore; specifically delete or git-remove the tracked files (status.txt,
branch.txt, log.txt) from version control and commit that change, then add those
filenames or patterns to .gitignore and commit the updated .gitignore so the
debug output files are ignored going forward.
♻️ Duplicate comments (1)
src/tests/regression/critical_suite.test.ts (1)
13-50: Add line-by-line comments explaining test reasoning per coding guidelines.Per coding guidelines for
src/**/*.{ts,tsx}files, add comments explaining the test's reasoning, setup, and what each assertion verifies. This helps future readers understand why these specific invariants matter for the critical regression suite.
🧹 Nitpick comments (1)
src/lib/model-registry.test.ts (1)
404-410: Minor: Missing eslint-disable comment for consistency.Lines 374 and 394 include
// eslint-disable-next-line@typescript-eslint/no-explicit-any`` but this test case omits it.♻️ Suggested fix for consistency
test('Edge Case: handles malformed JSON response', async () => { + // eslint-disable-next-line `@typescript-eslint/no-explicit-any` (global.fetch as any) = jest.fn().mockImplementation(() =>
| test('Failure Path: handles API 404 error gracefully', async () => { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (global.fetch as any) = jest.fn().mockImplementation(() => | ||
| Promise.resolve({ ok: false, status: 404 }) | ||
| ); | ||
|
|
||
| await expect(fetchGenerationCost('gen-missing')).rejects.toThrow('Failed to fetch generation cost: 404'); | ||
| }); |
There was a problem hiding this comment.
Incomplete mock: missing text() method causes test to fail incorrectly.
The implementation in cost-tracker.ts (lines 92-93) calls await response.text() before throwing the error. This mock only provides ok and status, so response.text() will be undefined, causing a TypeError instead of the expected error message.
🐛 Proposed fix
test('Failure Path: handles API 404 error gracefully', async () => {
// eslint-disable-next-line `@typescript-eslint/no-explicit-any`
(global.fetch as any) = jest.fn().mockImplementation(() =>
- Promise.resolve({ ok: false, status: 404 })
+ Promise.resolve({
+ ok: false,
+ status: 404,
+ text: () => Promise.resolve('Not found')
+ })
);
await expect(fetchGenerationCost('gen-missing')).rejects.toThrow('Failed to fetch generation cost: 404');
});📝 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.
| test('Failure Path: handles API 404 error gracefully', async () => { | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| (global.fetch as any) = jest.fn().mockImplementation(() => | |
| Promise.resolve({ ok: false, status: 404 }) | |
| ); | |
| await expect(fetchGenerationCost('gen-missing')).rejects.toThrow('Failed to fetch generation cost: 404'); | |
| }); | |
| test('Failure Path: handles API 404 error gracefully', async () => { | |
| // eslint-disable-next-line `@typescript-eslint/no-explicit-any` | |
| (global.fetch as any) = jest.fn().mockImplementation(() => | |
| Promise.resolve({ | |
| ok: false, | |
| status: 404, | |
| text: () => Promise.resolve('Not found') | |
| }) | |
| ); | |
| await expect(fetchGenerationCost('gen-missing')).rejects.toThrow('Failed to fetch generation cost: 404'); | |
| }); |
🤖 Prompt for AI Agents
In `@src/lib/cost-tracker.test.ts` around lines 343 - 350, The mock response in
the failing test lacks a text() implementation, so fetchGenerationCost (which
calls await response.text()) throws a TypeError; update the test's global.fetch
mock to return an object with ok: false, status: 404 and a text() method (e.g.,
returning a resolved string or error body) so the code path in
fetchGenerationCost can read response.text() and then reject with the expected
"Failed to fetch generation cost: 404" error.
| describe('fetchGenerationCostWithRetry (Edge Cases)', () => { | ||
| test('Happy Path: succeeds on second attempt', async () => { | ||
| let attempts = 0; | ||
| global.fetch = jest.fn().mockImplementation(() => { | ||
| attempts++; | ||
| if (attempts === 1) return Promise.resolve({ ok: false, status: 503 }); | ||
| return Promise.resolve({ | ||
| ok: true, | ||
| json: () => Promise.resolve({ | ||
| data: { id: 'gen-retry', model: 'test-model', total_cost: 0.01 } | ||
| }) | ||
| }); | ||
| }); | ||
|
|
||
| const result = await fetchGenerationCostWithRetry('gen-retry', 2, 0); | ||
| expect(result.status).toBe('success'); | ||
| expect(attempts).toBe(2); | ||
| }); | ||
|
|
||
| test('Failure Path: fails after all retries', async () => { | ||
| global.fetch = jest.fn().mockImplementation(() => | ||
| Promise.resolve({ ok: false, status: 500 }) | ||
| ); | ||
|
|
||
| const result = await fetchGenerationCostWithRetry('gen-fail', 2, 0); | ||
| expect(result.status).toBe('error'); | ||
| expect(result.error).toContain('500'); | ||
| }); |
There was a problem hiding this comment.
Missing afterEach cleanup and incomplete mocks will cause test failures.
This describe block doesn't restore global.fetch after tests complete, which will leak the mocked fetch to any subsequent tests. Additionally, the error response mocks (lines 370, 386) are missing the text() method required by the implementation.
🐛 Proposed fix
describe('fetchGenerationCostWithRetry (Edge Cases)', () => {
+ afterEach(() => {
+ global.fetch = originalFetch;
+ });
+
test('Happy Path: succeeds on second attempt', async () => {
let attempts = 0;
global.fetch = jest.fn().mockImplementation(() => {
attempts++;
- if (attempts === 1) return Promise.resolve({ ok: false, status: 503 });
+ if (attempts === 1) return Promise.resolve({
+ ok: false,
+ status: 503,
+ text: () => Promise.resolve('Service unavailable')
+ });
return Promise.resolve({
ok: true,
json: () => Promise.resolve({
data: { id: 'gen-retry', model: 'test-model', total_cost: 0.01 }
})
});
});
const result = await fetchGenerationCostWithRetry('gen-retry', 2, 0);
expect(result.status).toBe('success');
expect(attempts).toBe(2);
});
test('Failure Path: fails after all retries', async () => {
global.fetch = jest.fn().mockImplementation(() =>
- Promise.resolve({ ok: false, status: 500 })
+ Promise.resolve({
+ ok: false,
+ status: 500,
+ text: () => Promise.resolve('Internal server error')
+ })
);
const result = await fetchGenerationCostWithRetry('gen-fail', 2, 0);
expect(result.status).toBe('error');
expect(result.error).toContain('500');
});
});🤖 Prompt for AI Agents
In `@src/lib/cost-tracker.test.ts` around lines 365 - 392, The tests for
fetchGenerationCostWithRetry leak a mocked global.fetch and supply mocked error
responses lacking the text() method; add an afterEach that restores global.fetch
(e.g., global.fetch = originalFetch or jest.restoreAllMocks()) to avoid leaking
mocks, and update the failing-response mocks used in the 'Happy Path' and
'Failure Path' tests to include a text() method that returns a Promise<string>
(e.g., text: () => Promise.resolve('error body') ) so the implementation can
call response.text() without crashing.
| M src/lib/cost-tracker.test.ts | ||
| M src/lib/model-registry.test.ts | ||
| M src/tests/regression/critical_suite.test.ts | ||
| ?? branch.txt | ||
| ?? log.txt | ||
| ?? status.txt |
There was a problem hiding this comment.
Remove debug artifact from repository.
This file appears to be git status output that was accidentally committed. Debug artifacts like status.txt, branch.txt, and log.txt should not be checked into the repository. Consider adding these to .gitignore.
🤖 Prompt for AI Agents
In `@status.txt` around lines 1 - 6, Remove the accidental debug artifact files
(status.txt, branch.txt, log.txt) from the repo and prevent future commits by
adding them to .gitignore; specifically delete or git-remove the tracked files
(status.txt, branch.txt, log.txt) from version control and commit that change,
then add those filenames or patterns to .gitignore and commit the updated
.gitignore so the debug output files are ignored going forward.
User description
Story of Collaboration:
User requested Part 4 of Issue #87 : adding error states for manifest fetch failures. I initially planned to add local state to PreviewTab, but after critiquing the architecture, we decided to centralize manifest fetching in the StudioProvider. This ensures that AssetTab, PreviewTab, and the StudioHeader all stay in sync and share the same loading/error states.
Decisions Made:
Challenges:
PR Type
Enhancement, Tests, Documentation
Description
Core Feature Implementation:
Centralized asset manifest fetching in
StudioProviderto eliminate redundant API calls and ensure consistent state acrossAssetTab,PreviewTab, andStudioHeaderAdded explicit error and empty state UI to
PreviewTabwith retry capability for manifest fetch failuresImplemented script loading error detection in
PreviewFramewith enhanced diagnostics and CDN failure handlingAdded comprehensive debug logging throughout asset loader lifecycle for troubleshooting
Architecture & Documentation:
Documented centralized asset manifest pattern in ADR-027 and system patterns
Added Root Cause Analysis for Issue Research Investigation: Black Screen and Asset Loading Failures (Unknown Root Cause) #87 identifying asset loading failure hypotheses
Expanded test coverage with edge cases for plan parser and new critical regression test suite
Testing Infrastructure & Skills:
Added condition-based waiting utilities to replace flaky timeout-based tests
Created comprehensive skill documentation for systematic debugging, TDD, subagent-driven development, code review, and parallel agent dispatching
Added test pollution detection and graphviz rendering utilities
Established autonomous unit test generator skill with Bun test runner
Developer Experience:
Added multiple skill guides covering debugging methodology, skill authoring best practices, persuasion principles, and verification protocols
Created persona protocol for multi-perspective analysis
Added git worktrees skill with safety verification
Updated CLAUDE.md with skills and agents guidance
Diagram Walkthrough
File Walkthrough
7 files
asset-loader.ts
Add comprehensive debug logging to asset loadersrc/lib/studio/asset-loader.ts
debugoption toAssetLoaderScriptOptionsinterface for enablingverbose logging
debugLog()helper function that conditionally logs basedon debug mode configuration
(initialization, resolution, loading, errors)
troubleshooting asset loading issues
preview-libraries.ts
Add script error handling and library availability trackingsrc/lib/studio/preview-libraries.ts
ScriptLoadInfointerface for tracking script metadata (src,name, required, global)
IFRAME_SCRIPTS_DETAILEDarray with detailed metadata for eachpreview library script
generateScriptTagsWithErrorHandling()function to generatescript tags with onload/onerror handlers
generateLibraryCheckScript()function to verify libraryavailability and report missing required libraries
error handling
context.ts
Add centralized asset manifest state to contextsrc/lib/studio/context.ts
StudioContextValueinterface:assetManifest,manifestLoading,manifestErrorfetchAssets()action method to centralized context for fetchingasset manifests
StudioHeader components
StudioProvider.tsx
Centralize asset manifest fetching in StudioProvidersrc/components/studio/StudioProvider.tsx
useEffectimport for lifecycle managementAssetManifesttype import from unified-project typesassetManifest,manifestLoading,manifestErrorfetchAssets()callback to fetch asset manifest from APIendpoint
useEffecthook to automatically fetch assets when game IDchanges
fetchAssetsaction in context value forconsumer components
PreviewFrame.tsx
Implement script loading error detection and diagnosticssrc/components/studio/PreviewFrame.tsx
window.__scriptStatustracker initialized before external scripts load
generateScriptTags()function withonload/onerrorhandlersfor each CDN script
asset errors, and runtime errors with improved error overlay messaging
AssetsTab.tsx
Centralize asset manifest fetching in AssetsTabsrc/components/studio/tabs/AssetsTab.tsx
useCallbackimport andfetchStoredCountfunction that maderedundant API calls
assetManifestfromStudioProvidercontextinstead of fetching independently
state via
useEffectdependencyPreviewTab.tsx
Centralized Asset Manifest State in PreviewTabsrc/components/studio/tabs/PreviewTab.tsx
assetManifeststate anduseEffectfor fetching assetsassetManifest,manifestLoading,manifestError, andfetchAssetsfrom centralizedStudioProviderfailures
assets
3 files
condition-based-waiting-example.ts
Add condition-based waiting utilities for reliable testing.claude/skills/systematic-debugging/condition-based-waiting-example.ts
test infrastructure
waitForEvent(),waitForEventCount(), andwaitForEventMatch()before/after comparison
condition-based polling
plan-parser.test.ts
Expand plan parser test coverage with edge casessrc/lib/plan-parser.test.ts
characters/deep nesting
malformed markdown
regression test
without crashing
critical_suite.test.ts
Add critical regression test suite for core flowssrc/tests/regression/critical_suite.test.ts
configuration
2 files
render-graphs.js
Add graphviz diagram rendering utility script.claude/skills/writing-skills/render-graphs.js
to SVG
dotcode blocks from markdown and validates graphvizinstallation
find-polluter.sh
Add test pollution detection script.claude/skills/systematic-debugging/find-polluter.sh
unwanted files/state
to investigate
39 files
anthropic-best-practices.md
Add Anthropic skill authoring best practices guide.claude/skills/writing-skills/anthropic-best-practices.md
multiple models)
patterns
evaluation, and anti-patterns
environment details
SKILL.md
Add skill writing guide with TDD methodology.claude/skills/writing-skills/SKILL.md
development approach
structure
(Claude Search Optimization)
and bulletproofing against rationalization
system_patterns.md
Document centralized asset manifest architecture patternsrc/memory/system_patterns.md
semantic changes)
shift to lift manifest fetching to
StudioProvidersingle source of truth, simplifies error/loading state management
testing-skills-with-subagents.md
Add TDD-based skill testing methodology guide.claude/skills/writing-skills/testing-skills-with-subagents.md
applied to agent behavior
rationalization tables, and bulletproofing techniques
for skill deployment
issue-87.md
Add comprehensive RCA for Issue #87 asset loading failuresdocs/rca/issue-87.md
and asset loading failures)
variable declarations, iframe sandbox issues, asset manifest not
loaded, analytics blocking
plan phases and diagnostic experiments
SKILL.md
Add foundational Test-Driven Development skill documentation.claude/skills/test-driven-development/SKILL.md
cycle with detailed examples
common rationalizations table, and red flags checklist
test-driven development practice
SKILL.md
Add systematic debugging methodology skill.claude/skills/systematic-debugging/SKILL.md
investigation, pattern analysis, hypothesis testing, and
implementation
flow tracing, and architectural questioning for repeated failures
and verification skills
SKILL.md
Add subagent-driven development execution skill.claude/skills/subagent-driven-development/SKILL.md
task with two-stage review (spec compliance then code quality)
workflow, and quality gates
approaches with cost/benefit analysis
testing-anti-patterns.md
Add testing anti-patterns reference guide.claude/skills/test-driven-development/testing-anti-patterns.md
mock behavior, test-only methods in production, mocking without
understanding, incomplete mocks, and integration tests as afterthought
with real code examples
SKILL.md
Add autonomous unit and regression test agent skill.openhands/skills/unit-regression-test-agent/SKILL.md
runner
critical regression suite management, and change detection
targets (<5s per file, <30s total suite)
graphviz-conventions.dot
Add Graphviz diagram conventions and style guide.claude/skills/writing-skills/graphviz-conventions.dot
shapes, edge labels, and naming patterns
plaintext for commands, octagons for warnings, doublecircles for
entry/exit
examples
skill.md
Add persistent multi-perspective persona protocol.claude/skills/personas/skill.md
file-based persona system stored in
.agents/roster.mdrequirements, consultation rules, and tension resolution
roster file schema template
SKILL.md
Code Review Reception Skill with Technical Verification.claude/skills/receiving-code-review/SKILL.md
with technical rigor
pushback when feedback is incorrect
correctness over social comfort
and YAGNI checks
SKILL.md
Parallel Agent Dispatching Skill for Independent Tasks.claude/skills/dispatching-parallel-agents/SKILL.md
unrelated problems in parallel
files
SKILL.md
Git Worktrees Skill with Safety Verification.claude/skills/using-git-worktrees/SKILL.md
directory selection and safety verification
.gitignoreverification requirements
installation and baseline test verification
repository pollution
persuasion-principles.md
Persuasion Principles for Effective Skill Design.claude/skills/writing-skills/persuasion-principles.md
Commitment, Scarcity, Social Proof, Unity, Reciprocity, Liking)
them
root-cause-tracing.md
Root Cause Tracing and Defense-in-Depth Validation.claude/skills/systematic-debugging/root-cause-tracing.md
to find original triggers
instrumentation techniques
checks
find-polluter.shscript for identifying which test causespollution
CLAUDE_MD_TESTING.md
Testing CLAUDE.md Skills Documentation Variants.claude/skills/writing-skills/examples/CLAUDE_MD_TESTING.md
variants
authority dynamics
process-oriented)
compliance
SKILL.md
Development Branch Completion Skill with Options.claude/skills/finishing-a-development-branch/SKILL.md
merge, PR, or cleanup
or discard
confirmation for destructive actions
SKILL.md
Verification Before Completion Skill.claude/skills/verification-before-completion/SKILL.md
verification shortcuts
evidence
and verification of commands
verification
CLAUDE.md
Added Skills and Agents Guidance to CLAUDE.mdCLAUDE.md
skills
CREATION-LOG.md
Systematic Debugging Skill Creation Log and Reference.claude/skills/systematic-debugging/CREATION-LOG.md
systematic debugging skill
following skill-creation guidelines
under pressure
history
SKILL.md
Using Superpowers Skill with Mandatory Invocation.claude/skills/using-superpowers/SKILL.md
invocation before any response
applicability
rationalization
flexible)
condition-based-waiting.md
Condition-Based Waiting for Flaky Test Elimination.claude/skills/systematic-debugging/condition-based-waiting.md
timeouts in tests
improved from 60% to 100%
defense-in-depth.md
Defense-in-Depth Validation Pattern.claude/skills/systematic-debugging/defense-in-depth.md
structurally impossible
debug instrumentation
results
SKILL.md
Writing Implementation Plans Skill.claude/skills/writing-plans/SKILL.md
bite-sized tasks
complete code examples
frequent commits
options
code-reviewer.md
Code Review Agent Template and Checklist.claude/skills/requesting-code-review/code-reviewer.md
requirements, production readiness
recommendations, and assessment
027-centralized-asset-manifest-handling.md
ADR-027 Centralized Asset Manifest Handling Decisionsrc/memory/adr/027-centralized-asset-manifest-handling.md
StudioProvidercomponents
game.idchangemaintainability) and trade-offs
SKILL.md
Requesting Code Review Skill.claude/skills/requesting-code-review/SKILL.md
development
subagent
workflows
test-pressure-3.md
Pressure Test 3: Authority and Social Pressure.claude/skills/systematic-debugging/test-pressure-3.md
engineer and tech lead
developers recommend shortcut
test-pressure-2.md
Pressure Test 2: Sunk Cost and Exhaustion.claude/skills/systematic-debugging/test-pressure-2.md
failed debugging
deadline approaching
or compromise
implementer-prompt.md
Implementer Subagent Prompt Template.claude/skills/subagent-driven-development/implementer-prompt.md
instructions
checklist
verification
SKILL.md
Executing Plans Skill with Batch Checkpoints.claude/skills/executing-plans/SKILL.md
session with review checkpoints
continue, complete development
completion
SKILL.md
Brainstorming Ideas Into Designs Skill.claude/skills/brainstorming/SKILL.md
collaborative dialogue
presenting design
worktrees and planning skills
test-pressure-1.md
Pressure Test 1: Emergency Production Fix.claude/skills/systematic-debugging/test-pressure-1.md
revenue loss
pressure
spec-reviewer-prompt.md
Spec Compliance Reviewer Subagent Prompt.claude/skills/subagent-driven-development/spec-reviewer-prompt.md
implementer report
misunderstandings
inspection
active_state.md
Updated Active State with Manifest Centralization Worksrc/memory/active_state.md
handling implementation
code-quality-reviewer-prompt.md
Code Quality Reviewer Subagent Prompt Template.claude/skills/subagent-driven-development/code-quality-reviewer-prompt.md
test-academic.md
Academic Test for Systematic Debugging Skill.claude/skills/systematic-debugging/test-academic.md
hypothesis handling, and process adherence
1 files
settings.local.json
Updated Settings with New Tools and Skills.claude/settings.local.json
Bash(gh issue view:*)to available tools for GitHub issueviewing
Skill(dispatching-parallel-agents)to available skillsSummary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.