Add gameplay review workflows for streamers#1677
Open
Ayush-Raj-Chourasia wants to merge 3 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a full gameplay review system: Rust backend RPCs, schemas, and storage; a TypeScript service layer; a React workspace UI for importing/analyzing frames, Q&A, and drafting clip metadata; Intelligence page integration; and component tests. ChangesGameplay Review Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/openhuman/gameplay_review/ops.rs (1)
1-631:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix rustfmt violations before merge.
CI is currently failing on
cargo fmt --checkfor this file. Please runcargo fmt --alland commit the formatted output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/gameplay_review/ops.rs` around lines 1 - 631, The file fails rustfmt checks; run rustfmt over this module and commit the changes. Format the file containing functions like analyze_session_frames, build_session, truncate, and the impl HighlightKind (and tests such as register_and_list_sessions_round_trip) by running `cargo fmt --all` (or `rustfmt` on this file) and commit the resulting changes so `cargo fmt --check` passes in CI.src/openhuman/gameplay_review/schemas.rs (1)
1-179:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRun
cargo fmtto fix formatting issues blocking the pipeline.The pipeline reports formatting differences that must be resolved before merge.
cargo fmt --all🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/gameplay_review/schemas.rs` around lines 1 - 179, The repository is failing CI due to rustfmt formatting differences; run `cargo fmt --all` (or `rustfmt` on changed files) and commit the resulting formatting changes so functions like all_controller_schemas, schemas, json_output, deserialize_params, the various handle_* functions, and to_json in src/openhuman/gameplay_review/schemas.rs are properly formatted; re-run the pipeline and push the formatted file(s).src/core/all.rs (1)
332-410:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd namespace description for "gameplay_review" to CLI help output.
The
namespace_descriptionfunction is missing an entry for the new "gameplay_review" namespace. This meansopenhuman gameplay_review --helpwon't display a description.Suggested addition
"whatsapp_data" => Some( "Structured WhatsApp conversation and message store — list chats, read messages, and search across WhatsApp Web data.", ), + "gameplay_review" => Some( + "Import gameplay keyframes, analyze sessions for highlights, draft clip metadata, and manage game coaching presets.", + ), _ => None, } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/all.rs` around lines 332 - 410, The CLI help lacks a description for the new "gameplay_review" namespace in the namespace_description function; add a new match arm for "gameplay_review" returning Some(...) with a short descriptive string (e.g., "Gameplay review tooling: run playtests, collect logs, and annotate sessions for QA and design feedback.") so openhuman gameplay_review --help shows a helpful description; update the match in namespace_description to include "gameplay_review" alongside the other namespace entries.src/openhuman/gameplay_review/store.rs (1)
1-176:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRun
cargo fmtto fix formatting issues blocking the pipeline.The pipeline reports formatting differences including function signature wrapping and
map_errline breaks that must be resolved before merge.cargo fmt --all🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/gameplay_review/store.rs` around lines 1 - 176, Formatting in src/openhuman/gameplay_review/store.rs is broken (signature wrapping and map_err line breaks); run rustfmt to fix it: execute cargo fmt --all and commit the changes so functions like slugify, ensure_dirs, session_path, save_session, load_session, list_sessions, save_preset, load_preset, list_presets and preset_from_input follow the project's rustfmt rules and restore the expected line-wrapping for map_err chains and function signatures.
🧹 Nitpick comments (6)
app/src/components/intelligence/GameplayReviewWorkspace.test.tsx (1)
164-168: ⚡ Quick winPrefer accessible queries over implementation-detail selectors.
The test uses
container.querySelector('input[type="file"]')and a specific CSS class selector (div.font-medium.text-stone-900) to find elements. These implementation-detail queries make the test brittle and go against the behavior-driven testing guideline. Consider alternatives:
- For the file input: use
screen.getByLabelText()or add adata-testidif no accessible label exists- For the highlight title: remove the CSS selector constraint and rely on the text match alone
As per coding guidelines, prefer testing behavior over implementation details in Vitest unit tests.
Also applies to: 175-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/intelligence/GameplayReviewWorkspace.test.tsx` around lines 164 - 168, The test is using implementation-detail selectors (container.querySelector('input[type="file"]') and a CSS class selector) which makes it brittle; replace these with accessible queries: locate the file input via its accessible label using screen.getByLabelText('...') or add a data-testid to the input and use screen.getByTestId('file-input') (update the test that declares fileInput and the fireEvent.change call to use that retrieval), and remove the CSS-class constrained lookup for the highlight title—use screen.getByText('Highlight Title Text') or screen.getByRole/getByLabelText for the element text instead so the assertions rely on behavior/accessible queries rather than implementation details.app/src/components/intelligence/GameplayReviewWorkspace.tsx (3)
208-217: 💤 Low value
handleDraftClipssilently drops drafts whenprev.analysisis missing.If
draftGameplayClipMetadatasucceeds for an unanalyzed session (e.g., the user reopened an imported-only session and the button was somehow enabled), the merge falls into the: prev.analysisbranch and the returneddraftsare thrown away. Today the button is disabled when there's noactiveSession, butactiveSession.analysisbeing null is still reachable. Synthesizing a minimal analysis object preserves the result.♻️ Proposed refactor
setActiveSession(prev => prev ? { ...prev, analysis: prev.analysis ? { ...prev.analysis, draft_metadata: drafts } - : prev.analysis, + : { + recap: '', + highlights: [], + clip_candidates: [], + draft_metadata: drafts, + follow_up_questions: [], + spoiler_note: null, + }, } : prev );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/intelligence/GameplayReviewWorkspace.tsx` around lines 208 - 217, handleDraftClips currently drops the new drafts if activeSession exists but activeSession.analysis is null because the updater returns prev.analysis when falsy; change the updater passed to setActiveSession (used in handleDraftClips) to synthesize a minimal analysis object when prev.analysis is missing (e.g., set analysis to { draft_metadata: drafts } or merge into existing analysis when present) so draftGameplayClipMetadata's result is preserved in activeSession.analysis.draft_metadata; ensure you only mutate via the updater and keep other prev fields intact.
183-197: 💤 Low valueGuard against empty/whitespace questions and clear stale answers.
handleQuestionwill issue an RPC even whenquestionis an empty/whitespace string, and the previousquestionAnswerlingers until the new one resolves. A simplequestion.trim()guard plus clearingquestionAnswerbefore the call would tighten the UX.♻️ Proposed refactor
const handleQuestion = useCallback(async () => { if (!activeSession) { setError('Select or analyze a session first.'); return; } + const trimmed = question.trim(); + if (!trimmed) { + setError('Type a question to ask the session.'); + return; + } setQuestionBusy(true); + setQuestionAnswer(''); try { - const response = await askGameplaySession({ session_id: activeSession.session_id, question }); + const response = await askGameplaySession({ + session_id: activeSession.session_id, + question: trimmed, + }); setQuestionAnswer(response.answer); } catch (err) { setError(normalizeGameplayError(err)); } finally { setQuestionBusy(false); } }, [activeSession, question]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/intelligence/GameplayReviewWorkspace.tsx` around lines 183 - 197, In handleQuestion, prevent empty/whitespace submissions and clear stale answers: check question.trim() at the top of the handleQuestion callback (when activeSession exists) and return early (setError('Enter a question.') or similar) if it's empty; before making the RPC call to askGameplaySession, clear previous answer by calling setQuestionAnswer('') so the UI doesn't show stale data while awaiting the response; keep these changes inside the existing handleQuestion useCallback.
159-180: 💤 Low value
analysisBusyflickers onregister/saveGameplayPresetfailures.
setAnalysisBusy(true)is only set right beforeanalyzeGameplaySession, butsetAnalysisBusy(false)runs infinallyregardless. That's fine on the success path, but ifsaveGameplayPresetorregisterGameplaySessionthrows, the "Analyzing…" state never gets set to begin with — yet the orphan session is still registered server-side and will reappear viarefreshSessionswithout analysis. Consider either:
- analyzing inside its own
try/catchso a partial failure surfaces a more specific message, or- using a single
busystate for the whole import-and-analyze flow to avoid mismatched busy semantics across handlers.This is not a correctness bug, just a UX/observability gap when the pipeline fails between register and analyze.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/intelligence/GameplayReviewWorkspace.tsx` around lines 159 - 180, The UX issue is that setAnalysisBusy(true) is only called immediately before analyzeGameplaySession while setAnalysisBusy(false) is run in finally, causing a flicker when registerGameplaySession or saveGameplayPreset fail; fix by either (A) moving the analysis busy flag earlier (setAnalysisBusy(true) before calling registerGameplaySession/saveGameplayPreset) and ensuring each error path clears it appropriately, or (B) replace the two separate flags with a single combined busy state (e.g., useImportAndAnalyzeBusy) and use that across registerGameplaySession, saveGameplayPreset, analyzeGameplaySession and refreshSessions so the UI reflects the entire import+analyze pipeline; reference setAnalysisBusy, setImportBusy, registerGameplaySession, saveGameplayPreset, analyzeGameplaySession, and refreshSessions when making the change.app/src/services/gameplayReviewService.ts (2)
127-142: ⚡ Quick winRead and encode frames in parallel, and consider
FileReader.readAsDataURL.The current loop awaits
file.arrayBuffer()sequentially and then manually chunks the bytes throughString.fromCharCode+btoa. Switching toPromise.allparallelizes the I/O, andFileReader.readAsDataURL(orBlob→Response.arrayBuffer()→btoa) avoids the intermediate binary‑string concatenation, which can spike memory on larger frame folders.♻️ Proposed refactor
- const prepared: PreparedGameplayFrame[] = []; - for (const file of imageFiles) { - const buffer = await file.arrayBuffer(); - const bytes = new Uint8Array(buffer); - let base64 = ''; - for (let index = 0; index < bytes.length; index += 0x8000) { - base64 += String.fromCharCode(...bytes.subarray(index, index + 0x8000)); - } - prepared.push({ - source_name: file.webkitRelativePath || file.name, - file_name: file.webkitRelativePath || file.name, - image_ref: `data:${file.type || 'image/png'};base64,${btoa(base64)}`, - captured_at_ms: file.lastModified || null, - }); - } - return prepared; + return Promise.all( + imageFiles.map( + file => + new Promise<PreparedGameplayFrame>((resolve, reject) => { + const reader = new FileReader(); + reader.onerror = () => reject(reader.error ?? new Error('Failed to read file')); + reader.onload = () => { + const name = file.webkitRelativePath || file.name; + resolve({ + source_name: name, + file_name: name, + image_ref: String(reader.result), + captured_at_ms: file.lastModified || null, + }); + }; + reader.readAsDataURL(file); + }) + ) + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/services/gameplayReviewService.ts` around lines 127 - 142, The current serial loop in prepare frames reads each File via file.arrayBuffer() and builds a binary string for btoa, which is memory-inefficient and slow; change the logic in the block that builds prepared: use Promise.all over imageFiles to read them in parallel and produce data URLs (either via FileReader.readAsDataURL or by using blob -> Response -> arrayBuffer -> base64 helper) instead of manual chunked String.fromCharCode + btoa; ensure you still set source_name/file_name from file.webkitRelativePath || file.name and captured_at_ms from file.lastModified, and replace the image_ref construction with the direct data URL returned by the parallel reader.
213-222: 💤 Low valueMake the
lightspoiler mode case explicit.Routing
'light'throughdefaultworks today but silently mis-labels any futureSpoilerModevalue as "Light spoilers". Handle'light'explicitly and let the exhaustiveness of the union surface unhandled cases.♻️ Proposed refactor
export function formatSpoilerMode(mode: SpoilerMode): string { switch (mode) { case 'off': return 'Spoiler-safe'; + case 'light': + return 'Light spoilers'; case 'full': return 'Full spoilers'; default: return 'Light spoilers'; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/services/gameplayReviewService.ts` around lines 213 - 222, The function formatSpoilerMode currently routes 'light' through the default branch which hides new/unhandled SpoilerMode values; change formatSpoilerMode to include an explicit case for 'light' returning "Light spoilers" and replace the default return with an exhaustive check (e.g. an assertUnreachable or throwing Error for unknown mode) so the TypeScript union SpoilerMode exhaustiveness will surface unhandled cases at compile time.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/components/intelligence/GameplayReviewWorkspace.tsx`:
- Around line 78-92: refreshSessions currently closes over activeSession which
forces its identity to change and re-run the useEffect each time a session is
selected; to fix it, remove activeSession from refreshSessions' dependency list
and change the auto-select logic to use the functional updater form when setting
active session (e.g. call setActiveSession(prev => prev ?? sessions[0]) after
obtaining sessions from listGameplaySessions), keep refreshSessions memoized
without activeSession, and keep the useEffect dependent only on refreshSessions
so the RPC runs once on mount.
In `@src/core/all.rs`:
- Around line 156-159: Run rustfmt (cargo fmt --all) to fix formatting;
specifically format the call to controllers.extend and the path
crate::openhuman::gameplay_review::all_gameplay_review_registered_controllers()
so it matches repository style (e.g., collapse the multi-line extend argument
into the canonical formatting produced by rustfmt). Ensure you save the file
after formatting and re-run cargo fmt to confirm no remaining diffs.
In `@src/openhuman/about_app/catalog.rs`:
- Around line 655-704: The new Capability struct entries (e.g., entries with id
"screen_intelligence.gameplay_review_import",
"screen_intelligence.gameplay_review_analyze", etc.) are misformatted and
failing CI; run rustfmt (cargo fmt --all) or apply the project's rustfmt
settings to reformat the Capability blocks so they match the repository style
(align braces/fields, spacing, and trailing commas) and then commit the
formatted changes.
In `@src/openhuman/gameplay_review/ops.rs`:
- Around line 23-149: All RPC handlers (register_session, analyze_session,
get_session, list_sessions, set_preset, list_presets, ask_session,
draft_clip_metadata) and the analysis path (analyze_session ->
analyze_session_frames) lack structured diagnostics; add debug/trace logs with a
stable grep-friendly prefix like "[rpc][gameplay_review]" at entry and exit of
each public function, around key branch decisions (e.g., missing session/preset,
branch in analyze_session for preset presence), state transitions
(session.analyzed_at_ms, session.analysis set), and external I/O calls
(store::load_session, store::save_session, store::load_preset,
analyze_session_frames), always including correlation fields such as session_id,
game_id, frame_index where applicable; use the tracing or log crate at
debug/trace level and ensure messages include the prefix and the key fields for
easy grep and correlation.
In `@src/openhuman/gameplay_review/schemas.rs`:
- Around line 118-174: Add entry/exit diagnostic logging to each handler
(handle_register_session, handle_analyze_session, handle_get_session,
handle_list_sessions, handle_set_preset, handle_list_presets,
handle_ask_session, handle_draft_clip_metadata) using the tracing/log crate at
debug/trace level: emit an "entered" log with a correlation id (generate or
extract from params if present) and a redacted snapshot of request params, log
before/after the RPC call (e.g., crate::openhuman::gameplay_review::rpc::*)
including the correlation id, and emit an "exiting"/result log with the
correlation id and outcome; on errors log at error level with the correlation id
and error details. Ensure any sensitive fields in the deserialized payload are
redacted before logging and that correlation ids are consistently reused across
entry/exit/RPC/error logs.
In `@src/openhuman/gameplay_review/store.rs`:
- Around line 100-102: The listing functions currently silently ignore files
that fail JSON deserialization; update list_sessions and list_presets to log a
warning when serde_json::from_str::<GameplayReviewSession>(&raw) (and the
analogous deserialization in list_presets) returns Err: capture the error and
call the module logger (or process logger used elsewhere) with a warning that
includes the file name/path and the error details before continuing, instead of
silently skipping, so operators see which files are corrupted while still
continuing to push valid sessions into sessions via sessions.push(session).
- Around line 114-121: save_preset currently writes directly to the target file
and can corrupt presets on a crash; change it to the same atomic
temp-file-then-rename pattern used by save_session: after ensure_dirs and
computing path via preset_path, serialize the preset to payload, write payload
to a temp file in the same directory (unique name or .tmp suffix), flush/close
it, then atomically rename (fs::rename) the temp file to the final path, mapping
errors to the same Result<String> messages; reference save_preset, save_session,
preset_path, and ensure_dirs when making the change.
- Around line 63-121: Add verbose diagnostics logging to all storage functions
(save_session, load_session, list_sessions, save_preset and the not-shown
load_preset/list_presets) by emitting entry and exit logs and logging each
external filesystem call and error path using stable grep-friendly prefixes
(e.g. "[storage]" or "[storage:session]"); include correlation fields
(session_id, game_id, file paths) in the messages and log both success and
failure details (serialize/write/read/rename results and underlying error
strings) so every branch (start, FS ops, errors, and successful return) is
logged with consistent level semantics (debug/info for entry/exit and op
details, error for failures).
In `@src/openhuman/mod.rs`:
- Line 78: The module declaration pub mod gameplay_review; is misordered per
rustfmt; run cargo fmt --all (or rustfmt) to reorder module declarations so
gameplay_review is placed in the correct alphabetical position in the mod list
(adjust the module block around pub mod gameplay_review; and neighboring pub mod
entries to match rustfmt's ordering).
---
Outside diff comments:
In `@src/core/all.rs`:
- Around line 332-410: The CLI help lacks a description for the new
"gameplay_review" namespace in the namespace_description function; add a new
match arm for "gameplay_review" returning Some(...) with a short descriptive
string (e.g., "Gameplay review tooling: run playtests, collect logs, and
annotate sessions for QA and design feedback.") so openhuman gameplay_review
--help shows a helpful description; update the match in namespace_description to
include "gameplay_review" alongside the other namespace entries.
In `@src/openhuman/gameplay_review/ops.rs`:
- Around line 1-631: The file fails rustfmt checks; run rustfmt over this module
and commit the changes. Format the file containing functions like
analyze_session_frames, build_session, truncate, and the impl HighlightKind (and
tests such as register_and_list_sessions_round_trip) by running `cargo fmt
--all` (or `rustfmt` on this file) and commit the resulting changes so `cargo
fmt --check` passes in CI.
In `@src/openhuman/gameplay_review/schemas.rs`:
- Around line 1-179: The repository is failing CI due to rustfmt formatting
differences; run `cargo fmt --all` (or `rustfmt` on changed files) and commit
the resulting formatting changes so functions like all_controller_schemas,
schemas, json_output, deserialize_params, the various handle_* functions, and
to_json in src/openhuman/gameplay_review/schemas.rs are properly formatted;
re-run the pipeline and push the formatted file(s).
In `@src/openhuman/gameplay_review/store.rs`:
- Around line 1-176: Formatting in src/openhuman/gameplay_review/store.rs is
broken (signature wrapping and map_err line breaks); run rustfmt to fix it:
execute cargo fmt --all and commit the changes so functions like slugify,
ensure_dirs, session_path, save_session, load_session, list_sessions,
save_preset, load_preset, list_presets and preset_from_input follow the
project's rustfmt rules and restore the expected line-wrapping for map_err
chains and function signatures.
---
Nitpick comments:
In `@app/src/components/intelligence/GameplayReviewWorkspace.test.tsx`:
- Around line 164-168: The test is using implementation-detail selectors
(container.querySelector('input[type="file"]') and a CSS class selector) which
makes it brittle; replace these with accessible queries: locate the file input
via its accessible label using screen.getByLabelText('...') or add a data-testid
to the input and use screen.getByTestId('file-input') (update the test that
declares fileInput and the fireEvent.change call to use that retrieval), and
remove the CSS-class constrained lookup for the highlight title—use
screen.getByText('Highlight Title Text') or screen.getByRole/getByLabelText for
the element text instead so the assertions rely on behavior/accessible queries
rather than implementation details.
In `@app/src/components/intelligence/GameplayReviewWorkspace.tsx`:
- Around line 208-217: handleDraftClips currently drops the new drafts if
activeSession exists but activeSession.analysis is null because the updater
returns prev.analysis when falsy; change the updater passed to setActiveSession
(used in handleDraftClips) to synthesize a minimal analysis object when
prev.analysis is missing (e.g., set analysis to { draft_metadata: drafts } or
merge into existing analysis when present) so draftGameplayClipMetadata's result
is preserved in activeSession.analysis.draft_metadata; ensure you only mutate
via the updater and keep other prev fields intact.
- Around line 183-197: In handleQuestion, prevent empty/whitespace submissions
and clear stale answers: check question.trim() at the top of the handleQuestion
callback (when activeSession exists) and return early (setError('Enter a
question.') or similar) if it's empty; before making the RPC call to
askGameplaySession, clear previous answer by calling setQuestionAnswer('') so
the UI doesn't show stale data while awaiting the response; keep these changes
inside the existing handleQuestion useCallback.
- Around line 159-180: The UX issue is that setAnalysisBusy(true) is only called
immediately before analyzeGameplaySession while setAnalysisBusy(false) is run in
finally, causing a flicker when registerGameplaySession or saveGameplayPreset
fail; fix by either (A) moving the analysis busy flag earlier
(setAnalysisBusy(true) before calling
registerGameplaySession/saveGameplayPreset) and ensuring each error path clears
it appropriately, or (B) replace the two separate flags with a single combined
busy state (e.g., useImportAndAnalyzeBusy) and use that across
registerGameplaySession, saveGameplayPreset, analyzeGameplaySession and
refreshSessions so the UI reflects the entire import+analyze pipeline; reference
setAnalysisBusy, setImportBusy, registerGameplaySession, saveGameplayPreset,
analyzeGameplaySession, and refreshSessions when making the change.
In `@app/src/services/gameplayReviewService.ts`:
- Around line 127-142: The current serial loop in prepare frames reads each File
via file.arrayBuffer() and builds a binary string for btoa, which is
memory-inefficient and slow; change the logic in the block that builds prepared:
use Promise.all over imageFiles to read them in parallel and produce data URLs
(either via FileReader.readAsDataURL or by using blob -> Response -> arrayBuffer
-> base64 helper) instead of manual chunked String.fromCharCode + btoa; ensure
you still set source_name/file_name from file.webkitRelativePath || file.name
and captured_at_ms from file.lastModified, and replace the image_ref
construction with the direct data URL returned by the parallel reader.
- Around line 213-222: The function formatSpoilerMode currently routes 'light'
through the default branch which hides new/unhandled SpoilerMode values; change
formatSpoilerMode to include an explicit case for 'light' returning "Light
spoilers" and replace the default return with an exhaustive check (e.g. an
assertUnreachable or throwing Error for unknown mode) so the TypeScript union
SpoilerMode exhaustiveness will surface unhandled cases at compile time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30299d21-83fa-4f43-ad9e-c2a0dc16e483
📒 Files selected for processing (12)
app/src/components/intelligence/GameplayReviewWorkspace.test.tsxapp/src/components/intelligence/GameplayReviewWorkspace.tsxapp/src/pages/Intelligence.tsxapp/src/services/gameplayReviewService.tssrc/core/all.rssrc/openhuman/about_app/catalog.rssrc/openhuman/gameplay_review/mod.rssrc/openhuman/gameplay_review/ops.rssrc/openhuman/gameplay_review/schemas.rssrc/openhuman/gameplay_review/store.rssrc/openhuman/gameplay_review/types.rssrc/openhuman/mod.rs
Comment on lines
+156
to
+159
| // Desktop gameplay review workflow | ||
| controllers.extend( | ||
| crate::openhuman::gameplay_review::all_gameplay_review_registered_controllers(), | ||
| ); |
Contributor
There was a problem hiding this comment.
Run cargo fmt to fix formatting issues blocking the pipeline.
The pipeline reports formatting differences in the controllers.extend call that must be resolved before merge.
cargo fmt --all🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/all.rs` around lines 156 - 159, Run rustfmt (cargo fmt --all) to fix
formatting; specifically format the call to controllers.extend and the path
crate::openhuman::gameplay_review::all_gameplay_review_registered_controllers()
so it matches repository style (e.g., collapse the multi-line extend argument
into the canonical formatting produced by rustfmt). Ensure you save the file
after formatting and re-run cargo fmt to confirm no remaining diffs.
Comment on lines
+655
to
+704
| Capability { | ||
| id: "screen_intelligence.gameplay_review_import", | ||
| name: "Import Gameplay Review Sessions", | ||
| domain: "screen_intelligence", | ||
| category: CapabilityCategory::ScreenIntelligence, | ||
| description: "Import local gameplay keyframes or a recordings folder into a review session.", | ||
| how_to: "Intelligence > Gameplay > Import session", | ||
| status: CapabilityStatus::Beta, | ||
| privacy: LOCAL_RAW, | ||
| }, | ||
| Capability { | ||
| id: "screen_intelligence.gameplay_review_analyze", | ||
| name: "Analyze Gameplay Sessions", | ||
| domain: "screen_intelligence", | ||
| category: CapabilityCategory::ScreenIntelligence, | ||
| description: "Generate a recap, highlight list, and review notes from imported gameplay frames.", | ||
| how_to: "Intelligence > Gameplay > Analyze session", | ||
| status: CapabilityStatus::Beta, | ||
| privacy: LOCAL_RAW, | ||
| }, | ||
| Capability { | ||
| id: "screen_intelligence.gameplay_review_clip_metadata", | ||
| name: "Draft Clip Metadata", | ||
| domain: "screen_intelligence", | ||
| category: CapabilityCategory::ScreenIntelligence, | ||
| description: "Generate platform-ready titles, descriptions, and tags for gameplay clips.", | ||
| how_to: "Intelligence > Gameplay > Draft clips", | ||
| status: CapabilityStatus::Beta, | ||
| privacy: LOCAL_RAW, | ||
| }, | ||
| Capability { | ||
| id: "screen_intelligence.gameplay_review_presets", | ||
| name: "Save Game Coaching Presets", | ||
| domain: "screen_intelligence", | ||
| category: CapabilityCategory::ScreenIntelligence, | ||
| description: "Store game-specific coaching focus, spoiler mode, and notes for repeat reviews.", | ||
| how_to: "Intelligence > Gameplay > Presets", | ||
| status: CapabilityStatus::Beta, | ||
| privacy: LOCAL_RAW, | ||
| }, | ||
| Capability { | ||
| id: "screen_intelligence.gameplay_review_spoilers", | ||
| name: "Set Spoiler Controls", | ||
| domain: "screen_intelligence", | ||
| category: CapabilityCategory::ScreenIntelligence, | ||
| description: "Keep gameplay coaching spoiler-safe or story-aware per session.", | ||
| how_to: "Intelligence > Gameplay > Spoiler mode", | ||
| status: CapabilityStatus::Beta, | ||
| privacy: LOCAL_RAW, | ||
| }, |
Contributor
There was a problem hiding this comment.
Run cargo fmt to fix formatting issues blocking the pipeline.
The pipeline reports formatting differences in the new capability definitions that must be resolved before merge.
cargo fmt --all🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/about_app/catalog.rs` around lines 655 - 704, The new
Capability struct entries (e.g., entries with id
"screen_intelligence.gameplay_review_import",
"screen_intelligence.gameplay_review_analyze", etc.) are misformatted and
failing CI; run rustfmt (cargo fmt --all) or apply the project's rustfmt
settings to reformat the Capability blocks so they match the repository style
(align braces/fields, spacing, and trailing commas) and then commit the
formatted changes.
Contributor
|
@Ayush-Raj-Chourasia CI is failing on changes in this PR — please fix before review. |
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
2 participants
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
Adds a gameplay-review workflow in the core and Intelligence tab so streamers can import keyframe folders, generate recaps/highlights, ask follow-up questions, and draft clip metadata.
Validation
git diff --checkAyush-Raj-Chourasia/openhumanNotes
The feature is implemented as a new gameplay-review domain backed by workspace persistence, with the UI using local keyframe import plus RPC analysis.
Summary by CodeRabbit
New Features
Backend / Platform
Tests