feat: Phase 10 - Over-the-Shoulder Coaching, Voice & Skill Refresh#9
Merged
zackfunprojects merged 2 commits intomainfrom Apr 14, 2026
Merged
feat: Phase 10 - Over-the-Shoulder Coaching, Voice & Skill Refresh#9zackfunprojects merged 2 commits intomainfrom
zackfunprojects merged 2 commits intomainfrom
Conversation
Premium teaching modalities: screen-aware coaching, voice dialogue, file uploads, and spaced skill review for completed treks. New Edge Functions: - screen-analyze: Claude Vision analyzes screenshots for OTS coaching (Pro) - sherpa-voice: Whisper STT -> Claude -> ElevenLabs TTS voice pipeline - skill-refresh: generates review exercises from notebook entries (Pro) New frontend: - voice.js: MediaRecorder, base64 encoding, audio playback utilities - CoachingPanel: OTS slide-in with screen capture via getDisplayMedia - FiresideMode: warm amber voice dialogue overlay with Sherpa - SkillRefresh: inline review exercises on notebook entries - VoiceResponseLedge: full implementation replacing stub - FileUploadLedge: full implementation with Supabase Storage upload Modified: - sherpa.js: added analyzeScreen, voiceChat, refreshSkill wrappers - LearningView: OTS Coach and Fireside buttons + panel integration - TrekNotebookView: Skill Refresh integration per entry - SummitCard: added Skill Refresh button External API requirements: - OPENAI_API_KEY (Whisper STT) - ELEVENLABS_API_KEY + ELEVENLABS_VOICE_ID (TTS) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds interactive voice and file-based learning flows, AI-powered coaching analysis, and skill refresh exercises. It includes three new Supabase Edge Functions (screen-analyze, sherpa-voice, skill-refresh), a voice utility library, and updates multiple React components to enable file uploads, voice responses, voice chat, screen-based coaching, and pro-tier skill practice. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant VRL as VoiceResponseLedge
participant VM as Voice Module
participant SC as Sherpa Client
participant SF as sherpa-voice<br/>Edge Function
participant OpenAI as OpenAI Whisper
participant Claude as Claude API
User->>VRL: Click Record
VRL->>VM: startRecording()
VM->>VM: Request microphone access
User->>VRL: Speak & Click Stop
VRL->>VM: stopRecording()
VM->>VM: Encode audio to base64
VM-->>VRL: base64 audio
VRL->>SC: voiceChat(audioBase64)
SC->>SF: POST sherpa-voice
SF->>OpenAI: Transcribe audio
OpenAI-->>SF: transcript
SF->>Claude: Generate response<br/>(with conversation history)
Claude-->>SF: response_text
SF->>SF: Optional ElevenLabs TTS
SF-->>SC: { transcript, response_text, audio_base64 }
SC-->>VRL: response data
VRL->>VRL: Append to messages
alt audio_base64 present
VRL->>VM: playAudioBase64()
VM->>VM: Play via Audio element
end
VRL-->>User: Display transcript & play response
sequenceDiagram
actor User
participant CP as CoachingPanel
participant SC as Sherpa Client
participant SF as screen-analyze<br/>Edge Function
participant MediaAPI as navigator<br/>mediaDevices
participant Claude as Claude API
User->>CP: Click "Share Screen<br/>for Coaching"
CP->>MediaAPI: getDisplayMedia()
User->>MediaAPI: Select screen
MediaAPI-->>CP: MediaStream
CP->>CP: Capture frame via<br/>ImageCapture
CP->>CP: Convert to base64 PNG
CP->>SC: analyzeScreen(screenshot)
SC->>SF: POST screen-analyze
SF->>SF: Validate auth & pro-tier
SF->>Claude: Analyze screenshot<br/>with context
Claude-->>SF: { analysis,<br/>coaching_points,<br/>suggestion }
SF-->>SC: analysis data
SC-->>CP: analysis result
CP->>CP: Stop media tracks
CP-->>User: Display analysis,<br/>coaching points,<br/>suggestion
sequenceDiagram
actor User
participant SR as SkillRefresh
participant SC as Sherpa Client
participant SF as skill-refresh<br/>Edge Function
participant Claude as Claude API
participant DB as Supabase
User->>SR: Click "Start Refresh"
SR->>SC: refreshSkill(notebookEntryId)
SC->>SF: POST skill-refresh
SF->>DB: Fetch notebook entry
DB-->>SF: entry data
SF->>SF: Validate ownership<br/>& pro-tier
SF->>Claude: Generate 3–5<br/>exercise prompts
Claude-->>SF: { exercises: [...] }
SF->>DB: Update<br/>last_refreshed_at
SF-->>SC: exercises array
SC-->>SR: exercises data
SR->>SR: Initialize currentIndex=0
loop For each exercise
User->>SR: Answer question
SR->>SR: Store answer
User->>SR: Click Next
SR->>SR: Advance currentIndex
end
User->>SR: Click Finish
SR-->>User: Close / Done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (2)
src/components/trek/FiresideMode.jsx (1)
20-25: Consider resetting conversation state when panel closes.The cleanup effect stops the recorder but doesn't reset
messages,error, or other state. When the user reopens the Fireside panel, the previous conversation persists. If this is intentional (session continuity), this is fine. If each session should start fresh, add state resets.♻️ Optional: Reset state on close
// Cleanup recorder on close useEffect(() => { - if (!open && recorderRef?.state === 'recording') { - recorderRef.stop() + if (!open) { + if (recorderRef?.state === 'recording') { + recorderRef.stop() + } + // Uncomment if fresh session desired: + // setMessages([]) + // setError(null) } }, [open, recorderRef])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/trek/FiresideMode.jsx` around lines 20 - 25, The cleanup effect that watches open and recorderRef currently only stops the recorder but doesn't clear conversation state; update the effect (the useEffect that checks if (!open && recorderRef?.state === 'recording')) to also reset component state on panel close by calling the relevant setters such as setMessages([]), setError(null) and any other session-specific setters you have (e.g., setIsRecording(false), setConversationId(null) or similar) so that reopening FiresideMode starts a fresh session; keep the recorder stop behavior and ensure these resets only run when open becomes false.src/components/summit/SkillRefresh.jsx (1)
145-164: Short answer allows empty submissions.Once a user types anything in the short answer input (even if they clear it),
answers[currentIndex]becomes an empty string. Since'' !== undefinedistrue, the "Next" button appears, allowing users to proceed with a blank answer. Consider requiring non-empty input.♻️ Proposed fix to require non-empty answers
{/* Next button */} - {answers[currentIndex] !== undefined && ( + {(answers[currentIndex] !== undefined && + (currentExercise.type !== 'short_answer' || answers[currentIndex]?.trim())) && ( <button🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/summit/SkillRefresh.jsx` around lines 145 - 164, The Next/Finish button is shown when answers[currentIndex] !== undefined which treats an empty string as valid; change the visibility check to require a non-empty, non-whitespace answer (e.g. test answers[currentIndex] exists and answers[currentIndex].toString().trim().length > 0) so blank submissions are disabled; update the conditional that renders the button (and any related logic in handleNext/onClose if they assume non-empty) and reference answers, currentIndex, handleAnswer, handleNext, onClose, currentExercise.type and exercises to locate the code to modify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ledges/FileUploadLedge.jsx`:
- Around line 40-42: The ext extraction in FileUploadLedge.jsx currently uses
file.name.split('.').pop() which returns the whole filename for names without
extensions (e.g., "Makefile") or mishandles dotfiles; update the logic inside
the try block where ext and path are created so ext is derived only when there
is a true extension (use file.name.lastIndexOf('.') or a /\.([^.]+)$/ regex to
capture the suffix) and fall back to a safe default like 'bin' otherwise, then
build path using that ext (the existing path template using user.id and
Date.now. should remain unchanged).
- Line 11: FileUploadLedge currently destructures { spec, onSubmit } and calls
onSubmit(...) which mismatches ExerciseWrapper (which passes onResponseChange)
and causes runtime undefined; change the prop name to onResponseChange in the
component signature and all internal uses (replace onSubmit references,
including the call in handleSubmit) to onResponseChange to match other ledges
(ConversationSimLedge, VoiceResponseLedge), and also accept and use the disabled
prop (e.g., prevent submission or disable inputs when disabled) so the passed
disabled flag is honored.
In `@src/components/trek/CoachingPanel.jsx`:
- Around line 97-99: In CoachingPanel.jsx update the copy inside the paragraph
element (the <p className="font-mono text-phosphor-green/40 text-xs"> node) used
in the Pro gate message: replace the incorrect phrase "real tools" with "real
time" so the sentence reads "The Sherpa watches your screen in real time and
coaches live." Keep everything else (className and markup) unchanged.
- Around line 25-31: The ImageCapture usage can throw in browsers without
support (e.g., Firefox); update the section that calls
navigator.mediaDevices.getDisplayMedia, extracts track =
stream.getVideoTracks()[0], creates new ImageCapture(track) and calls
imageCapture.grabFrame() to first feature-detect support and wrap the
ImageCapture creation/grabFrame call in a try-catch; on failure fall back to
capturing a frame from a hidden video element/canvas using the MediaStream (or
show a clear user-facing error) and ensure the track is stopped on error so
resources are released.
In `@src/components/trek/FiresideMode.jsx`:
- Around line 183-185: The UI copy in FiresideMode.jsx is incorrect: the
paragraph using the conditional expression with recording and processing
currently shows "Hold to speak" but the component uses click-to-toggle
start/stop handlers (not press-and-hold). Update the text to reflect click/tap
behavior (for example "Click to speak" or "Tap to speak" as appropriate) in the
JSX where the expression {recording ? 'Release to send' : processing ?
'Processing...' : 'Hold to speak'} is defined so the default branch matches the
actual toggle interaction; keep the existing conditional logic but replace the
"Hold to speak" string with the correct wording.
In `@src/lib/voice.js`:
- Around line 21-25: The MediaRecorder fallback is unsafe: update
isRecordingSupported() to check MediaRecorder API existence AND probe a
prioritized list of MIME types via MediaRecorder.isTypeSupported (e.g.,
'audio/webm;codecs=opus', 'audio/webm', 'audio/ogg;codecs=opus', 'audio/wav')
and return true only if one is supported; when creating the recorder in
startRecording() select the first supported MIME and store it (e.g.,
selectedMime) alongside the Blob, and send that MIME to the backend with the
upload (e.g., as a form field or an HTTP header); then update the backend
handler in sherpa-voice/index.ts to accept the provided MIME type parameter and
use it instead of hardcoding 'audio/webm' (and validate it against an allowlist)
so the server correctly processes the actual format recorded.
- Around line 86-107: The playAudioBase64 function currently ignores the Promise
returned by audio.play(), leaving playAudioBase64 unresolved on playback
rejection; update playAudioBase64 to capture the return value of audio.play(),
check if it's a Promise, and attach .then/.catch handlers that on success do
nothing (playback proceeds) and on rejection revoke the object URL and reject
the outer Promise with the error; ensure existing audio.onended and
audio.onerror still revoke the URL and resolve/reject, and guard against
double-settlement by only resolving/rejecting once (e.g., via a local settled
flag) so legacy browsers that return undefined still work.
In `@supabase/functions/screen-analyze/index.ts`:
- Around line 137-138: The catch block in
supabase/functions/screen-analyze/index.ts currently logs part of Claude’s
response via console.error("Failed to parse screen analysis:", text.slice(0,
300)); remove any direct logging of the variable text (which may contain PII)
and instead log only a safe diagnostic such as the caught error object/message
and a non-sensitive metric (e.g., text length or "redacted response"); update
the catch to accept the error parameter and call console.error with a generic
message and error (but not the response content) so you no longer write
screen-derived model output to logs.
- Around line 130-147: The JSON parsing block currently assigns
JSON.parse(cleaned) directly to result and then assumes result is an object;
instead, parse into a local const (e.g., parsed = JSON.parse(cleaned)), validate
that parsed is non-null, typeof parsed === "object", and Array.isArray(parsed)
=== false, and if validation fails throw a new Error("Expected JSON object") so
the existing catch handles it; after validation assign parsed to result (e.g.,
result = parsed as Record<string, unknown>) and then continue with the existing
normalization of result.analysis, result.coaching_points, and result.suggestion
to avoid runtime errors when Claude returns an array or scalar.
In `@supabase/functions/sherpa-voice/index.ts`:
- Around line 91-171: Add a transcript-only short-circuit: read a boolean flag
from the incoming request body (e.g., transcriptOnly or transcript_only) and if
true immediately perform the minimal transcript upload/DB update and return the
transcript JSON response without calling anthropic.messages.create
(callWithRetry/anthropic block) or invoking the ElevenLabs TTS fetch block;
update the handler in this file (around the code that constructs messages, the
callWithRetry(...) anthropic call, and the TTS block) to check the flag before
building messages/trek context and before the ElevenLabs section so those
expensive calls are skipped when only the transcript is needed.
- Around line 107-116: The trek lookup uses the service-role Supabase client and
only filters by trek_id, allowing any caller to inject arbitrary trek data into
contextBlock; fix it by enforcing authorization: retrieve the caller's
authenticated user (e.g., via supabase.auth.getUser() or using an authenticated
client) and modify the query to ensure the trek belongs to or is visible to that
user (for example add .eq('owner_id', user.id) or .or('is_public.eq.true') to
the supabase.from("treks").select(...).eq("id", trek_id) call) before setting
contextBlock so only authorized trek data is included.
In `@supabase/functions/skill-refresh/index.ts`:
- Around line 124-139: Ensure the parsed model payload is an object before
mutating it: after JSON.parse(text) (the local variable result) check that
typeof result === "object" && result !== null && !Array.isArray(result); if the
check fails, log the offending payload (use text.slice(0,300) or the parsed
value) and return the same 500 Response used for parse failures instead of
proceeding to set result.exercises = ..., to avoid treating arrays or scalars as
valid objects.
- Around line 141-146: The update of last_refreshed_at via
supabase.from("trek_notebook").update(...) currently ignores the response and
lets the function return exercises even on failure; modify the code to check the
Supabase response (error and status) after the update of last_refreshed_at (and
use the same notebook_entry_id and authUserId used in the .eq calls), and if an
error is returned, log/record the error and stop the flow (return an error
response or throw) so exercises are not returned when the persistence fails;
ensure the error branch returns a clear failure to the caller and includes the
error details in the log/response.
---
Nitpick comments:
In `@src/components/summit/SkillRefresh.jsx`:
- Around line 145-164: The Next/Finish button is shown when
answers[currentIndex] !== undefined which treats an empty string as valid;
change the visibility check to require a non-empty, non-whitespace answer (e.g.
test answers[currentIndex] exists and
answers[currentIndex].toString().trim().length > 0) so blank submissions are
disabled; update the conditional that renders the button (and any related logic
in handleNext/onClose if they assume non-empty) and reference answers,
currentIndex, handleAnswer, handleNext, onClose, currentExercise.type and
exercises to locate the code to modify.
In `@src/components/trek/FiresideMode.jsx`:
- Around line 20-25: The cleanup effect that watches open and recorderRef
currently only stops the recorder but doesn't clear conversation state; update
the effect (the useEffect that checks if (!open && recorderRef?.state ===
'recording')) to also reset component state on panel close by calling the
relevant setters such as setMessages([]), setError(null) and any other
session-specific setters you have (e.g., setIsRecording(false),
setConversationId(null) or similar) so that reopening FiresideMode starts a
fresh session; keep the recorder stop behavior and ensure these resets only run
when open becomes false.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4aed1183-1bfe-4d5b-a839-6dd9b4643605
📒 Files selected for processing (13)
src/components/ledges/FileUploadLedge.jsxsrc/components/ledges/VoiceResponseLedge.jsxsrc/components/summit/SkillRefresh.jsxsrc/components/summit/SummitCard.jsxsrc/components/trek/CoachingPanel.jsxsrc/components/trek/FiresideMode.jsxsrc/lib/sherpa.jssrc/lib/voice.jssrc/views/LearningView.jsxsrc/views/TrekNotebookView.jsxsupabase/functions/screen-analyze/index.tssupabase/functions/sherpa-voice/index.tssupabase/functions/skill-refresh/index.ts
| /** | ||
| * File upload exercise: upload a creative deliverable for AI evaluation. | ||
| */ | ||
| export default function FileUploadLedge({ spec, onSubmit }) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how FileUploadLedge is used across the codebase
rg -n "FileUploadLedge" --type=js --type=jsx -A 2 -B 2Repository: zackfunprojects/altius
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Find FileUploadLedge and ExerciseWrapper files
fd -t f "FileUploadLedge" || fd -t f "ExerciseWrapper"Repository: zackfunprojects/altius
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Search for FileUploadLedge usage without specifying jsx type
rg -n "FileUploadLedge" -A 2 -B 2Repository: zackfunprojects/altius
Length of output: 2462
🏁 Script executed:
#!/bin/bash
# Search for ExerciseWrapper to find the file
fd -e js -e jsx | xargs grep -l "ExerciseWrapper" | head -5Repository: zackfunprojects/altius
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Read FileUploadLedge to see onSubmit usage
cat -n src/components/ledges/FileUploadLedge.jsxRepository: zackfunprojects/altius
Length of output: 5999
🏁 Script executed:
#!/bin/bash
# Read ExerciseWrapper to see how FileUploadLedge is used and handleResponseChange
sed -n '160,180p' src/components/exercises/ExerciseWrapper.jsxRepository: zackfunprojects/altius
Length of output: 764
🏁 Script executed:
#!/bin/bash
# Check if there are any other usages of FileUploadLedge
rg "FileUploadLedge" -cRepository: zackfunprojects/altius
Length of output: 201
Prop mismatch: ExerciseWrapper passes onResponseChange, but this component expects onSubmit.
FileUploadLedge is destructuring { spec, onSubmit } on line 11 and calling onSubmit(...) on line 64. However, ExerciseWrapper passes onResponseChange instead (line 172 of ExerciseWrapper.jsx), causing the submit functionality to fail with an undefined function error at runtime.
Align the component signature with other ledges (ConversationSimLedge, VoiceResponseLedge) that all accept onResponseChange:
-export default function FileUploadLedge({ spec, onSubmit }) {
+export default function FileUploadLedge({ spec, onResponseChange }) {Then update the handleSubmit call:
const handleSubmit = useCallback(() => {
if (!uploadedUrl) return
- onSubmit({
+ onResponseChange({
file_url: uploadedUrl,
file_name: file?.name,
file_type: file?.type,
type: 'file_upload',
})
- }, [uploadedUrl, file, onSubmit])
+ }, [uploadedUrl, file, onResponseChange])Also note: the disabled prop is being passed but not used in the component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ledges/FileUploadLedge.jsx` at line 11, FileUploadLedge
currently destructures { spec, onSubmit } and calls onSubmit(...) which
mismatches ExerciseWrapper (which passes onResponseChange) and causes runtime
undefined; change the prop name to onResponseChange in the component signature
and all internal uses (replace onSubmit references, including the call in
handleSubmit) to onResponseChange to match other ledges (ConversationSimLedge,
VoiceResponseLedge), and also accept and use the disabled prop (e.g., prevent
submission or disable inputs when disabled) so the passed disabled flag is
honored.
Comment on lines
+40
to
+42
| try { | ||
| const ext = file.name.split('.').pop() || 'bin' | ||
| const path = `exercise-files/${user.id}/${Date.now()}.${ext}` |
There was a problem hiding this comment.
Edge case: Extension extraction fails for files without extensions.
If a file has no extension (e.g., "Makefile" or "README"), split('.').pop() returns the entire filename, creating a path like 1713100000000.Makefile. Consider a more robust extraction.
🛡️ Proposed fix
- const ext = file.name.split('.').pop() || 'bin'
+ const parts = file.name.split('.')
+ const ext = parts.length > 1 ? parts.pop() : 'bin'
const path = `exercise-files/${user.id}/${Date.now()}.${ext}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ledges/FileUploadLedge.jsx` around lines 40 - 42, The ext
extraction in FileUploadLedge.jsx currently uses file.name.split('.').pop()
which returns the whole filename for names without extensions (e.g., "Makefile")
or mishandles dotfiles; update the logic inside the try block where ext and path
are created so ext is derived only when there is a true extension (use
file.name.lastIndexOf('.') or a /\.([^.]+)$/ regex to capture the suffix) and
fall back to a safe default like 'bin' otherwise, then build path using that ext
(the existing path template using user.id and Date.now. should remain
unchanged).
Comment on lines
+25
to
+31
| const stream = await navigator.mediaDevices.getDisplayMedia({ | ||
| video: { mediaSource: 'screen' }, | ||
| }) | ||
|
|
||
| const track = stream.getVideoTracks()[0] | ||
| const imageCapture = new ImageCapture(track) | ||
| const bitmap = await imageCapture.grabFrame() |
There was a problem hiding this comment.
ImageCapture API has limited browser support and may throw in Firefox.
The ImageCapture API is not supported in Firefox and some other browsers. This will cause an unhandled error when a Firefox user (even with Pro) attempts to use the feature. Consider adding feature detection or a try-catch specifically around ImageCapture usage, with a user-friendly error message.
🛡️ Proposed fix to add feature detection
const stream = await navigator.mediaDevices.getDisplayMedia({
- video: { mediaSource: 'screen' },
+ video: true,
})
const track = stream.getVideoTracks()[0]
+
+ // Check for ImageCapture support (not available in Firefox)
+ if (typeof ImageCapture === 'undefined') {
+ stream.getTracks().forEach((t) => t.stop())
+ throw new Error('Screen capture analysis is not supported in this browser.')
+ }
+
const imageCapture = new ImageCapture(track)Does Firefox support the ImageCapture API?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/trek/CoachingPanel.jsx` around lines 25 - 31, The ImageCapture
usage can throw in browsers without support (e.g., Firefox); update the section
that calls navigator.mediaDevices.getDisplayMedia, extracts track =
stream.getVideoTracks()[0], creates new ImageCapture(track) and calls
imageCapture.grabFrame() to first feature-detect support and wrap the
ImageCapture creation/grabFrame call in a try-catch; on failure fall back to
capturing a frame from a hidden video element/canvas using the MediaStream (or
show a clear user-facing error) and ensure the track is stopped on error so
resources are released.
Comment on lines
+97
to
+99
| <p className="font-mono text-phosphor-green/40 text-xs"> | ||
| The Sherpa watches your screen in real tools and coaches live. | ||
| </p> |
There was a problem hiding this comment.
Typo: "real tools" should be "real time".
Minor copy error in the Pro gate message.
📝 Proposed fix
<p className="font-mono text-phosphor-green/40 text-xs">
- The Sherpa watches your screen in real tools and coaches live.
+ The Sherpa watches your screen in real time and coaches live.
</p>📝 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.
Suggested change
| <p className="font-mono text-phosphor-green/40 text-xs"> | |
| The Sherpa watches your screen in real tools and coaches live. | |
| </p> | |
| <p className="font-mono text-phosphor-green/40 text-xs"> | |
| The Sherpa watches your screen in real time and coaches live. | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/trek/CoachingPanel.jsx` around lines 97 - 99, In
CoachingPanel.jsx update the copy inside the paragraph element (the <p
className="font-mono text-phosphor-green/40 text-xs"> node) used in the Pro gate
message: replace the incorrect phrase "real tools" with "real time" so the
sentence reads "The Sherpa watches your screen in real time and coaches live."
Keep everything else (className and markup) unchanged.
Comment on lines
+183
to
+185
| <p className="font-ui text-catalog-cream/40 text-xs"> | ||
| {recording ? 'Release to send' : processing ? 'Processing...' : 'Hold to speak'} | ||
| </p> |
There was a problem hiding this comment.
UI text "Hold to speak" is misleading - this is click-to-toggle, not press-and-hold.
The button uses onClick handlers for start/stop, meaning users click once to start and click again to stop. The text "Hold to speak" implies press-and-hold behavior which doesn't match the implementation. This could confuse users.
📝 Proposed fix
<p className="font-ui text-catalog-cream/40 text-xs">
- {recording ? 'Release to send' : processing ? 'Processing...' : 'Hold to speak'}
+ {recording ? 'Tap to send' : processing ? 'Processing...' : 'Tap to speak'}
</p>📝 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.
Suggested change
| <p className="font-ui text-catalog-cream/40 text-xs"> | |
| {recording ? 'Release to send' : processing ? 'Processing...' : 'Hold to speak'} | |
| </p> | |
| <p className="font-ui text-catalog-cream/40 text-xs"> | |
| {recording ? 'Tap to send' : processing ? 'Processing...' : 'Tap to speak'} | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/trek/FiresideMode.jsx` around lines 183 - 185, The UI copy in
FiresideMode.jsx is incorrect: the paragraph using the conditional expression
with recording and processing currently shows "Hold to speak" but the component
uses click-to-toggle start/stop handlers (not press-and-hold). Update the text
to reflect click/tap behavior (for example "Click to speak" or "Tap to speak" as
appropriate) in the JSX where the expression {recording ? 'Release to send' :
processing ? 'Processing...' : 'Hold to speak'} is defined so the default branch
matches the actual toggle interaction; keep the existing conditional logic but
replace the "Hold to speak" string with the correct wording.
Comment on lines
+137
to
+138
| } catch { | ||
| console.error("Failed to parse screen analysis:", text.slice(0, 300)); |
There was a problem hiding this comment.
Keep screen-derived model output out of logs.
On parse failures this writes part of Claude’s response to server logs. Because that response is based on a screenshot, it can easily echo secrets or PII visible on the user’s screen.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/screen-analyze/index.ts` around lines 137 - 138, The catch
block in supabase/functions/screen-analyze/index.ts currently logs part of
Claude’s response via console.error("Failed to parse screen analysis:",
text.slice(0, 300)); remove any direct logging of the variable text (which may
contain PII) and instead log only a safe diagnostic such as the caught error
object/message and a non-sensitive metric (e.g., text length or "redacted
response"); update the catch to accept the error parameter and call
console.error with a generic message and error (but not the response content) so
you no longer write screen-derived model output to logs.
Comment on lines
+91
to
+171
| // Step 2: Get Sherpa response via Claude | ||
| // Build context messages from conversation history | ||
| const messages: Array<{ role: string; content: string }> = []; | ||
|
|
||
| if (Array.isArray(conversation_history)) { | ||
| for (const msg of conversation_history.slice(-10)) { | ||
| if (msg.role && msg.content) { | ||
| messages.push({ role: msg.role, content: msg.content.slice(0, 2000) }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| messages.push({ role: "user", content: transcript }); | ||
|
|
||
| // Fetch trek context for Sherpa | ||
| let contextBlock = ""; | ||
| if (trek_id) { | ||
| const { data: trek } = await supabase | ||
| .from("treks") | ||
| .select("trek_name, skill_description, difficulty") | ||
| .eq("id", trek_id) | ||
| .single(); | ||
|
|
||
| if (trek) { | ||
| contextBlock = `\n\nTrek context: ${trek.trek_name} (${trek.difficulty}) - ${trek.skill_description}`; | ||
| } | ||
| } | ||
|
|
||
| const sherpaResponse = await callWithRetry(() => | ||
| anthropic.messages.create({ | ||
| model: "claude-sonnet-4-20250514", | ||
| max_tokens: 512, | ||
| system: `${SHERPA_SYSTEM_PROMPT} | ||
|
|
||
| You are in Fireside Lesson mode - a spoken conversation with the climber. Keep responses concise and conversational (2-4 sentences). Speak naturally as you would around a campfire. Do not use markdown formatting, bullet points, or code blocks - this will be read aloud.${contextBlock}`, | ||
| messages: messages as Array<{ role: "user" | "assistant"; content: string }>, | ||
| }) | ||
| ); | ||
|
|
||
| const responseText = | ||
| sherpaResponse.content?.[0]?.type === "text" ? sherpaResponse.content[0].text : ""; | ||
|
|
||
| // Step 3: Synthesize speech via ElevenLabs (optional) | ||
| let audioResponseBase64 = null; | ||
|
|
||
| if (elevenlabsApiKey && responseText) { | ||
| try { | ||
| const ttsResponse = await fetch( | ||
| `https://api.elevenlabs.io/v1/text-to-speech/${elevenlabsVoiceId}`, | ||
| { | ||
| method: "POST", | ||
| headers: { | ||
| "xi-api-key": elevenlabsApiKey, | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ | ||
| text: responseText, | ||
| model_id: "eleven_monolingual_v1", | ||
| voice_settings: { | ||
| stability: 0.6, | ||
| similarity_boost: 0.75, | ||
| }, | ||
| }), | ||
| } | ||
| ); | ||
|
|
||
| if (ttsResponse.ok) { | ||
| const audioBuffer = await ttsResponse.arrayBuffer(); | ||
| const bytes = new Uint8Array(audioBuffer); | ||
| let binary = ""; | ||
| for (let i = 0; i < bytes.length; i++) { | ||
| binary += String.fromCharCode(bytes[i]); | ||
| } | ||
| audioResponseBase64 = btoa(binary); | ||
| } else { | ||
| console.error("ElevenLabs TTS error:", await ttsResponse.text()); | ||
| } | ||
| } catch (ttsErr) { | ||
| console.error("TTS synthesis failed:", (ttsErr as Error).message); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add a transcript-only path for voice-response uploads.
src/components/ledges/VoiceResponseLedge.jsx calls this endpoint just to get transcript, but this handler always continues on to Claude and may also hit ElevenLabs. That makes every voice-response submission slower and more expensive than necessary.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/sherpa-voice/index.ts` around lines 91 - 171, Add a
transcript-only short-circuit: read a boolean flag from the incoming request
body (e.g., transcriptOnly or transcript_only) and if true immediately perform
the minimal transcript upload/DB update and return the transcript JSON response
without calling anthropic.messages.create (callWithRetry/anthropic block) or
invoking the ElevenLabs TTS fetch block; update the handler in this file (around
the code that constructs messages, the callWithRetry(...) anthropic call, and
the TTS block) to check the flag before building messages/trek context and
before the ElevenLabs section so those expensive calls are skipped when only the
transcript is needed.
Comment on lines
+107
to
+116
| if (trek_id) { | ||
| const { data: trek } = await supabase | ||
| .from("treks") | ||
| .select("trek_name, skill_description, difficulty") | ||
| .eq("id", trek_id) | ||
| .single(); | ||
|
|
||
| if (trek) { | ||
| contextBlock = `\n\nTrek context: ${trek.trek_name} (${trek.difficulty}) - ${trek.skill_description}`; | ||
| } |
There was a problem hiding this comment.
Authorize the trek lookup before injecting it into the prompt.
This uses the service-role client and filters only by id. Any authenticated caller who knows another trek UUID can load that trek’s name, skill description, and difficulty into Claude’s context.
🔒 Suggested fix
- const { data: trek } = await supabase
+ const { data: trek, error: trekError } = await supabase
.from("treks")
.select("trek_name, skill_description, difficulty")
.eq("id", trek_id)
+ .eq("user_id", authUserId)
.single();
- if (trek) {
+ if (trekError) {
+ return new Response(
+ JSON.stringify({ error: "Access denied" }),
+ { status: 403, headers: { ...cors, "Content-Type": "application/json" } }
+ );
+ }
+
+ if (trek) {
contextBlock = `\n\nTrek context: ${trek.trek_name} (${trek.difficulty}) - ${trek.skill_description}`;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/sherpa-voice/index.ts` around lines 107 - 116, The trek
lookup uses the service-role Supabase client and only filters by trek_id,
allowing any caller to inject arbitrary trek data into contextBlock; fix it by
enforcing authorization: retrieve the caller's authenticated user (e.g., via
supabase.auth.getUser() or using an authenticated client) and modify the query
to ensure the trek belongs to or is visible to that user (for example add
.eq('owner_id', user.id) or .or('is_public.eq.true') to the
supabase.from("treks").select(...).eq("id", trek_id) call) before setting
contextBlock so only authorized trek data is included.
Comment on lines
+124
to
+139
| let result; | ||
| try { | ||
| const cleaned = text | ||
| .replace(/```json?\s*/g, "") | ||
| .replace(/```\s*/g, "") | ||
| .trim(); | ||
| result = JSON.parse(cleaned); | ||
| } catch { | ||
| console.error("Failed to parse skill refresh:", text.slice(0, 300)); | ||
| return new Response( | ||
| JSON.stringify({ error: "Failed to generate refresh exercises. Please try again." }), | ||
| { status: 500, headers: { ...cors, "Content-Type": "application/json" } } | ||
| ); | ||
| } | ||
|
|
||
| result.exercises = Array.isArray(result.exercises) ? result.exercises : []; |
There was a problem hiding this comment.
Reject non-object model payloads before mutating result.
A bare array or scalar is still valid JSON. Here that either returns the wrong wire shape or throws at result.exercises = ..., which turns schema drift into a generic 500.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/skill-refresh/index.ts` around lines 124 - 139, Ensure the
parsed model payload is an object before mutating it: after JSON.parse(text)
(the local variable result) check that typeof result === "object" && result !==
null && !Array.isArray(result); if the check fails, log the offending payload
(use text.slice(0,300) or the parsed value) and return the same 500 Response
used for parse failures instead of proceeding to set result.exercises = ..., to
avoid treating arrays or scalars as valid objects.
Comment on lines
+141
to
+146
| // Update last_refreshed_at | ||
| await supabase | ||
| .from("trek_notebook") | ||
| .update({ last_refreshed_at: new Date().toISOString() }) | ||
| .eq("id", notebook_entry_id) | ||
| .eq("user_id", authUserId); |
There was a problem hiding this comment.
Don’t ignore failures when persisting last_refreshed_at.
The function returns exercises even if this update fails, which silently breaks the spacing signal the feature depends on.
🛠️ Suggested fix
- await supabase
+ const { error: updateError } = await supabase
.from("trek_notebook")
.update({ last_refreshed_at: new Date().toISOString() })
.eq("id", notebook_entry_id)
.eq("user_id", authUserId);
+
+ if (updateError) {
+ console.error("Failed to update last_refreshed_at:", updateError.message);
+ return new Response(
+ JSON.stringify({ error: "Failed to save refresh state. Please try again." }),
+ { status: 500, headers: { ...cors, "Content-Type": "application/json" } }
+ );
+ }📝 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.
Suggested change
| // Update last_refreshed_at | |
| await supabase | |
| .from("trek_notebook") | |
| .update({ last_refreshed_at: new Date().toISOString() }) | |
| .eq("id", notebook_entry_id) | |
| .eq("user_id", authUserId); | |
| // Update last_refreshed_at | |
| const { error: updateError } = await supabase | |
| .from("trek_notebook") | |
| .update({ last_refreshed_at: new Date().toISOString() }) | |
| .eq("id", notebook_entry_id) | |
| .eq("user_id", authUserId); | |
| if (updateError) { | |
| console.error("Failed to update last_refreshed_at:", updateError.message); | |
| return new Response( | |
| JSON.stringify({ error: "Failed to save refresh state. Please try again." }), | |
| { status: 500, headers: { ...cors, "Content-Type": "application/json" } } | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/skill-refresh/index.ts` around lines 141 - 146, The update
of last_refreshed_at via supabase.from("trek_notebook").update(...) currently
ignores the response and lets the function return exercises even on failure;
modify the code to check the Supabase response (error and status) after the
update of last_refreshed_at (and use the same notebook_entry_id and authUserId
used in the .eq calls), and if an error is returned, log/record the error and
stop the flow (return an error response or throw) so exercises are not returned
when the persistence fails; ensure the error branch returns a clear failure to
the caller and includes the error details in the log/response.
Edge Functions: - screen-analyze: validate JSON shape (reject arrays/scalars), remove PII from logs - sherpa-voice: add trek ownership check (.eq user_id), add transcribe_only mode for VoiceResponseLedge (skips Claude + TTS), accept transcribe_only param - skill-refresh: validate JSON shape, handle last_refreshed_at update error Frontend: - voice.js: broader MIME type support (mp4, ogg fallbacks), safer playback error handling - CoachingPanel: fallback for Firefox (no ImageCapture API), fix "real tools" -> "real time" typo - FiresideMode: fix "Hold to speak" -> "Click to speak" (matches click-to-toggle behavior) - FileUploadLedge: robust extension extraction for extensionless files - VoiceResponseLedge: use transcribe_only mode (faster, cheaper) - SkillRefresh: require non-empty short answer before allowing Next - sherpa.js: pass transcribeOnly param through voiceChat wrapper Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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
New Files
supabase/functions/screen-analyze/index.tssupabase/functions/sherpa-voice/index.tssupabase/functions/skill-refresh/index.tssrc/lib/voice.jssrc/components/trek/CoachingPanel.jsxsrc/components/trek/FiresideMode.jsxsrc/components/summit/SkillRefresh.jsxExternal API Requirements
OPENAI_API_KEYin Supabase secrets (Whisper STT)ELEVENLABS_API_KEY+ELEVENLABS_VOICE_IDin Supabase secrets (TTS)Test plan
pnpm buildandpnpm lintpass clean🤖 Generated with Claude Code
Summary by CodeRabbit