Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a full Blazor web application with audio capture, transcription (OpenRouter/LmStudio), LLM-driven concern extraction/refinement and follow-ups, FHIR observation mapping and bundle upload, SQLite persistence, AI telemetry, Tailwind frontend pipeline, Docker/CICD workflows, and unit tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Server as ASP.NET Server
participant AudioSvc as AudioProcessingService
participant Transcriber as OpenRouter/LmStudio
participant LLM as TextCompletionService
participant DB as SQLite
participant FHIR as FHIR Server
Browser->>Server: POST /api/audio/process (multipart audio + subject)
Server->>AudioSvc: ProcessAsync(AudioInput, ProcessingContext)
AudioSvc->>Transcriber: Transcribe audio
Transcriber-->>AudioSvc: Transcript
AudioSvc->>LLM: Extract concerns from transcript
LLM-->>AudioSvc: Concerns
AudioSvc->>LLM: (optional) Generate follow-ups / apply answers
LLM-->>AudioSvc: Refined concerns
AudioSvc->>AudioSvc: Map concerns -> FHIR Observations
AudioSvc->>AudioSvc: Build FHIR Bundle
AudioSvc->>DB: Persist ConcernRecord
AudioSvc-->>Server: ProcessingResult (transcript, concerns, bundle)
Server-->>Browser: ProcessingResponse (JSON)
alt User uploads bundle
Browser->>Server: POST bundle upload
Server->>FHIR: POST Bundle (auth if configured)
FHIR-->>Server: Response
Server-->>Browser: FhirUploadResult
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (7)
✏️ Tip: You can disable this entire section by setting Comment |
Summary of ChangesHello @venikman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes the initial version of an application designed to streamline the process of capturing and managing patient concerns. It focuses on converting audio input into structured data, leveraging AI for transcription and concern extraction, and then formatting this information into FHIR R4 Observation bundles. The setup includes robust infrastructure for deployment and monitoring, laying the groundwork for a scalable and maintainable healthcare solution. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This pull request adds the initial application infrastructure including a FAST UI, telemetry, Dockerfile, and GitHub Actions CI/CD. The changes include comprehensive test coverage for service layer components and third-party Bootstrap frontend assets.
Changes:
- Added unit tests for service layer components (concern extraction, follow-up questions, FHIR mapping, refinement)
- Added test project configuration with MSTest and supporting libraries
- Added Bootstrap 5.3.3 static assets (minified JS and CSS files)
Reviewed changes
Copilot reviewed 82 out of 130 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/AudioSharp.App.Tests/Services/FollowUpQuestionServiceTests.cs |
Tests for follow-up question generation with JSON parsing and fallback scenarios |
tests/AudioSharp.App.Tests/Services/FhirObservationMapperTests.cs |
Tests for FHIR observation mapping with concern details |
tests/AudioSharp.App.Tests/Services/ConcernRefinementServiceTests.cs |
Tests for applying answers to concerns with JSON handling |
tests/AudioSharp.App.Tests/Services/ConcernExtractionServiceTests.cs |
Tests for extracting concerns from transcript text |
tests/AudioSharp.App.Tests/MSTestSettings.cs |
MSTest parallelization configuration |
tests/AudioSharp.App.Tests/AudioSharp.App.Tests.csproj |
Test project configuration targeting .NET 10.0 |
src/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.min.js.map |
Bootstrap JavaScript source map (minified third-party asset) |
src/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.min.js |
Bootstrap JavaScript library (minified third-party asset) |
src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-reboot.rtl.min.css |
Bootstrap RTL CSS (minified third-party asset) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private const int MaxEvents = 8; | ||
| private readonly object _sync = new(); | ||
| private UsageCounterData _audio = new(); | ||
| private UsageCounterData _text = new(); |
There was a problem hiding this comment.
Field '_text' can be 'readonly'.
| { | ||
| private const int MaxEvents = 8; | ||
| private readonly object _sync = new(); | ||
| private UsageCounterData _audio = new(); |
There was a problem hiding this comment.
Field '_audio' can be 'readonly'.
| if (result.Success) | ||
| { | ||
| FhirStatusMessage = string.IsNullOrWhiteSpace(result.Location) | ||
| ? "FHIR server accepted the bundle." | ||
| : $"FHIR server accepted the bundle: {result.Location}"; | ||
| } | ||
| else | ||
| { | ||
| FhirStatusMessage = $"FHIR server returned {result.StatusCode}."; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
| if (result.Success) | |
| { | |
| FhirStatusMessage = string.IsNullOrWhiteSpace(result.Location) | |
| ? "FHIR server accepted the bundle." | |
| : $"FHIR server accepted the bundle: {result.Location}"; | |
| } | |
| else | |
| { | |
| FhirStatusMessage = $"FHIR server returned {result.StatusCode}."; | |
| } | |
| FhirStatusMessage = result.Success | |
| ? (string.IsNullOrWhiteSpace(result.Location) | |
| ? "FHIR server accepted the bundle." | |
| : $"FHIR server accepted the bundle: {result.Location}") | |
| : $"FHIR server returned {result.StatusCode}."; |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the initial version of the AudioSharp application, a Blazor-based tool for capturing patient concerns from audio and converting them to FHIR resources. The submission includes the web application, data persistence using Entity Framework Core with SQLite, services for interacting with LLMs for transcription and data extraction, a Dockerfile for containerization, and initial unit tests. The overall structure is well-organized, and the use of modern .NET features is commendable. My review focuses on improving performance, maintainability, and aligning with modern web standards. I've identified a critical performance issue in data retrieval, a high-severity issue regarding a deprecated browser API, and a few medium-severity suggestions for better configuration management and production readiness.
| public async Task<IReadOnlyList<ConcernRecord>> GetRecentAsync(int count, CancellationToken cancellationToken) | ||
| { | ||
| var records = await _dbContext.ConcernRecords | ||
| .AsNoTracking() | ||
| .ToListAsync(cancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| return records | ||
| .OrderByDescending(record => record.CreatedAtUtc) | ||
| .Take(count) | ||
| .ToList(); | ||
| } |
There was a problem hiding this comment.
The current implementation of GetRecentAsync fetches all ConcernRecord entities from the database into memory before performing sorting and filtering. This will lead to significant performance degradation and high memory usage as the number of records grows. The filtering and ordering should be done at the database level to ensure efficiency.
public async Task<IReadOnlyList<ConcernRecord>> GetRecentAsync(int count, CancellationToken cancellationToken)
{
return await _dbContext.ConcernRecords
.AsNoTracking()
.OrderByDescending(record => record.CreatedAtUtc)
.Take(count)
.ToListAsync(cancellationToken)
.ConfigureAwait(false);
}| inputSampleRate = audioContext.sampleRate; | ||
|
|
||
| source = audioContext.createMediaStreamSource(stream); | ||
| processor = audioContext.createScriptProcessor(4096, options.channelCount, options.channelCount); |
There was a problem hiding this comment.
The createScriptProcessor API is deprecated and has been for a long time. It runs on the main UI thread, which can lead to audio glitches, stuttering, and an unresponsive UI, especially under load. The modern and recommended approach is to use AudioWorklet. Migrating to AudioWorklet will move audio processing to a separate thread, improving performance and reliability.
| COPY ["src/AudioSharp.App/AudioSharp.App.csproj", "src/AudioSharp.App/"] | ||
| RUN dotnet restore "src/AudioSharp.App/AudioSharp.App.csproj" | ||
|
|
||
| COPY . . | ||
| WORKDIR /src/src/AudioSharp.App |
There was a problem hiding this comment.
The current Dockerfile structure doesn't fully leverage Docker's layer caching. The COPY . . command on line 10 invalidates the cache for the dotnet restore layer whenever any file in the project changes, not just project dependencies. A more optimal pattern is to copy the solution (.slnx) and project (.csproj) files first, run dotnet restore, and then copy the rest of the source code. This ensures that the dependency layer is only rebuilt when project files change.
COPY ["AudioSharp.slnx", "."]
COPY ["src/AudioSharp.App/AudioSharp.App.csproj", "src/AudioSharp.App/"]
COPY ["tests/AudioSharp.App.Tests/AudioSharp.App.Tests.csproj", "tests/AudioSharp.App.Tests/"]
RUN dotnet restore "AudioSharp.slnx"
COPY . .
WORKDIR /src/src/AudioSharp.App
| } | ||
|
|
||
| @code { | ||
| private const long MaxAudioBytes = 25 * 1024 * 1024; |
There was a problem hiding this comment.
The MaxAudioBytes constant is hardcoded to 25MB. While this is a reasonable value, hardcoding configuration values like this makes the application harder to configure and maintain. This value should be moved to appsettings.json and injected as part of an options class, similar to how RecordingOptions is handled. This would allow for easier changes without modifying the code.
src/AudioSharp.App/Program.cs
Outdated
| { | ||
| PropertyNameCaseInsensitive = true, | ||
| DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, | ||
| WriteIndented = true, |
There was a problem hiding this comment.
Setting WriteIndented = true for the singleton JsonSerializerOptions is great for debugging but is not optimal for production environments. It increases the size of JSON payloads, leading to higher network bandwidth usage and slower serialization/deserialization. It's better to only enable this in the Development environment.
WriteIndented = builder.Environment.IsDevelopment(),![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In @.github/workflows/deploy.yml:
- Around line 44-50: The Deploy step currently passes OPENROUTER_API_KEY via
--set-env-vars in the gcloud run deploy command, which exposes the secret on the
command line; change this to use --set-secrets to reference a Secret Manager
secret instead (e.g., replace the OPENROUTER_API_KEY env var in the
--set-env-vars string with a secret mapping like
OPENROUTER_API_KEY=projects/PROJECT_ID/secrets/SECRET_NAME:latest), ensure the
secret exists in Secret Manager and the Cloud Run service account has
secretmanager.secretAccessor permission, and remove any plaintext secret
interpolation (${{ secrets.OPENROUTER_API_KEY }}) from the gcloud command so
secrets are never passed via CLI or workflow logs.
- Around line 11-17: The workflow-level env block is incorrectly using other env
variables to build IMAGE (IMAGE references
REGION/PROJECT_ID/AR_REPO/IMAGE_NAME), which is unsupported; move construction
of the IMAGE string into a job step (or create it as a step output) instead of
the top-level env: define IMAGE inside a step that runs a shell command to
concatenate ${{ env.REGION }}, ${{ env.PROJECT_ID }}, ${{ env.AR_REPO }}, ${{
env.IMAGE_NAME }} and ${{ github.sha }} and write it to GITHUB_OUTPUT (e.g.,
step id "image") or export it for later steps, then reference that step output
(steps.image.outputs.IMAGE) or the per-step env value in subsequent build/push
steps.
In `@src/AudioSharp.App/Components/Layout/ReconnectModal.razor.js`:
- Around line 1-9: The code attaches event listeners at module load without null
checks, which will throw if elements are missing; wrap the DOM queries and
listener attachments for reconnectModal, retryButton, and resumeButton with
guards that verify each element is non-null before calling addEventListener, and
if needed, log or silently skip when an element is absent; ensure you still
reference the handler functions handleReconnectStateChanged, retry, and resume
only after confirming their corresponding elements exist.
In `@src/AudioSharp.App/Components/Pages/Home.razor`:
- Around line 219-221: The Razor implicit expression is misusing the
null-coalescing operator with plain markup (record.SubjectDisplay ??
record.SubjectReference ?? "Unknown subject"), which causes a syntax error; fix
it by rendering the combined expression as an explicit Razor expression — wrap
the null-coalescing chain for record.SubjectDisplay, record.SubjectReference and
the fallback "Unknown subject" inside @(... ) so Razor evaluates it correctly.
In `@src/AudioSharp.App/Components/Pages/NotFound.razor`:
- Around line 4-5: The NotFound.razor page uses an <h3> heading which prevents
FocusOnNavigate (Selector="h1" in Routes.razor) from moving focus correctly;
change the heading element in NotFound.razor to <h1> (update the heading tag
around "Not Found") so it matches the FocusOnNavigate selector and restores
accessible focus behavior.
In `@src/AudioSharp.App/Data/ConcernRepository.cs`:
- Around line 21-32: GetRecentAsync currently pulls the entire
_dbContext.ConcernRecords into memory then orders and takes which is
inefficient; update the LINQ query on _dbContext.ConcernRecords to apply
AsNoTracking().OrderByDescending(record => record.CreatedAtUtc).Take(count)
before awaiting ToListAsync(cancellationToken).ConfigureAwait(false) so sorting
and limiting happen in the database and only the needed records are
materialized.
In `@src/AudioSharp.App/Program.cs`:
- Around line 181-190: The FhirServer HttpClient registration
(builder.Services.AddHttpClient with FhirServerOptions) leaves
client.BaseAddress unset when options.BaseUrl is null/empty, which will cause
relative HTTP requests to fail; update the Program.cs startup code that calls
AddHttpClient("FhirServer", ...) to validate options.BaseUrl (from
Microsoft.Extensions.Options.IOptions<FhirServerOptions>.Value) and either
throw/stop startup or log a clear warning if BaseUrl is missing, and ensure
EnsureTrailingSlash is only called when BaseUrl is present so client.BaseAddress
is always set for expected FHIR usage.
- Around line 210-214: The endpoint mapping for ProcessAudioAsync currently
calls .DisableAntiforgery(), creating a CSRF risk; either remove
.DisableAntiforgery() so the route uses the app's antiforgery protection (leave
ProcessAudioAsync unchanged) or, if you must keep antiforgery disabled here,
update the client fetch in src/AudioSharp.App/wwwroot/js/audio-recorder.js (the
fetch with credentials: "same-origin" around line 140) to include the CSRF token
header (e.g., read the token from a meta tag or an endpoint and add it as
X-CSRF-TOKEN) and ensure server-side code validates that header; pick one
approach and apply consistently so ProcessAudioAsync is protected.
In `@src/AudioSharp.App/Services/AudioFormatHelper.cs`:
- Around line 12-14: The split-and-indexing on contentType can throw
IndexOutOfRangeException when contentType yields no tokens (e.g., ";;;"); update
the logic in AudioFormatHelper (where the variable normalized is computed) to
first guard against null/empty/whitespace contentType and to handle the case
where Split(..., RemoveEmptyEntries) returns an empty array (e.g., use a safe
default or fallback to contentType.Trim() before ToLowerInvariant()), ensuring
you check the array length or use a safe-first-element retrieval before trimming
and lowercasing.
In `@src/AudioSharp.App/Services/FhirObservationMapper.cs`:
- Around line 17-21: The Map method in FhirObservationMapper can receive a null
concerns parameter and will throw a NullReferenceException when iterating; add a
guard clause at the start of Map (method signature:
Map(IReadOnlyList<ConcernItem> concerns, ProcessingContext context,
DateTimeOffset recordedAt)) that handles null by either returning an empty
IReadOnlyList<FhirObservation> immediately or by treating concerns as an empty
collection (e.g., replace null with Array.Empty<ConcernItem>()), ensuring the
subsequent foreach over concerns is safe; update any callers/tests if they rely
on null being treated specially.
🧹 Nitpick comments (32)
src/AudioSharp.App/Properties/launchSettings.json (1)
4-21: Consider prioritizing HTTPS profile.For a healthcare application handling patient data and FHIR integration, the HTTPS profile should be listed first to encourage secure-by-default development practices.
.dockerignore (1)
1-14: Consider adding.envto prevent secrets leakage in Docker builds.The
.gitignoreexcludes.envfiles, but they're not excluded here. If a.envfile exists locally, it could be sent to the Docker build context and potentially included in the image.♻️ Suggested addition
**/.git **/.github +**/.env +**/.env.* **/.ideasrc/AudioSharp.App/Models/Fhir/FhirAnnotation.cs (1)
5-14: LGTM - Correct FHIR annotation model with proper JSON mapping.The
JsonPropertyName("text")correctly aligns with FHIR specification conventions.Optional: This could be simplified to a record for consistency with other models in the project:
♻️ Optional simplification
using System.Text.Json.Serialization; namespace AudioSharp.App.Models.Fhir; -public sealed class FhirAnnotation -{ - public FhirAnnotation(string text) - { - Text = text; - } - - [JsonPropertyName("text")] - public string Text { get; init; } -} +public sealed record FhirAnnotation( + [property: JsonPropertyName("text")] string Text);src/AudioSharp.App/Components/Layout/ReconnectModal.razor.js (1)
16-17: Consider removing existing listener before adding to prevent duplicates.If the state transitions to
"failed"multiple times beforeretry()is called, thevisibilitychangelistener could be registered multiple times. WhileremoveEventListeneris idempotent with the same function reference, adding a defensive removal ensures robustness.♻️ Optional defensive removal
} else if (event.detail.state === "failed") { + document.removeEventListener("visibilitychange", retryWhenDocumentBecomesVisible); document.addEventListener("visibilitychange", retryWhenDocumentBecomesVisible); } else if (event.detail.state === "rejected") {src/AudioSharp.App/Components/Layout/ReconnectModal.razor.css (1)
34-41: CSS nesting block has inconsistent formatting.The
&[open]nested selector has an unusual brace placement with a blank line separating the selector from its opening brace. Consider reformatting for consistency.♻️ Suggested formatting fix
- &[open] - -{ + &[open] { animation: components-reconnect-modal-slideUp 1.5s cubic-bezier(.05, .89, .25, 1.02) 0.3s, components-reconnect-modal-fadeInOpacity 0.5s ease-in-out 0.3s; animation-fill-mode: both; } - } +}src/AudioSharp.App/wwwroot/app.css (2)
1-1: Consider self-hosting fonts for privacy and performance.Loading fonts from Google's CDN sends user IP addresses and referrer data to Google, which may raise GDPR/privacy concerns in healthcare contexts. Consider:
- Self-hosting the fonts
- Using
font-display: swapfallback if keeping the CDN (already present)- Adding
preconnecthints if external loading is retained
36-36:blur(0px)is a no-op.This filter has no visual effect. If it's intentional for GPU layer promotion, consider using
will-change: transforminstead. Otherwise, remove it.Dockerfile (1)
14-19: Consider adding a non-root user and health check.Running containers as root is a security risk. Consider adding:
🔒 Proposed security improvements
FROM mcr.microsoft.com/dotnet/aspnet:${DOTNET_VERSION} AS final WORKDIR /app + +RUN adduser --disabled-password --gecos '' appuser +USER appuser + ENV ASPNETCORE_URLS=http://+:8080 EXPOSE 8080 COPY --from=build /app/publish . +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD curl -f http://localhost:8080/health || exit 1 ENTRYPOINT ["dotnet", "AudioSharp.App.dll"]Note: This assumes you have a
/healthendpoint. If usingcurlisn't feasible, consider usingwgetor implementing a custom health check binary.src/AudioSharp.App/Services/JsonParsingHelper.cs (1)
1-1: Unused import.
System.Textis imported but not used in this file.🧹 Suggested cleanup
-using System.Text; using System.Text.Json;src/AudioSharp.App/Services/AiUsageTelemetry.cs (1)
41-82: Therefmodifier is unnecessary for class types.
UsageCounterDatais a class (reference type), so passing it byrefis unnecessary—the reference is already passed by value, and mutations apply to the same instance. While this works correctly, it can mislead readers into thinking struct semantics are involved.Suggested fix
private void Record( - ref UsageCounterData counter, + UsageCounterData counter, string provider, string model, bool success, TimeSpan duration, string kind)And update call sites:
- Record(ref _audio, provider, model, success, duration, "Audio"); + Record(_audio, provider, model, success, duration, "Audio");- Record(ref _text, provider, model, success, duration, "Text"); + Record(_text, provider, model, success, duration, "Text");src/AudioSharp.App/Components/Pages/Home.razor (2)
461-461: Consider using a cancellable token for the save operation.
CancellationToken.Noneis used forAddAsync, but if the user navigates away or disposes the component mid-save, the operation continues orphaned. Consider using a component-scoped token for consistency.
671-697: Potential race condition when auto-stopping recording.When
RecordingSeconds >= MaxRecordingSeconds,StopRecordingAsyncis invoked viaInvokeAsync. However,StopRecordingAsynccallsStopRecordingTimer()which cancelsRecordingTimerCts, causing theWaitForNextTickAsyncto throwOperationCanceledException. This is handled, but the flow is subtle. Consider adding a comment or using a flag to make the intent clearer.src/AudioSharp.App/wwwroot/js/audio-recorder.js (3)
181-208: Simple averaging downsampler may introduce aliasing artifacts.The current implementation averages samples without a proper low-pass filter before downsampling. For speech at 16kHz target, this is usually acceptable, but artifacts may occur with certain audio. Consider adding a comment noting this trade-off.
252-263: Potential stack overflow with very large audio files.
String.fromCharCode.apply(null, chunk)can hit the JavaScript call stack limit with large chunks. The chunk size of 0x8000 (32KB) should be safe for most engines, but this is worth noting.For maximum safety, consider using
TextDecoderor iterating character-by-character for very long recordings:Alternative approach
const arrayBufferToBase64 = (buffer) => { const bytes = new Uint8Array(buffer); let binary = ""; for (let i = 0; i < bytes.length; i++) { binary += String.fromCharCode(bytes[i]); } return btoa(binary); };
47-58: Migrate from deprecatedScriptProcessorNodetoAudioWorkletNode.
ScriptProcessorNode(created viacreateScriptProcessor) is deprecated in favor ofAudioWorkletNode. The former runs on the main thread, causing audio glitches and stutters under load;AudioWorkletNoderuns on the audio rendering thread and avoids those issues. WhileScriptProcessorNodecurrently works in all browsers, it may be removed in the future.src/AudioSharp.App/Models/TranscriptResult.cs (1)
1-3: Consider adding null validation for theTextparameter.The record structure is clean and appropriate for this use case. However, since this is part of a medical transcription pipeline, you may want to guard against null or empty text being passed through.
💡 Optional: Add validation
namespace AudioSharp.App.Models; -public sealed record TranscriptResult(string Text); +public sealed record TranscriptResult(string Text) +{ + public string Text { get; init; } = Text ?? throw new ArgumentNullException(nameof(Text)); +}Alternatively, if empty transcripts are valid edge cases, this can remain as-is with validation handled upstream.
src/AudioSharp.App/Components/Pages/NotFound.razor (1)
2-2: Redundant@layoutdirective.Since
Routes.razoralready setsDefaultLayout="typeof(Layout.MainLayout)", this explicit@layoutdirective is unnecessary. It's harmless but adds minor maintenance overhead.src/AudioSharp.App/appsettings.json (2)
16-16: Consider removing secret placeholders from base configuration.Empty
ApiKeyin the baseappsettings.jsoncreates a risk of accidentally committing real credentials. Prefer loading secrets exclusively from environment variables, user secrets (development), or a secrets manager (production) rather than having placeholder fields in version-controlled config.♻️ Suggested approach
Remove the
ApiKeyfield entirely fromappsettings.jsonand rely on environment variable binding (e.g.,OpenRouter__ApiKey) or .NET User Secrets for local development. The same applies toBearerTokenon line 39."OpenRouter": { - "ApiKey": "", "BaseUrl": "https://openrouter.ai/api/v1/",
36-40: Same concern forFhirServersecrets.The
BearerTokenplaceholder should also be removed from version-controlled configuration and loaded from secure sources.♻️ Suggested change
"FhirServer": { "BaseUrl": "", "BundleEndpoint": "Bundle", - "BearerToken": "" }src/AudioSharp.App/Models/Fhir/FhirReference.cs (1)
5-12: LGTM!Well-designed immutable DTO with appropriate
initaccessors and JSON serialization attributes matching FHIR naming conventions.Consider using a
recordtype for a more concise definition:♻️ Optional: Convert to record
-public sealed class FhirReference -{ - [JsonPropertyName("reference")] - public string? Reference { get; init; } - - [JsonPropertyName("display")] - public string? Display { get; init; } -} +public sealed record FhirReference( + [property: JsonPropertyName("reference")] string? Reference, + [property: JsonPropertyName("display")] string? Display +);src/AudioSharp.App/Options/FhirServerOptions.cs (1)
7-9: Consider adding validation forBaseUrl.
BaseUrlis nullable, but the FHIR client likely requires it to function. Consider either:
- Adding
[Required]attribute for options validation- Documenting that the FHIR integration is disabled when
BaseUrlis nullThis helps catch misconfiguration early via
ValidateOnStart()if intended as required.💡 Optional: Add validation attribute
+using System.ComponentModel.DataAnnotations; + namespace AudioSharp.App.Options; public sealed class FhirServerOptions { public const string SectionName = "FhirServer"; + [Required] public string? BaseUrl { get; init; } public string BundleEndpoint { get; init; } = "Bundle"; public string? BearerToken { get; init; } }tests/AudioSharp.App.Tests/Services/ConcernExtractionServiceTests.cs (1)
15-18:PropertyNameCaseInsensitiveis redundant withJsonSerializerDefaults.Web.
JsonSerializerDefaults.Webalready setsPropertyNameCaseInsensitive = true, so explicitly setting it again is unnecessary.Additionally, consider extracting the shared
JsonSerializerOptionssetup to a class-level field to reduce duplication:♻️ Suggested refactor
[TestClass] public sealed class ConcernExtractionServiceTests { + private static readonly JsonSerializerOptions JsonOptions = new(JsonSerializerDefaults.Web); + [TestMethod] public async Task ExtractAsync_ValidJson_ReturnsConcerns() { - var jsonOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web) - { - PropertyNameCaseInsensitive = true - }; - var client = new Mock<ITextCompletionClient>(); // ... - var service = new ConcernExtractionService(client.Object, jsonOptions); + var service = new ConcernExtractionService(client.Object, JsonOptions);Also applies to: 37-40
src/AudioSharp.App/Services/ConcernExtractionService.cs (1)
64-66: Consider normalizing severity values.The severity from the LLM is passed directly without validation. If the LLM returns an unexpected value (e.g., "critical", "moderate", "HIGH"), it won't match the documented
"low|medium|high|unknown"schema. Consider normalizing or falling back to "unknown" for unrecognized values:♻️ Suggested normalization
+ private static readonly HashSet<string> ValidSeverities = + new(["low", "medium", "high", "unknown"], StringComparer.OrdinalIgnoreCase); + + private static string NormalizeSeverity(string? severity) => + severity is not null && ValidSeverities.Contains(severity) + ? severity.ToLowerInvariant() + : "unknown"; // In ExtractAsync: var concerns = payload.Concerns? .Select(item => new ConcernItem( item.Summary ?? string.Empty, - item.Severity, + NormalizeSeverity(item.Severity), item.Onset,src/AudioSharp.App/Services/FhirServerClient.cs (1)
25-70: Consider adding timeout configuration for resilience.The HTTP request relies on default timeouts. For a medical application uploading to external FHIR servers, consider configuring explicit timeouts either on the named HttpClient registration or via
CancellationTokenSourcewith timeout to prevent hanging requests.src/AudioSharp.App/Components/Layout/NavMenu.razor (1)
82-87: Consider logging exceptions from the fire-and-forget call.The discarded task from
_ = InvokeAsync(StateHasChanged)will silently swallow any exceptions. For telemetry display this is likely acceptable, but if unexpected issues arise, they won't be visible.♻️ Optional: Add exception handling
private void OnTelemetryChanged() { Snapshot = AiUsageTelemetry.GetSnapshot(); RecentEvents = AiUsageTelemetry.GetRecentEvents(); - _ = InvokeAsync(StateHasChanged); + _ = InvokeAsync(StateHasChanged).ContinueWith( + t => Console.Error.WriteLine($"StateHasChanged failed: {t.Exception}"), + TaskContinuationOptions.OnlyOnFaulted); }tests/AudioSharp.App.Tests/Services/FollowUpQuestionServiceTests.cs (1)
15-18: Consider extracting sharedJsonSerializerOptionssetup.Both tests duplicate the same
JsonSerializerOptionsconfiguration. Extract to a class-level field orTestInitializemethod:♻️ Optional refactor
[TestClass] public sealed class FollowUpQuestionServiceTests { + private readonly JsonSerializerOptions _jsonOptions = new(JsonSerializerDefaults.Web) + { + PropertyNameCaseInsensitive = true + }; + [TestMethod] public async Task GenerateAsync_ValidJson_ReturnsQuestions() { - var jsonOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web) - { - PropertyNameCaseInsensitive = true - }; - var client = new Mock<ITextCompletionClient>(); // ... - var service = new FollowUpQuestionService(client.Object, jsonOptions); + var service = new FollowUpQuestionService(client.Object, _jsonOptions);Also applies to: 42-45
tests/AudioSharp.App.Tests/Services/ConcernRefinementServiceTests.cs (1)
15-18:PropertyNameCaseInsensitiveis redundant withJsonSerializerDefaults.Web.
JsonSerializerDefaults.Webalready setsPropertyNameCaseInsensitive = trueby default. Consider removing the redundant setting and extracting the options to a shared field.♻️ Suggested refactor
+ private static readonly JsonSerializerOptions s_jsonOptions = new(JsonSerializerDefaults.Web); + [TestMethod] public async Task ApplyAnswersAsync_ValidJson_ReturnsUpdatedConcerns() { - var jsonOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web) - { - PropertyNameCaseInsensitive = true - }; - var client = new Mock<ITextCompletionClient>(); // ... - var service = new ConcernRefinementService(client.Object, jsonOptions); + var service = new ConcernRefinementService(client.Object, s_jsonOptions);Also applies to: 44-47
src/AudioSharp.App/Services/AudioProcessingService.cs (1)
37-37: Consider injectingTimeProviderfor testability.Using
DateTimeOffset.UtcNowdirectly makes it harder to test time-dependent scenarios. .NET 8+ providesTimeProviderfor this purpose.src/AudioSharp.App/Services/FollowUpQuestionService.cs (1)
67-96: Potential prompt injection risk with transcript content.The
transcriptis directly interpolated into the prompt template without sanitization. If the transcript contains text like<<ANSWERS>>or JSON structures, it could confuse the model or cause unexpected behavior. Consider whether any sanitization is needed for medical transcripts.💡 Optional: Add basic sanitization for template markers
var prompt = UserPromptTemplate .Replace("<<MISSING_FIELDS>>", missingJson) - .Replace("<<TRANSCRIPT>>", transcript ?? string.Empty); + .Replace("<<TRANSCRIPT>>", SanitizeForPrompt(transcript ?? string.Empty)); + +private static string SanitizeForPrompt(string input) +{ + // Remove or escape any template markers to prevent injection + return input.Replace("<<", "< <").Replace(">>", "> >"); +}src/AudioSharp.App/Services/ConcernRefinementService.cs (1)
107-147: Minor: NormalizeAnswers does not validate ConcernIndex upper bound.The
NormalizeAnswersmethod checksanswer.ConcernIndex < 0but doesn't validate againstconcerns.Count. While this is handled later inApplyAnswersFallback(out-of-range keys simply won't match), it could lead to confusion if an answer references a non-existent concern.The current approach works because
ApplyAnswersFallbackonly iterates valid concern indices. Consider adding the upper bound check for clarity if you want strict validation early.src/AudioSharp.App/Program.cs (2)
36-44: Consider adding OTLP exporter for production.The console exporter is useful for development but won't work well in production. Consider conditionally configuring exporters based on environment.
💡 Suggested: Add OTLP exporter option
builder.Services.AddOpenTelemetry() .ConfigureResource(resource => resource.AddService("AudioSharp")) .WithTracing(tracing => { tracing .AddAspNetCoreInstrumentation(options => options.RecordException = true) - .AddHttpClientInstrumentation(options => options.RecordException = true) - .AddConsoleExporter(); + .AddHttpClientInstrumentation(options => options.RecordException = true); + + if (builder.Environment.IsDevelopment()) + { + tracing.AddConsoleExporter(); + } + // Consider: tracing.AddOtlpExporter() for production });
284-296: Consider differentiating client cancellation from timeout.
OperationCanceledExceptioncan be thrown both when the client disconnects and when an internal timeout occurs. Returning 408 for client-initiated cancellations may be misleading. Consider checkingcancellationToken.IsCancellationRequestedto distinguish.💡 Differentiate cancellation sources
catch (OperationCanceledException) { - return TypedResults.Problem( - "Audio processing canceled.", - statusCode: StatusCodes.Status408RequestTimeout); + // Client disconnect vs internal timeout + return cancellationToken.IsCancellationRequested + ? TypedResults.StatusCode(StatusCodes.Status499ClientClosedRequest) // or just return empty + : TypedResults.Problem( + "Audio processing timed out.", + statusCode: StatusCodes.Status408RequestTimeout); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (47)
src/AudioSharp.App/concerns.dbis excluded by!**/*.dbsrc/AudioSharp.App/wwwroot/favicon.pngis excluded by!**/*.pngsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-grid.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-grid.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-grid.min.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-grid.min.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-grid.rtl.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-grid.rtl.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-grid.rtl.min.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-grid.rtl.min.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-reboot.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-reboot.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-reboot.min.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-reboot.min.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-reboot.rtl.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-reboot.rtl.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-reboot.rtl.min.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-reboot.rtl.min.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-utilities.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-utilities.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-utilities.min.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-utilities.min.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-utilities.rtl.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-utilities.rtl.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-utilities.rtl.min.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap-utilities.rtl.min.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap.min.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap.min.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap.rtl.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap.rtl.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap.rtl.min.cssis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/css/bootstrap.rtl.min.css.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.jsis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.js.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.min.jsis excluded by!**/dist/**,!**/*.min.jssrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.min.js.mapis excluded by!**/dist/**,!**/*.map,!**/*.min.js.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.esm.jsis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.esm.js.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.esm.min.jsis excluded by!**/dist/**,!**/*.min.jssrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.esm.min.js.mapis excluded by!**/dist/**,!**/*.map,!**/*.min.js.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.jsis excluded by!**/dist/**src/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.js.mapis excluded by!**/dist/**,!**/*.mapsrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.min.jsis excluded by!**/dist/**,!**/*.min.jssrc/AudioSharp.App/wwwroot/lib/bootstrap/dist/js/bootstrap.min.js.mapis excluded by!**/dist/**,!**/*.map,!**/*.min.js.mapsrc/AudioSharp.App/wwwroot/lib/fast/fast-components.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (83)
.dockerignore.github/workflows/ci.yml.github/workflows/deploy.yml.gitignoreAudioSharp.slnxDockerfilesrc/AudioSharp.App/AudioSharp.App.csprojsrc/AudioSharp.App/Components/App.razorsrc/AudioSharp.App/Components/Layout/MainLayout.razorsrc/AudioSharp.App/Components/Layout/MainLayout.razor.csssrc/AudioSharp.App/Components/Layout/NavMenu.razorsrc/AudioSharp.App/Components/Layout/NavMenu.razor.csssrc/AudioSharp.App/Components/Layout/ReconnectModal.razorsrc/AudioSharp.App/Components/Layout/ReconnectModal.razor.csssrc/AudioSharp.App/Components/Layout/ReconnectModal.razor.jssrc/AudioSharp.App/Components/Pages/Error.razorsrc/AudioSharp.App/Components/Pages/Home.razorsrc/AudioSharp.App/Components/Pages/NotFound.razorsrc/AudioSharp.App/Components/Routes.razorsrc/AudioSharp.App/Components/_Imports.razorsrc/AudioSharp.App/Data/ConcernRecord.cssrc/AudioSharp.App/Data/ConcernRepository.cssrc/AudioSharp.App/Data/ConcernsDbContext.cssrc/AudioSharp.App/Data/IConcernRepository.cssrc/AudioSharp.App/Models/AiTelemetryEvent.cssrc/AudioSharp.App/Models/AiUsageSnapshot.cssrc/AudioSharp.App/Models/AudioInput.cssrc/AudioSharp.App/Models/ChatMessage.cssrc/AudioSharp.App/Models/ConcernExtractionResult.cssrc/AudioSharp.App/Models/ConcernItem.cssrc/AudioSharp.App/Models/Fhir/FhirAnnotation.cssrc/AudioSharp.App/Models/Fhir/FhirBundle.cssrc/AudioSharp.App/Models/Fhir/FhirCodeableConcept.cssrc/AudioSharp.App/Models/Fhir/FhirCoding.cssrc/AudioSharp.App/Models/Fhir/FhirObservation.cssrc/AudioSharp.App/Models/Fhir/FhirReference.cssrc/AudioSharp.App/Models/FhirUploadResult.cssrc/AudioSharp.App/Models/FollowUpAnswer.cssrc/AudioSharp.App/Models/FollowUpQuestion.cssrc/AudioSharp.App/Models/ProcessingContext.cssrc/AudioSharp.App/Models/ProcessingResponse.cssrc/AudioSharp.App/Models/ProcessingResult.cssrc/AudioSharp.App/Models/TranscriptResult.cssrc/AudioSharp.App/Options/FhirMappingOptions.cssrc/AudioSharp.App/Options/FhirServerOptions.cssrc/AudioSharp.App/Options/LlmProviderOptions.cssrc/AudioSharp.App/Options/LmStudioOptions.cssrc/AudioSharp.App/Options/OpenRouterOptions.cssrc/AudioSharp.App/Options/RecordingOptions.cssrc/AudioSharp.App/Program.cssrc/AudioSharp.App/Properties/launchSettings.jsonsrc/AudioSharp.App/Services/AiUsageTelemetry.cssrc/AudioSharp.App/Services/AudioFormatHelper.cssrc/AudioSharp.App/Services/AudioProcessingService.cssrc/AudioSharp.App/Services/ConcernExtractionService.cssrc/AudioSharp.App/Services/ConcernRefinementService.cssrc/AudioSharp.App/Services/FhirBundleBuilder.cssrc/AudioSharp.App/Services/FhirObservationMapper.cssrc/AudioSharp.App/Services/FhirServerClient.cssrc/AudioSharp.App/Services/FollowUpQuestionService.cssrc/AudioSharp.App/Services/IAiUsageTelemetry.cssrc/AudioSharp.App/Services/IAudioProcessingService.cssrc/AudioSharp.App/Services/IAudioTranscriptionService.cssrc/AudioSharp.App/Services/IConcernExtractionService.cssrc/AudioSharp.App/Services/IConcernRefinementService.cssrc/AudioSharp.App/Services/IFhirBundleBuilder.cssrc/AudioSharp.App/Services/IFhirObservationMapper.cssrc/AudioSharp.App/Services/IFhirServerClient.cssrc/AudioSharp.App/Services/IFollowUpQuestionService.cssrc/AudioSharp.App/Services/ITextCompletionClient.cssrc/AudioSharp.App/Services/JsonParsingHelper.cssrc/AudioSharp.App/Services/OpenRouterAudioTranscriptionService.cssrc/AudioSharp.App/Services/TextCompletionClient.cssrc/AudioSharp.App/appsettings.Development.jsonsrc/AudioSharp.App/appsettings.jsonsrc/AudioSharp.App/wwwroot/app.csssrc/AudioSharp.App/wwwroot/js/audio-recorder.jstests/AudioSharp.App.Tests/AudioSharp.App.Tests.csprojtests/AudioSharp.App.Tests/MSTestSettings.cstests/AudioSharp.App.Tests/Services/ConcernExtractionServiceTests.cstests/AudioSharp.App.Tests/Services/ConcernRefinementServiceTests.cstests/AudioSharp.App.Tests/Services/FhirObservationMapperTests.cstests/AudioSharp.App.Tests/Services/FollowUpQuestionServiceTests.cs
🧰 Additional context used
🧬 Code graph analysis (17)
src/AudioSharp.App/Services/IFhirBundleBuilder.cs (1)
src/AudioSharp.App/Models/Fhir/FhirObservation.cs (1)
FhirObservation(5-33)
src/AudioSharp.App/Models/ProcessingResult.cs (1)
src/AudioSharp.App/Models/Fhir/FhirObservation.cs (1)
FhirObservation(5-33)
src/AudioSharp.App/Services/OpenRouterAudioTranscriptionService.cs (3)
src/AudioSharp.App/Options/OpenRouterOptions.cs (1)
OpenRouterOptions(3-13)src/AudioSharp.App/wwwroot/js/audio-recorder.js (1)
options(15-15)src/AudioSharp.App/Services/AudioFormatHelper.cs (2)
AudioFormatHelper(3-27)ResolveFormat(5-26)
src/AudioSharp.App/Services/ConcernExtractionService.cs (1)
src/AudioSharp.App/Services/JsonParsingHelper.cs (2)
JsonParsingHelper(6-44)TryDeserializeJson(8-31)
src/AudioSharp.App/Models/AiUsageSnapshot.cs (1)
src/AudioSharp.App/Services/AiUsageTelemetry.cs (1)
UsageCounter(94-104)
src/AudioSharp.App/Data/ConcernsDbContext.cs (1)
src/AudioSharp.App/Data/ConcernRecord.cs (1)
ConcernRecord(3-12)
src/AudioSharp.App/Program.cs (8)
src/AudioSharp.App/Options/OpenRouterOptions.cs (1)
OpenRouterOptions(3-13)src/AudioSharp.App/Options/FhirServerOptions.cs (1)
FhirServerOptions(3-10)src/AudioSharp.App/Options/LmStudioOptions.cs (1)
LmStudioOptions(3-9)src/AudioSharp.App/Options/LlmProviderOptions.cs (1)
LlmProviderOptions(9-14)src/AudioSharp.App/Options/FhirMappingOptions.cs (1)
FhirMappingOptions(3-11)src/AudioSharp.App/Options/RecordingOptions.cs (1)
RecordingOptions(5-11)src/AudioSharp.App/Services/AiUsageTelemetry.cs (1)
AiUsageTelemetry(5-106)src/AudioSharp.App/Services/FhirBundleBuilder.cs (1)
FhirBundleBuilder(5-17)
src/AudioSharp.App/Data/ConcernRepository.cs (1)
src/AudioSharp.App/Data/ConcernRecord.cs (1)
ConcernRecord(3-12)
src/AudioSharp.App/Models/Fhir/FhirBundle.cs (1)
src/AudioSharp.App/Models/Fhir/FhirObservation.cs (1)
FhirObservation(5-33)
src/AudioSharp.App/Services/FhirServerClient.cs (1)
src/AudioSharp.App/Options/FhirServerOptions.cs (1)
FhirServerOptions(3-10)
src/AudioSharp.App/Data/IConcernRepository.cs (1)
src/AudioSharp.App/Data/ConcernRecord.cs (1)
ConcernRecord(3-12)
src/AudioSharp.App/Services/FollowUpQuestionService.cs (1)
src/AudioSharp.App/Services/JsonParsingHelper.cs (2)
JsonParsingHelper(6-44)TryDeserializeJson(8-31)
src/AudioSharp.App/Services/FhirBundleBuilder.cs (2)
src/AudioSharp.App/Models/Fhir/FhirObservation.cs (1)
FhirObservation(5-33)src/AudioSharp.App/Models/Fhir/FhirBundle.cs (1)
FhirBundleEntry(17-21)
src/AudioSharp.App/Services/FhirObservationMapper.cs (3)
src/AudioSharp.App/Options/FhirMappingOptions.cs (1)
FhirMappingOptions(3-11)src/AudioSharp.App/Models/Fhir/FhirObservation.cs (1)
FhirObservation(5-33)src/AudioSharp.App/Models/Fhir/FhirCoding.cs (1)
FhirCoding(5-15)
src/AudioSharp.App/Services/ConcernRefinementService.cs (1)
src/AudioSharp.App/Services/JsonParsingHelper.cs (2)
JsonParsingHelper(6-44)TryDeserializeJson(8-31)
tests/AudioSharp.App.Tests/Services/FhirObservationMapperTests.cs (1)
src/AudioSharp.App/Options/FhirMappingOptions.cs (1)
FhirMappingOptions(3-11)
src/AudioSharp.App/Services/TextCompletionClient.cs (3)
src/AudioSharp.App/Options/OpenRouterOptions.cs (1)
OpenRouterOptions(3-13)src/AudioSharp.App/Options/LmStudioOptions.cs (1)
LmStudioOptions(3-9)src/AudioSharp.App/Options/LlmProviderOptions.cs (1)
LlmProviderOptions(9-14)
🪛 actionlint (1.7.10)
.github/workflows/deploy.yml
17-17: context "env" is not allowed here. available contexts are "github", "inputs", "secrets", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
🪛 Biome (2.1.2)
src/AudioSharp.App/Components/Layout/NavMenu.razor.css
[error] 66-66: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
[error] 78-78: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
[error] 83-83: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
src/AudioSharp.App/Components/Layout/MainLayout.razor.css
[error] 24-24: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
[error] 24-24: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
[error] 30-30: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
[error] 30-30: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
[error] 34-34: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
[error] 44-44: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
[error] 44-44: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
[error] 67-67: Unexpected unknown pseudo-elements: deep
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
⏰ 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). (2)
- GitHub Check: CodeQL analysis (csharp)
- GitHub Check: Agent
| public IReadOnlyList<FhirObservation> Map( | ||
| IReadOnlyList<ConcernItem> concerns, | ||
| ProcessingContext context, | ||
| DateTimeOffset recordedAt) | ||
| { |
There was a problem hiding this comment.
Add null check for concerns parameter.
If concerns is null, the foreach loop will throw NullReferenceException. Consider adding a guard clause or using null-conditional patterns.
🛡️ Suggested fix
public IReadOnlyList<FhirObservation> Map(
IReadOnlyList<ConcernItem> concerns,
ProcessingContext context,
DateTimeOffset recordedAt)
{
+ ArgumentNullException.ThrowIfNull(concerns);
+
var observations = new List<FhirObservation>();
foreach (var concern in concerns)📝 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.
| public IReadOnlyList<FhirObservation> Map( | |
| IReadOnlyList<ConcernItem> concerns, | |
| ProcessingContext context, | |
| DateTimeOffset recordedAt) | |
| { | |
| public IReadOnlyList<FhirObservation> Map( | |
| IReadOnlyList<ConcernItem> concerns, | |
| ProcessingContext context, | |
| DateTimeOffset recordedAt) | |
| { | |
| ArgumentNullException.ThrowIfNull(concerns); | |
| var observations = new List<FhirObservation>(); | |
| foreach (var concern in concerns) |
🤖 Prompt for AI Agents
In `@src/AudioSharp.App/Services/FhirObservationMapper.cs` around lines 17 - 21,
The Map method in FhirObservationMapper can receive a null concerns parameter
and will throw a NullReferenceException when iterating; add a guard clause at
the start of Map (method signature: Map(IReadOnlyList<ConcernItem> concerns,
ProcessingContext context, DateTimeOffset recordedAt)) that handles null by
either returning an empty IReadOnlyList<FhirObservation> immediately or by
treating concerns as an empty collection (e.g., replace null with
Array.Empty<ConcernItem>()), ensuring the subsequent foreach over concerns is
safe; update any callers/tests if they rely on null being treated specially.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 1-11: Package.json pins Tailwind CSS at "3.4.15" which is several
major versions behind; update the "tailwindcss" entry in devDependencies to the
desired 4.x version (e.g., "4.1.x"), run npm install, and verify the existing
scripts "build:css" and "watch:css" still work; review and apply any required
changes from the Tailwind v4 migration guide to config (tailwind.config.js) and
any utility or plugin updates before committing.
♻️ Duplicate comments (2)
.github/workflows/deploy.yml (2)
11-17: Theenvcontext self-referencing issue persists.This issue was previously flagged. The
IMAGEvariable at line 17 references${{ env.REGION }},${{ env.PROJECT_ID }}, etc., but theenvcontext is unavailable within the workflow-levelenv:block. This will cause empty substitutions and break the deployment.
53-59: API key exposure in command-line arguments persists.This issue was previously flagged. Passing
OPENROUTER_API_KEY=${{ secrets.OPENROUTER_API_KEY }}via--set-env-varsexposes the secret in workflow logs. Use--set-secretswith Google Secret Manager instead.
🧹 Nitpick comments (4)
.github/workflows/ci.yml (1)
3-5: Consider adding push trigger for main branch.The workflow only runs on
pull_request. Consider also triggering onpushtomainto catch issues from direct commits or merged PRs.Suggested improvement
on: pull_request: branches: ["main"] + push: + branches: ["main"]src/AudioSharp.App/AudioSharp.App.csproj (1)
23-27: Tailwind CSS only builds before Publish, not during regular builds.The
BuildTailwindCsstarget only runsBeforeTargets="Publish". During localdotnet buildordotnet run, this target won't execute, potentially resulting in missing or stale CSS. Consider also addingBuildto ensure consistent local development.Suggested improvement
- <Target Name="BuildTailwindCss" BeforeTargets="Publish" Condition="'$(SkipTailwindCss)' != 'true'"> + <Target Name="BuildTailwindCss" BeforeTargets="Build;Publish" Condition="'$(SkipTailwindCss)' != 'true'">Alternatively, if you prefer manual CSS building during development (using
npm run watch:css), document this workflow in the README.Dockerfile (1)
7-9: Consider caching npm dependencies for faster builds.Installing Node.js and running
npm ciduring every build can be slow. For better layer caching, consider copyingpackage.jsonandpackage-lock.jsonbeforeCOPY . .and runningnpm ciseparately.Suggested optimization
RUN apt-get update \ && apt-get install -y --no-install-recommends nodejs npm \ && rm -rf /var/lib/apt/lists/* +COPY ["package.json", "package-lock.json*", "./"] +RUN npm ci + COPY ["src/AudioSharp.App/AudioSharp.App.csproj", "src/AudioSharp.App/"] RUN dotnet restore "src/AudioSharp.App/AudioSharp.App.csproj"This allows the npm layer to be cached when only source code changes.
src/AudioSharp.App/Styles/app.css (1)
35-42: Consider removing the no-opfilter: blur(0px).This declaration has no visual effect and can be safely removed to reduce CSS complexity.
♻️ Proposed fix
body::before, body::after { content: ""; position: fixed; border-radius: 50%; z-index: 0; - filter: blur(0px); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
.github/workflows/ci.yml.github/workflows/deploy.yml.gitignoreDockerfileREADME.mdpackage.jsonsrc/AudioSharp.App/AudioSharp.App.csprojsrc/AudioSharp.App/Components/App.razorsrc/AudioSharp.App/Components/Layout/NavMenu.razorsrc/AudioSharp.App/Components/Pages/Error.razorsrc/AudioSharp.App/Styles/app.csssrc/AudioSharp.App/wwwroot/app.csstailwind.config.js
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .gitignore
- src/AudioSharp.App/Components/App.razor
- src/AudioSharp.App/Components/Pages/Error.razor
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/deploy.yml
17-17: context "env" is not allowed here. available contexts are "github", "inputs", "secrets", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
🪛 Biome (2.1.2)
src/AudioSharp.App/wwwroot/app.css
[error] 3-3: Duplicate font names are redundant and unnecessary: Emoji
Remove duplicate font names within the property
(lint/suspicious/noDuplicateFontNames)
🔇 Additional comments (10)
tailwind.config.js (1)
1-11: LGTM!The Tailwind configuration is well-structured with appropriate content paths covering Blazor templates (
.razor,.html,.cshtml) and JavaScript assets. The JSDoc type annotation provides good IDE support.Dockerfile (2)
11-16: Good layer caching strategy for .NET restore.Copying the
.csprojfile first and runningdotnet restorebeforeCOPY . .ensures the restore layer is cached unless project dependencies change. The--no-restoreflag on publish correctly leverages this.
18-23: Runtime stage is well-configured for Cloud Run.The final stage uses the minimal ASP.NET runtime image, correctly configures port 8080 (Cloud Run default), and copies only the published output. This results in a lean production image.
src/AudioSharp.App/AudioSharp.App.csproj (1)
15-21: Package versions are valid and consistent.All OpenTelemetry packages are correctly pinned to version 1.14.0 to ensure compatibility. Microsoft.EntityFrameworkCore.Sqlite 10.0.1 aligns with the .NET 10 target framework.
src/AudioSharp.App/Styles/app.css (1)
1-20: Well-structured design system foundation.The CSS custom properties provide a clean, maintainable theming approach with semantic naming (
--ink-*,--panel,--accent-*). The Tailwind integration is correctly configured with the@tailwinddirectives.src/AudioSharp.App/wwwroot/app.css (1)
1-3: Generated Tailwind output - no manual changes needed.This file is auto-generated by the Tailwind CSS build pipeline from
src/AudioSharp.App/Styles/app.css. The static analysis warning about duplicate font names ("Emoji") originates from Tailwind's standard preflight CSS and is expected in the generated output.Ensure this file is not manually edited; changes should be made to the source file in
Styles/app.css.src/AudioSharp.App/Components/Layout/NavMenu.razor (4)
1-11: Clean component structure with proper dependency injection.The component correctly uses
@implements IDisposableand injects the telemetry service. The top navigation bar structure is well-organized.
82-87: Fire-and-forget pattern is acceptable here but consider exception handling.The
_ = InvokeAsync(StateHasChanged)pattern is common in Blazor for UI updates from event handlers. Any exceptions will be handled by Blazor's error boundary. This is acceptable for this use case.
125-134: Division safety is correctly handled.The
GetSuccessPercentmethod properly guards against division by zero by checkingcounter.Total == 0before performing the calculation.
149-152: Proper disposal pattern preventing memory leaks.The component correctly unsubscribes from the
Changedevent inDispose(), preventing memory leaks from orphaned event handlers.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/AudioSharp.App/Services/FhirObservationMapper.cs`:
- Around line 17-23: The Map method currently only null-checks concerns but not
the ProcessingContext parameter, so calling Map with a null context will cause a
NullReferenceException in BuildSubject(context); add a null check for context at
the start of Map (e.g., ArgumentNullException.ThrowIfNull(context)) so
BuildSubject(context) and any access to context.SubjectReference are guarded;
update Map in FhirObservationMapper to validate context before proceeding.
🧹 Nitpick comments (4)
src/AudioSharp.App/Services/AiUsageTelemetry.cs (1)
31-39: Consider adding parameter validation.The
providerandmodelparameters are stored without null/empty checks. If callers pass invalid values, they'll propagate to snapshots and UI.🔧 Optional: Add defensive validation
public void RecordAudioTranscription(string provider, string model, bool success, TimeSpan duration) { + ArgumentException.ThrowIfNullOrWhiteSpace(provider); + ArgumentException.ThrowIfNullOrWhiteSpace(model); Record(_audio, provider, model, success, duration, "Audio"); } public void RecordTextCompletion(string provider, string model, bool success, TimeSpan duration) { + ArgumentException.ThrowIfNullOrWhiteSpace(provider); + ArgumentException.ThrowIfNullOrWhiteSpace(model); Record(_text, provider, model, success, duration, "Text"); }src/AudioSharp.App/Services/FhirObservationMapper.cs (1)
85-120: Consider extracting a helper to reduce repetition.The pattern of checking and adding annotations is repeated six times. A small helper could reduce duplication while maintaining clarity.
♻️ Optional refactor
private static List<FhirAnnotation> BuildNotes(ConcernItem concern) { var notes = new List<FhirAnnotation>(); + + AddNoteIfPresent(notes, "Severity", concern.Severity); + AddNoteIfPresent(notes, "Onset", concern.Onset); + AddNoteIfPresent(notes, "Duration", concern.Duration); + AddNoteIfPresent(notes, "Impact", concern.Impact); + AddNoteIfPresent(notes, "Context", concern.Context); + + if (!string.IsNullOrWhiteSpace(concern.PatientQuote)) + { + notes.Add(new FhirAnnotation($"Quote: \"{concern.PatientQuote}\"")); + } - if (!string.IsNullOrWhiteSpace(concern.Severity)) - { - notes.Add(new FhirAnnotation($"Severity: {concern.Severity}")); - } - - if (!string.IsNullOrWhiteSpace(concern.Onset)) - { - notes.Add(new FhirAnnotation($"Onset: {concern.Onset}")); - } - - if (!string.IsNullOrWhiteSpace(concern.Duration)) - { - notes.Add(new FhirAnnotation($"Duration: {concern.Duration}")); - } - - if (!string.IsNullOrWhiteSpace(concern.Impact)) - { - notes.Add(new FhirAnnotation($"Impact: {concern.Impact}")); - } - - if (!string.IsNullOrWhiteSpace(concern.Context)) - { - notes.Add(new FhirAnnotation($"Context: {concern.Context}")); - } - - if (!string.IsNullOrWhiteSpace(concern.PatientQuote)) - { - notes.Add(new FhirAnnotation($"Quote: \"{concern.PatientQuote}\"")); - } - return notes; } + private static void AddNoteIfPresent(List<FhirAnnotation> notes, string label, string? value) + { + if (!string.IsNullOrWhiteSpace(value)) + { + notes.Add(new FhirAnnotation($"{label}: {value}")); + } + }src/AudioSharp.App/Components/Layout/NavMenu.razor.css (1)
187-199: Consider using a standard breakpoint value.The breakpoint at
641pxis slightly unconventional. Common responsive breakpoints are640px(Tailwind'ssm) or768px(Bootstrap'smd). If the app uses Tailwind elsewhere, aligning to640pxwould improve consistency.♻️ Optional: Align breakpoint with Tailwind's `sm` (640px)
-@media (min-width: 641px) { +@media (min-width: 640px) {src/AudioSharp.App/wwwroot/app.css (1)
1-3: Add a header comment indicating this is a Tailwind-generated build artifact.The
build:cssnpm script compilessrc/AudioSharp.App/Styles/app.css(source) to this file with minification. Consider adding a comment at the top like/* Generated by Tailwind CSS build script. Do not edit manually. */to prevent accidental modifications.The static analysis warning about "Duplicate font names: Emoji" is a false positive. The font stack includes distinct fonts (
Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol,Noto Color Emoji) for cross-platform emoji rendering—this is standard Tailwind output.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/deploy.ymlsrc/AudioSharp.App/Components/Layout/MainLayout.razor.csssrc/AudioSharp.App/Components/Layout/NavMenu.razor.csssrc/AudioSharp.App/Services/AiUsageTelemetry.cssrc/AudioSharp.App/Services/FhirObservationMapper.cssrc/AudioSharp.App/Styles/app.csssrc/AudioSharp.App/wwwroot/app.css
🚧 Files skipped from review as they are similar to previous changes (3)
- src/AudioSharp.App/Styles/app.css
- src/AudioSharp.App/Components/Layout/MainLayout.razor.css
- .github/workflows/deploy.yml
🧰 Additional context used
🧬 Code graph analysis (1)
src/AudioSharp.App/Services/FhirObservationMapper.cs (3)
src/AudioSharp.App/Options/FhirMappingOptions.cs (1)
FhirMappingOptions(3-11)src/AudioSharp.App/Models/Fhir/FhirObservation.cs (1)
FhirObservation(5-33)src/AudioSharp.App/Models/Fhir/FhirCoding.cs (1)
FhirCoding(5-15)
🪛 Biome (2.1.2)
src/AudioSharp.App/wwwroot/app.css
[error] 3-3: Duplicate font names are redundant and unnecessary: Emoji
Remove duplicate font names within the property
(lint/suspicious/noDuplicateFontNames)
🔇 Additional comments (11)
src/AudioSharp.App/Services/AiUsageTelemetry.cs (5)
5-14: Well-structured thread-safe telemetry service.The class is properly sealed, fields are correctly marked
readonly, and the lock-based synchronization pattern is appropriately established.Note: The past Copilot comments about making
_textand_audioreadonly are now obsolete—these fields are alreadyreadonlyon lines 9-10.
15-21: LGTM!Thread-safe snapshot creation with proper lock scope.
23-29: LGTM!Returns a defensive copy of the events list, ensuring thread safety and preventing external mutation.
41-82: Correct thread-safety pattern for event invocation.Capturing the
Changedhandler inside the lock (line 78) and invoking it outside (line 81) is the right approach to prevent deadlocks when subscribers call back into the service.The list capping logic and counter updates are properly synchronized.
84-105: LGTM!Clean internal data class with proper
ToSnapshot()conversion to an immutable record.src/AudioSharp.App/Services/FhirObservationMapper.cs (4)
8-15: LGTM!The sealed class and constructor pattern with
IOptions<T>is idiomatic for .NET configuration injection.
24-42: LGTM!The observation creation logic is well-structured:
- GUID format "N" produces valid resource IDs.
- ISO 8601 "O" format is FHIR-compliant for
effectiveDateTime.- Setting
Noteto null when empty avoids unnecessary empty arrays in JSON output.
45-68: LGTM!The conditional coding creation correctly requires both
SystemandCodeto be present, and the text fallback ensures a meaningful display value is always set.
70-83: LGTM!The method correctly handles cases where subject information may be partially or fully absent.
src/AudioSharp.App/Components/Layout/NavMenu.razor.css (2)
1-72: LGTM! Navigation toggle and icon styling is well-implemented.The checkbox-based toggle pattern using
:checked ~ .nav-scrollableis a valid CSS-only approach for mobile navigation. Embedded SVG data URIs for small icons are appropriate here, reducing HTTP requests while keeping file size manageable.
74-185: Telemetry card UI styling is comprehensive and well-organized.The styling for KPIs, status indicators (
.telemetry-ok,.telemetry-fail), and section dividers follows a consistent design system. The use of CSS custom property--telemetry-percentfor the bar width (line 146) enables dynamic updates from the component.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/AudioSharp.App/wwwroot/app.css`:
- Line 2: Interactive controls (fast-button, input, textarea, .nav-link) have no
visible focus styles; add accessible focus indicators by creating :focus-visible
(and fallback :focus) rules for fast-button::part(control), input, textarea, and
.nav-link that apply a clear outline or high-contrast box-shadow (e.g., 2-3px
solid or 0 0 0 3px) and use outline-offset and/or border-color to avoid layout
shift; ensure styles for fast-button target fast-button::part(control) and
include a focus state for active/hover combos so keyboard users see a distinct
focus ring without interfering with existing .btn-primary/.btn-outline-light
visuals.
🧹 Nitpick comments (2)
src/AudioSharp.App/wwwroot/app.css (2)
1-1: Consider excluding generated CSS from version control.This is a compiled Tailwind output file. If your CI/CD pipeline runs
npm run build:cssbefore deployment, consider adding this file to.gitignoreto avoid merge conflicts and reduce repository size. If the deployment process doesn't include a build step, committing the compiled output is acceptable—just ensure developers remember to regenerate it after source changes.
2-2: CSS@importfor fonts is render-blocking.Loading Google Fonts via
@importin CSS blocks rendering until the fonts are fetched. For better performance, consider loading fonts via<link rel="preconnect">and<link rel="stylesheet">in the HTML<head>, or self-hosting the fonts to eliminate the external dependency and potential privacy concerns with third-party requests.Example in your HTML head:
<link rel="preconnect" href="https://fonts.googleapis.com"> <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin> <link href="https://fonts.googleapis.com/css2?family=DM+Serif+Display&family=Space+Grotesk:wght@400;500;600;700&display=swap" rel="stylesheet">Then remove the
@importfrom your Tailwind source CSS.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.jsonsrc/AudioSharp.App/wwwroot/app.css
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/deploy.yml:
- Line 62: The --set-secrets argument uses an unbraced shell variable
(OPENROUTER_API_KEY_SECRET) which can fail to expand; update the deployment step
so the secret reference uses brace expansion, e.g., replace
OPENROUTER_API_KEY=$OPENROUTER_API_KEY_SECRET:latest with
OPENROUTER_API_KEY=${OPENROUTER_API_KEY_SECRET}:latest to ensure correct shell
variable expansion in the --set-secrets flag.
In `@src/AudioSharp.App/Components/Layout/ReconnectModal.razor.js`:
- Around line 1-6: The top-level `return` causes a syntax error; wrap the module
initialization in an IIFE (or a top-level async IIFE if awaiting) so early-exit
logic can use `return` inside a function: move the existing setup (accessing
`reconnectModal`, `console.warn` etc.) into an immediately-invoked function
expression that checks `document.getElementById("components-reconnect-modal")`
and returns early if falsy, or alternatively replace the `return` with a
conditional guard that only runs the initialization when `reconnectModal` is
present; reference the `reconnectModal` variable and any subsequent handler
setup to locate where to wrap/guard.
🧹 Nitpick comments (8)
src/AudioSharp.App/wwwroot/app.css (2)
2-2: Consider preloading critical fonts to reduce render-blocking.The
@importfor Google Fonts at the top of the CSS is render-blocking. While the URL includesdisplay=swap, pairing it with a<link rel="preload">in the HTML<head>can improve perceived performance by starting the font fetch earlier.<link rel="preload" href="https://fonts.googleapis.com/css2?family=DM+Serif+Display&family=Space+Grotesk:wght@400;500;600;700&display=swap" as="style" />
1-2: Addwwwroot/app.cssto.gitignoreto prevent merge conflicts and stale CSS.Your CI/CD pipeline already runs
npm run build:cssin bothci.ymlanddeploy.ymlon every PR and push. Committing the compiled output creates unnecessary merge conflicts and requires developers to manually sync the artifact locally. Since the build is automated on every push to main and during deployments, you can safely ignore this file and let the pipeline regenerate it fresh each time.Developers can use
npm run watch:cssduring local development, and the artifact will be regenerated by CI/CD before any build/test/deploy steps.src/AudioSharp.App/Data/ConcernRepository.cs (1)
14-18: Consider validating the record parameter.For consistency with defensive coding practices, consider adding a null check for the
recordparameter, similar to what's done in other services in this PR.🛡️ Optional enhancement
public async Task<ConcernRecord> AddAsync(ConcernRecord record, CancellationToken cancellationToken) { + ArgumentNullException.ThrowIfNull(record); + _dbContext.ConcernRecords.Add(record); await _dbContext.SaveChangesAsync(cancellationToken).ConfigureAwait(false); return record; }src/AudioSharp.App/Services/FhirObservationMapper.cs (1)
86-121: Consider extracting note-building into a more compact pattern.The repetitive null-check and add pattern could be simplified, though current implementation is readable and maintainable.
♻️ Optional: More compact approach
private static List<FhirAnnotation> BuildNotes(ConcernItem concern) { - var notes = new List<FhirAnnotation>(); - - if (!string.IsNullOrWhiteSpace(concern.Severity)) - { - notes.Add(new FhirAnnotation($"Severity: {concern.Severity}")); - } - - if (!string.IsNullOrWhiteSpace(concern.Onset)) - { - notes.Add(new FhirAnnotation($"Onset: {concern.Onset}")); - } - // ... etc - - return notes; + var entries = new (string Label, string? Value)[] + { + ("Severity", concern.Severity), + ("Onset", concern.Onset), + ("Duration", concern.Duration), + ("Impact", concern.Impact), + ("Context", concern.Context), + ("Quote", concern.PatientQuote is { Length: > 0 } q ? $"\"{q}\"" : null) + }; + + return entries + .Where(e => !string.IsNullOrWhiteSpace(e.Value)) + .Select(e => new FhirAnnotation($"{e.Label}: {e.Value}")) + .ToList(); }src/AudioSharp.App/wwwroot/js/audio-recorder.js (1)
131-139: Remove dead code from ScriptProcessor fallback.Lines 136-138 reference
processor.onaudioprocess, which was part of the deprecated ScriptProcessor API. Since AudioWorklet is now exclusively used, this branch is unreachable dead code.🧹 Proposed cleanup
if (processor) { processor.disconnect(); if (processor.port) { processor.port.postMessage({ command: "stop" }); processor.port.onmessage = null; - } else { - processor.onaudioprocess = null; } }src/AudioSharp.App/Components/Pages/Home.razor (1)
637-648: Consider potential race with concurrentResetProcessingcalls.If a user rapidly triggers operations (e.g., clicking buttons in quick succession), the sequence
Cancel → Dispose → new CTScould race with an in-flight task that still holds a reference to the oldProcessingCts. The task would then try to access a disposed object.This is a minor concern given typical user interaction patterns, but wrapping with a lock or using
Interlocked.Exchangecould make it more robust.src/AudioSharp.App/Program.cs (2)
157-177: Consider logging a warning when OpenRouter API key is missing.Unlike the FHIR server configuration (which logs a warning when
BaseUrlis empty), the OpenRouter client silently proceeds without an API key. While this may work for local models, logging a warning could help debug configuration issues in production.Optional: Add warning for missing API key
if (!string.IsNullOrWhiteSpace(options.ApiKey)) { client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", options.ApiKey); } + else + { + var logger = serviceProvider.GetRequiredService<ILoggerFactory>() + .CreateLogger("OpenRouter"); + logger.LogWarning("OpenRouter API key is not configured; API calls may fail."); + }
327-334: Consider using migrations instead ofEnsureCreatedAsyncfor production readiness.
EnsureCreatedAsyncworks for development but doesn't support schema evolution. When the app moves to production, you'll want to use EF Core migrations for proper database versioning.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/workflows/deploy.ymlDockerfilesrc/AudioSharp.App/Components/Layout/ReconnectModal.razor.jssrc/AudioSharp.App/Components/Pages/Home.razorsrc/AudioSharp.App/Components/Pages/NotFound.razorsrc/AudioSharp.App/Data/ConcernRecord.cssrc/AudioSharp.App/Data/ConcernRepository.cssrc/AudioSharp.App/Options/RecordingOptions.cssrc/AudioSharp.App/Program.cssrc/AudioSharp.App/Services/AudioFormatHelper.cssrc/AudioSharp.App/Services/FhirObservationMapper.cssrc/AudioSharp.App/Styles/app.csssrc/AudioSharp.App/appsettings.Development.jsonsrc/AudioSharp.App/appsettings.jsonsrc/AudioSharp.App/wwwroot/app.csssrc/AudioSharp.App/wwwroot/js/audio-capture-worklet.jssrc/AudioSharp.App/wwwroot/js/audio-recorder.js
🚧 Files skipped from review as they are similar to previous changes (6)
- src/AudioSharp.App/appsettings.json
- src/AudioSharp.App/Options/RecordingOptions.cs
- src/AudioSharp.App/Data/ConcernRecord.cs
- src/AudioSharp.App/Services/AudioFormatHelper.cs
- src/AudioSharp.App/Styles/app.css
- Dockerfile
🧰 Additional context used
🧬 Code graph analysis (2)
src/AudioSharp.App/Services/FhirObservationMapper.cs (3)
src/AudioSharp.App/Options/FhirMappingOptions.cs (1)
FhirMappingOptions(3-11)src/AudioSharp.App/Models/Fhir/FhirObservation.cs (1)
FhirObservation(5-33)src/AudioSharp.App/Models/Fhir/FhirCoding.cs (1)
FhirCoding(5-15)
src/AudioSharp.App/Program.cs (7)
src/AudioSharp.App/wwwroot/js/audio-recorder.js (1)
options(15-15)src/AudioSharp.App/Options/OpenRouterOptions.cs (1)
OpenRouterOptions(3-13)src/AudioSharp.App/Options/FhirServerOptions.cs (1)
FhirServerOptions(3-10)src/AudioSharp.App/Options/LmStudioOptions.cs (1)
LmStudioOptions(3-9)src/AudioSharp.App/Options/LlmProviderOptions.cs (1)
LlmProviderOptions(9-14)src/AudioSharp.App/Options/FhirMappingOptions.cs (1)
FhirMappingOptions(3-11)src/AudioSharp.App/Options/RecordingOptions.cs (1)
RecordingOptions(5-14)
🪛 Biome (2.1.2)
src/AudioSharp.App/Components/Layout/ReconnectModal.razor.js
[error] 5-5: Illegal return statement outside of a function
(parse)
🔇 Additional comments (18)
src/AudioSharp.App/Components/Pages/NotFound.razor (1)
1-5: LGTM!The NotFound page is correctly implemented. The previous accessibility issue has been addressed—the heading is now
<h1>, which properly matches theFocusOnNavigateselector inRoutes.razorfor correct focus management on navigation.src/AudioSharp.App/wwwroot/app.css (1)
2-2: Focus-visible accessibility styles are now in place — nicely done.The
:focus-visiblerules for interactive elements (fast-button,fast-text-field,fast-text-area,input,textarea,.nav-link) with a fallback via@supports not selector(:focus-visible)satisfy WCAG 2.4.7 (Focus Visible). This addresses the previous accessibility concern.src/AudioSharp.App/Data/ConcernRepository.cs (1)
21-29: LGTM! Previous performance concern has been addressed.The query now correctly pushes
OrderByDescendingandTaketo the database level, avoiding loading the entire table into memory.src/AudioSharp.App/wwwroot/js/audio-capture-worklet.js (1)
1-38: LGTM! Well-structured AudioWorklet implementation.Good practices observed:
- Efficient zero-copy transfer using transferable objects (
[copy.buffer])- Proper guard clauses for missing input data
- Clean state management via port messaging
.github/workflows/deploy.yml (2)
55-62: Deployment configuration looks good overall.The workflow correctly addresses previous review feedback:
- Image tag is now constructed in a shell step (line 48)
- API key now uses
--set-secretsinstead of--set-env-vars
36-37: This review comment is based on outdated information..NET 10 was released as a stable, generally available (GA) version with LTS support on November 11, 2025, and the SDK is production-ready. The latest available version is 10.0.1 (SDK 10.0.101, updated December 9, 2025). Using
dotnet-version: 10.0.xin a production deployment workflow is appropriate and recommended.Likely an incorrect or invalid review comment.
src/AudioSharp.App/Services/FhirObservationMapper.cs (1)
17-44: LGTM! Previous null-check concerns have been addressed.The
Mapmethod now properly validates bothconcernsandcontextparameters withArgumentNullException.ThrowIfNull. The mapping logic is clean and correctly produces FHIR-compliant observations.src/AudioSharp.App/wwwroot/js/audio-recorder.js (3)
1-122: LGTM! Solid AudioWorklet-based audio capture implementation.Good practices observed:
- Clean module pattern with proper encapsulation
- Comprehensive error handling with cleanup on failure paths
- Antiforgery token caching for efficiency
- Proper audio graph management (source → processor → zeroGain → destination)
The migration from deprecated ScriptProcessor to AudioWorklet addresses the previous review concern.
243-270: Downsampling uses simple averaging.The current averaging approach works well for voice/speech but may introduce aliasing artifacts for other audio content. This is acceptable for the transcription use case.
281-306: WAV encoding is correct.The RIFF/WAV header construction follows the standard format for mono 16-bit PCM audio. Header fields and byte ordering are correct.
src/AudioSharp.App/Components/Layout/ReconnectModal.razor.js (1)
15-67: LGTM on the reconnect handling logic.The state handling, retry/resume flows, and visibility-based retry mechanism are well-structured with appropriate error handling.
src/AudioSharp.App/appsettings.Development.json (1)
1-24: LGTM!Development configuration is well-structured with sensible defaults. The empty
FhirServer:BaseUrlappropriately disables FHIR uploads in development, and the local LM Studio endpoint is correctly configured for local testing.src/AudioSharp.App/Components/Pages/Home.razor (3)
1-23: LGTM on directives and dependency injection setup.The page correctly sets up interactive server rendering and injects all necessary services for the audio capture, concern extraction, and FHIR workflow.
227-256: Clean state management design.The state properties, cancellation token sources, and computed properties are well-organized. Good use of C# 12 collection expressions for initialization.
714-719: LGTM on disposal.Both
CancellationTokenSourceinstances are properly cleaned up.src/AudioSharp.App/Program.cs (3)
16-50: Well-configured service setup.Antiforgery, form/SignalR limits, circuit timeouts, and OpenTelemetry are properly configured with sensible values for the audio processing use case.
52-109: Environment variable overrides are comprehensive.Good pattern for allowing runtime configuration of sensitive/deployment-specific values without code changes. The fallback from
OPENROUTER_MODELto both audio and text models is a nice touch.
244-325: Solid endpoint implementation with proper security and error handling.The
ProcessAudioAsynchandler correctly validates antiforgery, handles multipart form data, and provides appropriate status codes for different error scenarios (400/408/500).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| --region ${{ env.REGION }} \ | ||
| --allow-unauthenticated \ | ||
| --set-env-vars LlmProvider__TextProvider=OpenRouter,OPENROUTER_TEXT_MODEL=google/gemini-2.5-flash-lite,OPENROUTER_AUDIO_MODEL=google/gemini-2.5-flash-lite \ | ||
| --set-secrets OPENROUTER_API_KEY=$OPENROUTER_API_KEY_SECRET:latest |
There was a problem hiding this comment.
Fix the --set-secrets shell variable expansion.
The secret reference $OPENROUTER_API_KEY_SECRET lacks curly braces and may not expand correctly in the shell context. Use ${OPENROUTER_API_KEY_SECRET} for proper expansion.
🐛 Proposed fix
- --set-secrets OPENROUTER_API_KEY=$OPENROUTER_API_KEY_SECRET:latest
+ --set-secrets OPENROUTER_API_KEY=${OPENROUTER_API_KEY_SECRET}:latest📝 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.
| --set-secrets OPENROUTER_API_KEY=$OPENROUTER_API_KEY_SECRET:latest | |
| --set-secrets OPENROUTER_API_KEY=${OPENROUTER_API_KEY_SECRET}:latest |
🤖 Prompt for AI Agents
In @.github/workflows/deploy.yml at line 62, The --set-secrets argument uses an
unbraced shell variable (OPENROUTER_API_KEY_SECRET) which can fail to expand;
update the deployment step so the secret reference uses brace expansion, e.g.,
replace OPENROUTER_API_KEY=$OPENROUTER_API_KEY_SECRET:latest with
OPENROUTER_API_KEY=${OPENROUTER_API_KEY_SECRET}:latest to ensure correct shell
variable expansion in the --set-secrets flag.
| // Set up event handlers | ||
| const reconnectModal = document.getElementById("components-reconnect-modal"); | ||
| if (!reconnectModal) { | ||
| console.warn("Reconnect modal element not found."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Syntax error: return statement at module top level.
The return statement on line 5 is invalid in ES modules—it's only allowed inside functions. This will cause a parse error and prevent the script from loading.
Wrap the initialization logic in an IIFE or use a different early-exit pattern:
🛠️ Proposed fix using IIFE
-// Set up event handlers
-const reconnectModal = document.getElementById("components-reconnect-modal");
-if (!reconnectModal) {
- console.warn("Reconnect modal element not found.");
- return;
-}
-reconnectModal.addEventListener("components-reconnect-state-changed", handleReconnectStateChanged);
-
-const retryButton = document.getElementById("components-reconnect-button");
-retryButton?.addEventListener("click", retry);
-
-const resumeButton = document.getElementById("components-resume-button");
-resumeButton?.addEventListener("click", resume);
+// Set up event handlers
+(function init() {
+ const reconnectModal = document.getElementById("components-reconnect-modal");
+ if (!reconnectModal) {
+ console.warn("Reconnect modal element not found.");
+ return;
+ }
+ reconnectModal.addEventListener("components-reconnect-state-changed", handleReconnectStateChanged);
+
+ const retryButton = document.getElementById("components-reconnect-button");
+ retryButton?.addEventListener("click", retry);
+
+ const resumeButton = document.getElementById("components-resume-button");
+ resumeButton?.addEventListener("click", resume);
+})();🧰 Tools
🪛 Biome (2.1.2)
[error] 5-5: Illegal return statement outside of a function
(parse)
🤖 Prompt for AI Agents
In `@src/AudioSharp.App/Components/Layout/ReconnectModal.razor.js` around lines 1
- 6, The top-level `return` causes a syntax error; wrap the module
initialization in an IIFE (or a top-level async IIFE if awaiting) so early-exit
logic can use `return` inside a function: move the existing setup (accessing
`reconnectModal`, `console.warn` etc.) into an immediately-invoked function
expression that checks `document.getElementById("components-reconnect-modal")`
and returns early if falsy, or alternatively replace the `return` with a
conditional guard that only runs the initialization when `reconnectModal` is
present; reference the `reconnectModal` variable and any subsequent handler
setup to locate where to wrap/guard.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/AudioSharp.App/AudioSharp.App.csproj`:
- Around line 23-29: The CopyRecordRtc target currently silently skips when the
file at node_modules/recordrtc/RecordRTC.min.js is missing; modify the
AudioSharp.App.csproj to detect
!Exists('$(RepositoryRoot)/node_modules/recordrtc/RecordRTC.min.js') and emit an
explicit MSBuild Message (Importance="High") warning and/or an Error to fail CI;
update the CopyRecordRtc target (or add an adjacent target) to call <Message>
when missing and use an <Error> conditioned on a CI flag (e.g., '$(CI)' ==
'true') so local builds warn but CI builds fail, referencing the existing target
name CopyRecordRtc and the path node_modules/recordrtc/RecordRTC.min.js to
locate where to add the checks.
- Around line 15-21: Update the PackageReference for
Microsoft.EntityFrameworkCore.Sqlite in the project file by changing its Version
attribute from "10.0.1" to "10.0.2" (look for the PackageReference
Include="Microsoft.EntityFrameworkCore.Sqlite" entry) so the EF Core Sqlite
package matches the latest stable patch release.
In `@src/AudioSharp.App/wwwroot/js/audio-recorder.js`:
- Around line 174-175: Wrap the JSON.parse(responseText) call in a try-catch in
audio-recorder.js so malformed or empty server responses don't throw uncaught
exceptions; attempt to JSON.parse(responseText) inside try, handle empty string
by returning null or {}, and in catch log or throw a new descriptive error that
includes the responseText and context (e.g., "Failed to parse server response in
<function>") so callers get a clear message and can handle the failure.
🧹 Nitpick comments (2)
.gitignore (1)
1-10: LGTM! Well-structured .gitignore for the project stack.The ignore patterns appropriately cover environment files, build artifacts, Node.js dependencies, SQLite temporary files, and Google Cloud configuration. The entries align well with the Blazor + Node.js + Docker + Cloud Run setup described in the PR.
💡 Optional: Consider adding common .NET/Visual Studio patterns
For improved developer experience across different IDEs and setups, you might consider adding:
+# Visual Studio / Rider +.vs/ +.idea/ +*.user +*.suo +*.log + +# Test results +TestResults/ +coverage/These are purely optional and can be added incrementally as needed.
src/AudioSharp.App/AudioSharp.App.csproj (1)
31-40: Duplicate RecordRTC copy logic and missing error handling.Two observations:
DRY violation: Lines 36-39 duplicate the MakeDir/Copy logic from
CopyRecordRtc(lines 25-28). Consider extracting to a shared target or inline items.Potential failure: If
npm cifails ornode_modulesalready exists but is corrupted/incomplete, the subsequentCopytask will fail with a cryptic error. Thenpm cicondition only checks directory existence, not content validity.♻️ Suggested refactor to reduce duplication
+ <PropertyGroup> + <RecordRtcSource>$(RepositoryRoot)/node_modules/recordrtc/RecordRTC.min.js</RecordRtcSource> + <RecordRtcDest>$(MSBuildProjectDirectory)/wwwroot/lib/recordrtc/recordrtc.min.js</RecordRtcDest> + </PropertyGroup> + <Target Name="CopyRecordRtc" BeforeTargets="Build" - Condition="Exists('$(RepositoryRoot)/node_modules/recordrtc/RecordRTC.min.js')"> - <MakeDir Directories="$(MSBuildProjectDirectory)/wwwroot/lib/recordrtc" /> - <Copy SourceFiles="$(RepositoryRoot)/node_modules/recordrtc/RecordRTC.min.js" - DestinationFiles="$(MSBuildProjectDirectory)/wwwroot/lib/recordrtc/recordrtc.min.js" + Condition="Exists('$(RecordRtcSource)')"> + <MakeDir Directories="$([System.IO.Path]::GetDirectoryName('$(RecordRtcDest)'))" /> + <Copy SourceFiles="$(RecordRtcSource)" + DestinationFiles="$(RecordRtcDest)" SkipUnchangedFiles="true" /> </Target> <Target Name="PrepareFrontendAssets" BeforeTargets="Publish"> <Message Importance="high" Text="Preparing frontend assets..." /> <Exec Command="npm ci" WorkingDirectory="$(RepositoryRoot)" Condition="!Exists('$(RepositoryRoot)/node_modules')" /> <Exec Command="npm run build:css" WorkingDirectory="$(RepositoryRoot)" Condition="'$(SkipTailwindCss)' != 'true'" /> - <MakeDir Directories="$(MSBuildProjectDirectory)/wwwroot/lib/recordrtc" /> - <Copy SourceFiles="$(RepositoryRoot)/node_modules/recordrtc/RecordRTC.min.js" - DestinationFiles="$(MSBuildProjectDirectory)/wwwroot/lib/recordrtc/recordrtc.min.js" - SkipUnchangedFiles="true" /> + <CallTarget Targets="CopyRecordRtc" /> </Target>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.gitignorepackage.jsonsrc/AudioSharp.App/AudioSharp.App.csprojsrc/AudioSharp.App/Components/App.razorsrc/AudioSharp.App/wwwroot/js/audio-recorder.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/AudioSharp.App/Components/App.razor
- package.json
🔇 Additional comments (4)
src/AudioSharp.App/AudioSharp.App.csproj (2)
11-13: LGTM!The
RepositoryRootcalculation correctly navigates two levels up from the project directory, matching thesrc/AudioSharp.App/structure.
3-9: No action required. The project correctly targetsnet10.0and the Dockerfile is properly aligned withDOTNET_VERSION=10.0for both the build (SDK) and final (runtime) stages. The configuration is consistent across build environments.src/AudioSharp.App/wwwroot/js/audio-recorder.js (2)
76-93: Good migration to RecordRTC.Using RecordRTC with
StereoAudioRecorderis the correct modern approach for audio capture. This avoids the deprecatedcreateScriptProcessorand moves audio processing off the main thread, improving performance and reliability.
38-50: Proper resource cleanup.The cleanup function correctly stops all media tracks and destroys the recorder instance, preventing resource leaks.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Addressed CodeRabbit feedback in commit 82e6dea: RecordRTC asset checks/warnings with CI error, EF Core Sqlite bump to 10.0.2, JSON response parsing guard in audio-recorder.js, plus optional .editorconfig UTF-8 charset and .gitignore IDE/test output ignores. |
Adds app, FAST UI, telemetry, Dockerfile, and GitHub Actions CI/CD.
Summary by CodeRabbit
New Features
Style
Chores
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.