Fix action filtering logic and add safety improvements#163
Open
zky001 wants to merge 10 commits intoTHU-MAIC:mainfrom
Open
Fix action filtering logic and add safety improvements#163zky001 wants to merge 10 commits intoTHU-MAIC:mainfrom
zky001 wants to merge 10 commits intoTHU-MAIC:mainfrom
Conversation
1. formatQuestionsForPrompt called .join() on QuizOption objects, producing "[object Object]" in AI prompts instead of readable option text (e.g. "A. Option1, B. Option2"). 2. json-repair Fix 1 regex double-escaped valid JSON escapes (\t, \n, \r, \b, \f) and couldn't handle strings with escaped quotes. Now uses a proper string-aware regex and preserves valid escapes. 3. PlaybackEngine spotlight/laser actions used synchronous recursion via processNext(), risking stack overflow with many consecutive non-speech actions. Now uses queueMicrotask() to break recursion. https://claude.ai/code/session_01JdigqQtiAvomeifvWSQbjm
The early return in Step 6 (slide-only action filter) caused Step 7 (allowedActions whitelist) to be skipped for non-slide scenes. This meant hallucinated actions from agents in quiz/interactive/pbl scenes were not filtered by the role-based allowedActions whitelist. Refactored to use a mutable result variable instead of early returns so both filters always apply. https://claude.ai/code/session_01JdigqQtiAvomeifvWSQbjm
1. executePlayVideo could hang forever if the video element was invalid or the state change was missed between playVideo() and subscribe(). Added a 5-minute safety timeout to prevent indefinite blocking. 2. CJK language detection in browser TTS divided by chunkText.length without checking for empty strings, producing NaN. Now guards against zero-length text. https://claude.ai/code/session_01JdigqQtiAvomeifvWSQbjm
…o remaining content When a previously-partial text item completed but all content had already been streamed as deltas (remaining is empty), the code still pushed an ordered entry pointing to textChunks.length - 1, which could be -1 or reference an unrelated chunk. This caused spurious warnings in director-graph and potential text loss. Now only pushes the ordered entry when there is actual remaining content to emit. https://claude.ai/code/session_01JdigqQtiAvomeifvWSQbjm
… order 1. quiz-grade: `points` from request body was used without validation, causing NaN in score calculation and invalid JSON responses when points was undefined, negative, or non-numeric. Now validates it is a positive finite number. 2. parse-pdf: metadata spread order put the default `pageCount` before `...result.metadata`, so the spread would overwrite the fallback. Fixed by spreading metadata first, then applying defaults after. Also changed `||` to `??` so pageCount=0 is preserved correctly. https://claude.ai/code/session_01JdigqQtiAvomeifvWSQbjm
… in canCombine 1. use-order-element.ts: moveUpElement crashed with TypeError when a grouped element was already at the top of the z-order. The code accessed copyOfElementList[maxLevel + 1] without bounds checking, then dereferenced .groupId on undefined. Added early return guard matching the pattern used in the non-grouped branch. 2. video-providers.ts: normalizeVideoOptions had a dead-code ternary inside an if-block that already proved the condition was false. Simplified to directly use the fallback value. 3. use-canvas-operations.ts: canCombine check had incorrect parenthesization: (el.groupId && el.groupId) === firstGroupId evaluated the && first, which is equivalent to just el.groupId. Fixed to: el.groupId && el.groupId === firstGroupId. https://claude.ai/code/session_01JdigqQtiAvomeifvWSQbjm
There was a problem hiding this comment.
Pull request overview
This PR strengthens several runtime safety edges in the generation/playback pipeline while also fixing a few correctness issues (action filtering, quiz option formatting, JSON repair, and ordering/grouping edge cases) to reduce hangs and malformed outputs.
Changes:
- Consolidates action filtering so slide-only stripping and
allowedActionswhitelisting are applied sequentially. - Hardens playback and parsing: microtask scheduling for rapid spotlight/laser sequences, division-by-zero guard for language detection, safer LaTeX escaping in JSON repair, and a timeout to prevent indefinite video waits.
- Fixes smaller correctness issues across quiz prompting, PDF metadata normalization, video option normalization, canvas grouping checks, and element ordering boundaries.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| lib/playback/engine.ts | Avoids deep synchronous recursion for consecutive effect actions and guards against division by zero in TTS language detection. |
| lib/orchestration/stateless-generate.ts | Prevents emitting ordered text entries when no actual text delta is produced. |
| lib/media/video-providers.ts | Simplifies aspect ratio fallback to consistently pick the provider’s first supported ratio. |
| lib/hooks/use-order-element.ts | Adds boundary protection when moving grouped elements upward in z-order. |
| lib/hooks/use-canvas-operations.ts | Fixes group comparison predicate for determining if selection can be grouped. |
| lib/generation/scene-generator.ts | Formats quiz options using both option value and label in prompts. |
| lib/generation/json-repair.ts | Preserves valid JSON escapes while double-escaping LaTeX command escapes inside strings. |
| lib/generation/action-parser.ts | Ensures slide-only and allowedActions filters both apply in sequence without early returns. |
| lib/action/engine.ts | Adds a safety timeout to prevent playback hangs when waiting for video completion state. |
| app/api/quiz-grade/route.ts | Validates points as a positive finite number before grading. |
| app/api/parse-pdf/route.ts | Ensures pageCount is always set and not overwritten by metadata spread ordering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
INVALID_PARAMETER is not a member of API_ERROR_CODES. Use INVALID_REQUEST instead, which is the appropriate code for invalid request parameters. https://claude.ai/code/session_01JdigqQtiAvomeifvWSQbjm
Author
|
@wyuc @YizukiAme fixing a few correctness issues (action filtering, quiz option formatting, JSON repair, and ordering/grouping edge cases) to reduce hangs and malformed outputs. |
Contributor
Hey dude, I think you might have @'d me thinking I'm a maintainer...? I'm actually just a random community contributor 🙃 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR consolidates action filtering logic to prevent early returns and adds several safety improvements across the codebase, including timeout protection for video playback, better LaTeX escape handling in JSON parsing, and fixes for edge cases in video options and quiz grading.
Changes
resultvariable instead of multiple early returns, ensuring both slide-only and allowedActions filters are applied in sequencequeueMicrotaskfor spotlight/laser actions to prevent stack overflow from deep recursionType of Change
Verification
What was verified
Evidence
pnpm check && pnpm lint && npx tsc --noEmit)https://claude.ai/code/session_01JdigqQtiAvomeifvWSQbjm