Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughThis PR introduces benchmark creation functionality that allows superusers to save benchmark configurations from scribe data. It adds a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds functionality for users to create benchmarks from completed scribes, allowing them to save reference data for evaluating future transcriptions. The feature is restricted to superusers and stores benchmark data in local storage.
Key Changes
- Added
CreatedBenchmarkinterface and storage for user-created benchmarks - Created new
CreateBenchmarkcomponent with dialog UI for benchmark creation - Updated benchmark page button labels from "New Benchmark" to "Run Benchmark"
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/Benchmark.tsx | Added CreatedBenchmark interface and updated button labels to clarify "Run Benchmark" action |
| src/hooks/useStorage.ts | Added storage key for persisting created benchmarks in local storage |
| src/components/CreateBenchmark.tsx | New component implementing benchmark creation dialog with superuser restriction |
| src/components/Controller.tsx | Integrated CreateBenchmark component and tracked last scribe state |
| public/locale/en.json | Added translation keys for benchmark creation feature |
src/pages/Benchmark.tsx
Outdated
| id: string; | ||
| createdAt: Date; | ||
| questionnaireIds: string[]; | ||
| expectedResult: any; |
There was a problem hiding this comment.
The expectedResult field uses the any type, which defeats TypeScript's type safety. Consider defining a proper interface for the expected result structure, or at minimum use unknown if the structure is truly dynamic and add runtime validation when accessing this data.
| expectedResult: any; | |
| expectedResult: unknown; |
| return; | ||
| } | ||
|
|
||
| const form: any = formState ? formState : []; |
There was a problem hiding this comment.
Using any type for form bypasses type safety. Define a proper interface for formState in the component props and use that type here, or at minimum use unknown with type guards to safely access the data.
src/components/CreateBenchmark.tsx
Outdated
| } | ||
|
|
||
| const form: any = formState ? formState : []; | ||
| const qIds = form.map((f: any) => f.questionnaire.id); |
There was a problem hiding this comment.
The inline any type in the map callback removes type safety. If a proper interface is defined for the form items, this callback parameter would be automatically typed, eliminating the need for explicit type annotation and preventing potential runtime errors from accessing undefined properties.
|
|
||
| const handleCreateBenchmark = async () => { | ||
| if (!scribe) { | ||
| alert(t("no_scribe_to_benchmark")); |
There was a problem hiding this comment.
Using browser alert() provides poor user experience and is inconsistent with the modern UI components used elsewhere in the codebase. Consider using a toast notification or the Dialog component to display this error message.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useStorage.ts (1)
1-11: Remove type imports from page module to break circular dependencyA circular dependency exists:
useStorage.tsimportsBenchmarkandCreatedBenchmarktypes from@/pages/Benchmark.tsx, whileBenchmark.tsximports theuseStoragehook. This can cause module resolution issues and makes the codebase fragile.Move
BenchmarkandCreatedBenchmarktype definitions to a shared types module (e.g.,src/types/benchmark.ts), or useimport typesyntax inuseStorage.tsto ensure types are stripped at runtime and break the circular dependency chain.
🧹 Nitpick comments (3)
src/pages/Benchmark.tsx (1)
383-391: "Run Benchmark" label change is clear; consider i18n later if copy needs to be configurableThe updated wording (“Run Benchmark” for the trigger and sheet title) is clearer and consistent. If this copy will need localization like the page header (
t("benchmark")), you could eventually move these strings into the locale file as well; not blocking for now.src/components/Controller.tsx (1)
102-103: Confirm that persistinglastScribeacross cancels is intendedThe
lastScribestate is updated wheneverscribechanges and is never cleared whenhandleCancelruns, so:
- After using a scribe and then cancelling,
lastScriberemains set.- The
CreateBenchmarkbutton continues to use that last scribe even when the main controller state is reset.If the desired behavior is to always allow creating a benchmark from the last used scribe, this is perfect. If instead you want benchmark creation tied only to the currently active scribe session, consider clearing
lastScribeinsidehandleCancel.Also applies to: 420-424, 429-431
src/components/CreateBenchmark.tsx (1)
25-40: Benchmark creation logic matches current needs; consider tightening formState typing laterThe creation flow (collect form questionnaires, build a
CreatedBenchmark, append viauseStorage) is straightforward and matches theCreatedBenchmarkinterface.Two observations you might consider:
formStateis typed asunknownat the prop level (currently narrowed locally via ananycast). Defining a more specific shape for this prop would enable better type-checking when accessingf.questionnaire.idinstead of relying onany.- The
if (!scribe) alert(...)branch is unreachable—scribeis typed as the non-optionalScribeModelprop, so the condition will never be true. If you want to keep a guard, either change the prop type to allownullor remove the check.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
public/locale/en.json(1 hunks)src/components/Controller.tsx(3 hunks)src/components/CreateBenchmark.tsx(1 hunks)src/hooks/useStorage.ts(2 hunks)src/pages/Benchmark.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/hooks/useStorage.ts (1)
src/pages/Benchmark.tsx (1)
CreatedBenchmark(84-89)
src/components/CreateBenchmark.tsx (6)
src/types.ts (1)
ScribeModel(71-106)src/utils/constants.ts (1)
I18NNAMESPACE(1-1)src/hooks/useContainerRef.tsx (1)
useContainerRef(6-13)src/hooks/useStorage.ts (1)
useStorage(16-79)src/hooks/useAuthUser.ts (1)
useAuthUser(4-11)src/pages/Benchmark.tsx (1)
CreatedBenchmark(84-89)
src/components/Controller.tsx (2)
src/types.ts (1)
ScribeModel(71-106)src/components/CreateBenchmark.tsx (1)
CreateBenchmark(11-58)
🪛 GitHub Actions: Build and Deploy
src/components/Controller.tsx
[error] 54-54: TS6133: 'useAuthUser' is declared but its value is never read.
🪛 GitHub Check: build
src/components/CreateBenchmark.tsx
[failure] 17-17:
'createdBenchmarks' is declared but its value is never read.
src/components/Controller.tsx
[failure] 54-54:
'useAuthUser' is declared but its value is never read.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
public/locale/en.json (1)
124-127: New benchmark-related i18n keys look consistentKeys and copy are clear and match the CreateBenchmark UI usage; no issues from my side.
src/pages/Benchmark.tsx (1)
84-89: CreatedBenchmark interface aligns with current usage, with room to tighten typing laterThe shape (
id,createdAt,questionnaireIds,expectedResult) matches howCreateBenchmarkconstructs objects. TheexpectedResultfield currently receives the form object and is typed asanydue to upstream typing; you might narrow this away fromanyonce the consumer logic for stored benchmarks is finalized, but it's appropriate for this iteration.
| import useAuthUser from "@/hooks/useAuthUser"; | ||
| import CreateBenchmark from "./CreateBenchmark"; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "Controller.tsx" -type f 2>/dev/null | head -5Repository: ohcnetwork/care_scribe_fe
Length of output: 51
🏁 Script executed:
git ls-files | grep -i "controller.tsx"Repository: ohcnetwork/care_scribe_fe
Length of output: 98
🏁 Script executed:
rg -n "useAuthUser" --type tsxRepository: ohcnetwork/care_scribe_fe
Length of output: 96
🏁 Script executed:
cat -n src/components/Controller.tsx | sed -n '50,80p'Repository: ohcnetwork/care_scribe_fe
Length of output: 1441
🏁 Script executed:
rg "useAuthUser" src/components/Controller.tsxRepository: ohcnetwork/care_scribe_fe
Length of output: 115
🏁 Script executed:
wc -l src/components/Controller.tsxRepository: ohcnetwork/care_scribe_fe
Length of output: 102
🏁 Script executed:
cat -n src/components/Controller.tsx | grep -E "(useAuthUser|lastScribe|CreateBenchmark)"Repository: ohcnetwork/care_scribe_fe
Length of output: 372
🏁 Script executed:
cat tsconfig.json | head -50Repository: ohcnetwork/care_scribe_fe
Length of output: 281
🏁 Script executed:
cat -n src/components/CreateBenchmark.tsx | head -60Repository: ohcnetwork/care_scribe_fe
Length of output: 2307
🏁 Script executed:
cat tsconfig.app.json | grep -A 5 "noUnusedLocals"Repository: ohcnetwork/care_scribe_fe
Length of output: 245
Remove unused useAuthUser import to fix TS6133 build error
useAuthUser is imported but never used in this file. Since noUnusedLocals is enabled in tsconfig.app.json, this triggers TS6133. The auth gating already happens inside CreateBenchmark (line 42), so this import can be safely removed.
Apply this diff:
-import { cleanAIResponse, poller } from "@/utils/response-utils";
-import {
- getHydratedFields, getQuestionInputs
-} from "@/utils/field-utils";
-import { uploadScribeFile } from "@/utils/upload-utils";
-import useAuthUser from "@/hooks/useAuthUser";
-import CreateBenchmark from "./CreateBenchmark";
+import { cleanAIResponse, poller } from "@/utils/response-utils";
+import { getHydratedFields, getQuestionInputs } from "@/utils/field-utils";
+import { uploadScribeFile } from "@/utils/upload-utils";
+import CreateBenchmark from "./CreateBenchmark";📝 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.
| import useAuthUser from "@/hooks/useAuthUser"; | |
| import CreateBenchmark from "./CreateBenchmark"; | |
| import CreateBenchmark from "./CreateBenchmark"; |
🧰 Tools
🪛 GitHub Actions: Build and Deploy
[error] 54-54: TS6133: 'useAuthUser' is declared but its value is never read.
🪛 GitHub Check: build
[failure] 54-54:
'useAuthUser' is declared but its value is never read.
🤖 Prompt for AI Agents
In src/components/Controller.tsx around lines 54 to 56, the import of
useAuthUser is unused and causes a TS6133 build error; remove the unused import
line (import useAuthUser from "@/hooks/useAuthUser";) from the top of the file
so only the necessary imports remain (keeping CreateBenchmark import intact),
then run TypeScript/tsc or the project build to verify the error is resolved.
| const [createdBenchmarks, setCreatedBenchmarks] = useStorage( | ||
| "scribe-created-benchmarks", | ||
| ); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and search for all references to createdBenchmarks
rg -n "createdBenchmarks" src/components/CreateBenchmark.tsxRepository: ohcnetwork/care_scribe_fe
Length of output: 135
🏁 Script executed:
# Also read the relevant section of the file to understand the context
cat -n src/components/CreateBenchmark.tsx | head -50Repository: ohcnetwork/care_scribe_fe
Length of output: 2017
Fix unused createdBenchmarks binding to satisfy TS6133
createdBenchmarks is never read, triggering TS6133 given noUnusedLocals is enabled. Since only the setter is used, discard the first tuple element:
- const [createdBenchmarks, setCreatedBenchmarks] = useStorage(
- "scribe-created-benchmarks",
- );
+ const [, setCreatedBenchmarks] = useStorage("scribe-created-benchmarks");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [createdBenchmarks, setCreatedBenchmarks] = useStorage( | |
| "scribe-created-benchmarks", | |
| ); | |
| const [, setCreatedBenchmarks] = useStorage("scribe-created-benchmarks"); |
🧰 Tools
🪛 GitHub Check: build
[failure] 17-17:
'createdBenchmarks' is declared but its value is never read.
🤖 Prompt for AI Agents
In src/components/CreateBenchmark.tsx around lines 17 to 20, the
createdBenchmarks variable is never read (TS6133) — change the useStorage
destructuring to discard the first tuple element and only keep the setter (use
an array destructuring pattern with a leading comma so only setCreatedBenchmarks
is bound), then remove any unused createdBenchmarks references if present.
Deploying care-scribe-fe with
|
| Latest commit: |
10121b1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6aa3082c.care-scribe-fe.pages.dev |
| Branch Preview URL: | https://create-benchmark.care-scribe-fe.pages.dev |
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.