Fix/corrige build docker#16
Conversation
…issões de bibliotecas
There was a problem hiding this comment.
Pull request overview
This PR addresses Docker deployment issues and adds mobile audio support with GPU acceleration for the Ollama container. The changes include critical fixes for Docker builds, a new audio decoding pipeline for mobile clients, and CORS configuration for cross-origin requests.
Key Changes:
- Enables Nvidia GPU support for the Ollama container in Docker Compose
- Adds AAC-to-PCM audio decoding for mobile audio input using FFmpeg
- Configures CORS to allow mobile app requests and adds automatic database migrations on startup
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| compose.yaml | Removed HolidaysCsvPath environment variable, added timezone configuration, and configured Nvidia GPU support for Ollama |
| Dockerfile | Added native dependencies for Whisper, fixed permission issues, and added holidays CSV file copying with path configuration |
| Program.cs | Added DbContext DI registration, CORS configuration with unrestricted origin policy, automatic database migrations, and startup model download |
| MelissaHub.cs | Extracted model configuration to constants, added new AskMelissaAudioFromMobile endpoint with AAC decoding, significant code duplication between audio methods |
| AudioDecoder.cs | New static class for AAC-to-PCM conversion using FFmpeg (not installed in Docker image) |
| AppEndpoints.cs | Added GetNationalHolidaysAsTxt method to return holidays as string instead of file export |
| HolidayService.cs | Implemented GetNationalHolidaysAsTxt to query and format national holidays |
| SettingsWindow.axaml | Removed trailing whitespace |
| Melissa.cs | Updated system message to exclude formatting/emojis (contains spelling error) |
Comments suppressed due to low confidence (2)
src/Melissa/Melissa.WebServer/MelissaHub.cs:178
- Using the null-forgiving operator (
!) oncnVoicewithout null checking is unsafe. If no voice matching "pt-BR-FranciscaNeural" is found,cnVoicewill be null and accessingcnVoice!.Namewill throw aNullReferenceException.
Add proper null checking:
var voices = await edgeTts.GetVoices();
var cnVoice = voices.FirstOrDefault(v => v.ShortName == "pt-BR-FranciscaNeural");
if (cnVoice == null)
{
Log.Warning("Voice pt-BR-FranciscaNeural not found, using default voice");
cnVoice = voices.FirstOrDefault() ?? throw new InvalidOperationException("No voices available");
}
var options = new TTSOption
(
voice: cnVoice.Name,
// ...
); var voices = await edgeTts.GetVoices();
var cnVoice = voices.FirstOrDefault(v => v.ShortName == "pt-BR-FranciscaNeural");
var options = new TTSOption
(
voice: cnVoice!.Name,
src/Melissa/Melissa.WebServer/MelissaHub.cs:206
- There's significant code duplication between
AskMelissaAudioandAskMelissaAudioFromMobile. The only difference is the AAC decoding step (lines 132-134). The rest of the transcription, assistant interaction, and TTS logic is identical (90+ lines duplicated).
Consider refactoring to extract the common logic into a shared private method:
private async IAsyncEnumerable<byte[]> ProcessAudioQuery(
byte[] pcmBytes,
MelissaAssistant melissa,
[EnumeratorCancellation] CancellationToken cancellationToken)
{
// Common logic for transcription, assistant query, and TTS
}Then call it from both methods after obtaining the PCM bytes.
public async IAsyncEnumerable<byte[]> AskMelissaAudioFromMobile(IAsyncEnumerable<byte[]> audioStream,
[FromServices] MelissaAssistant melissa,
[EnumeratorCancellation] CancellationToken cancellationToken = default)
{
using var ms = new MemoryStream();
await foreach (var chunk in audioStream.WithCancellation(cancellationToken))
{
await ms.WriteAsync(chunk, cancellationToken);
}
// ✅ NOVO: Decodificar AAC para PCM
var aacBytes = ms.ToArray();
var pcmBytes = await AudioDecoder.DecodeAACToPCM(aacBytes);
if (!File.Exists(ModelFileName))
{
await DownloadModel(ModelFileName, GgmlType);
}
using var whisperFactory = WhisperFactory.FromPath("ggml-medium.bin");
await using var processor = whisperFactory.CreateBuilder()
.WithLanguage("pt")
.Build();
// ✅ Usar PCM decodificado em vez de tentar converter
var wavBytes = GenerateWav(pcmBytes);
var wavStream = new MemoryStream(wavBytes);
wavStream.Seek(0, SeekOrigin.Begin);
Log.Information("Iniciando transcrição de áudio...");
var msgBuilder = new StringBuilder();
await foreach (var result in processor.ProcessAsync(wavStream, cancellationToken))
{
msgBuilder.Append(result.Text);
}
var message = msgBuilder.ToString();
Log.Information("Usuário: {0}", message);
var question = new Question(message, "AudioHub", DateTimeOffset.Now);
string melissaReply;
var (isAvailable, statusMessage) = await melissa.CanUse();
if (isAvailable)
melissaReply =
await MelissaHub.SafeAskMelissaWithErrorHandlingAndRetry(melissa, question, cancellationToken);
else
melissaReply = statusMessage;
var edgeTts = new EdgeTTSNet();
var voices = await edgeTts.GetVoices();
var cnVoice = voices.FirstOrDefault(v => v.ShortName == "pt-BR-FranciscaNeural");
var options = new TTSOption
(
voice: cnVoice!.Name,
pitch: "+0Hz",
rate: "+25%",
volume: "+0%"
);
Log.Information("Assistente: {0}", melissaReply);
edgeTts = new EdgeTTSNet(options);
var channel = Channel.CreateUnbounded<byte[]>();
_ = Task.Run(async () =>
{
await edgeTts.TTS(melissaReply, (metaObj) =>
{
if (metaObj.Type == TTSMetadataType.Audio)
{
channel.Writer.TryWrite(metaObj.Data);
}
}, cancellationToken);
channel.Writer.Complete();
}, cancellationToken);
await foreach (var audioChunk in channel.Reader.ReadAllAsync(cancellationToken))
{
yield return audioChunk;
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var assistantFactory = new AssistantFactory(); | ||
| var melissa = await assistantFactory.TryCreateMelissa(TimeSpan.FromSeconds(10)); | ||
|
|
||
| await MelissaHub.DownloadModel(MelissaHub.ModelFileName, MelissaHub.GgmlType); |
There was a problem hiding this comment.
The model download is called synchronously during application startup before the services are built. If the model download fails or takes a long time, it will block the entire application startup process and could cause deployment failures.
Consider moving this to a background service or lazy loading it on first use, with proper error handling:
// Remove this line from here
// await MelissaHub.DownloadModel(MelissaHub.ModelFileName, MelissaHub.GgmlType);The model is already downloaded on-demand in AskMelissaAudio and AskMelissaAudioFromMobile (lines 46-48, 136-138 in MelissaHub.cs), so this pre-download is redundant.
| await MelissaHub.DownloadModel(MelissaHub.ModelFileName, MelissaHub.GgmlType); |
| RUN mkdir -p /app/Data | ||
| COPY ./Melissa.WebServer/Data/holidays_2025.csv /app/Data/holidays_2025.csv |
There was a problem hiding this comment.
The removal of the HolidaysCsvPath environment variable from compose.yaml combined with the Dockerfile copying the file to /app/Data/holidays_2025.csv (capital D) creates a path mismatch.
The application defaults to "data/holidays_2025.csv" (lowercase 'd') when HolidaysCsvPath is not set (line 73 in Program.cs), which will be resolved to /app/data/holidays_2025.csv at runtime. However, the Dockerfile copies the file to /app/Data/holidays_2025.csv (capital 'D').
This mismatch will cause the file not to be found. Either:
- Re-add the environment variable in compose.yaml:
- HolidaysCsvPath=/app/Data/holidays_2025.csvOR
- Change the Dockerfile to use lowercase:
RUN mkdir -p /app/data
COPY ./Melissa.WebServer/Data/holidays_2025.csv /app/data/holidays_2025.csv| RUN mkdir -p /app/Data | |
| COPY ./Melissa.WebServer/Data/holidays_2025.csv /app/Data/holidays_2025.csv | |
| RUN mkdir -p /app/data | |
| COPY ./Melissa.WebServer/Data/holidays_2025.csv /app/data/holidays_2025.csv |
| using (var process = System.Diagnostics.Process.Start(processInfo)) | ||
| { | ||
| await process.WaitForExitAsync(); | ||
| if (process.ExitCode != 0) | ||
| { | ||
| var error = process.StandardError.ReadToEnd(); | ||
| throw new InvalidOperationException($"FFmpeg failed: {error}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
The Process object is not properly disposed. While the using statement is used, the process should be explicitly disposed to ensure resources are released properly.
Change:
using var process = System.Diagnostics.Process.Start(processInfo);
await process.WaitForExitAsync();
if (process.ExitCode != 0)
{
var error = await process.StandardError.ReadToEndAsync();
throw new InvalidOperationException($"FFmpeg failed: {error}");
}This ensures the process resources are cleaned up via the using statement's Dispose call.
| using (var process = System.Diagnostics.Process.Start(processInfo)) | |
| { | |
| await process.WaitForExitAsync(); | |
| if (process.ExitCode != 0) | |
| { | |
| var error = process.StandardError.ReadToEnd(); | |
| throw new InvalidOperationException($"FFmpeg failed: {error}"); | |
| } | |
| } | |
| var process = System.Diagnostics.Process.Start(processInfo); | |
| if (process == null) | |
| throw new InvalidOperationException("Failed to start FFmpeg process."); | |
| try | |
| { | |
| await process.WaitForExitAsync(); | |
| if (process.ExitCode != 0) | |
| { | |
| var error = await process.StandardError.ReadToEndAsync(); | |
| throw new InvalidOperationException($"FFmpeg failed: {error}"); | |
| } | |
| } | |
| finally | |
| { | |
| process.Dispose(); | |
| } |
| using (var process = System.Diagnostics.Process.Start(processInfo)) | ||
| { | ||
| await process.WaitForExitAsync(); | ||
| if (process.ExitCode != 0) | ||
| { | ||
| var error = process.StandardError.ReadToEnd(); | ||
| throw new InvalidOperationException($"FFmpeg failed: {error}"); | ||
| } |
There was a problem hiding this comment.
The AudioDecoder.DecodeUsingFFmpeg method executes an external FFmpeg process without proper input validation or sanitization. While the current implementation uses GUIDs for temp files, there's a potential command injection risk if the file paths are manipulated.
Additionally:
- There's no check if FFmpeg is installed, which will cause runtime failures
- The error message from FFmpeg is read using
ReadToEnd()after the process has exited, butStandardErrorshould be read asynchronously to avoid deadlocks - Temp files may not be cleaned up if an exception occurs before the
finallyblock
Consider:
if (process.ExitCode != 0)
{
var error = await process.StandardError.ReadToEndAsync();
throw new InvalidOperationException($"FFmpeg failed: {error}");
}Also consider checking for FFmpeg availability at startup or providing a clear error message.
| /// Decodifica AAC/M4A para PCM 16-bit | ||
| /// </summary> |
There was a problem hiding this comment.
Missing documentation for the public method DecodeAACToPCM. Public APIs should have XML documentation comments explaining:
- What the method does
- The expected format/encoding of the input
aacData - What PCM format is returned (sample rate, bit depth, channels)
- What exceptions can be thrown (e.g., if FFmpeg is not installed)
Example:
/// <summary>
/// Decodes AAC/M4A audio data to PCM 16-bit format.
/// </summary>
/// <param name="aacData">The AAC/M4A encoded audio data.</param>
/// <returns>PCM audio data in 16-bit, 16kHz, mono format.</returns>
/// <exception cref="InvalidOperationException">Thrown when FFmpeg fails to decode the audio or is not installed.</exception>
public static async Task<byte[]> DecodeAACToPCM(byte[] aacData)| /// Decodifica AAC/M4A para PCM 16-bit | |
| /// </summary> | |
| /// Decodes AAC or M4A encoded audio data to raw PCM 16-bit audio. | |
| /// </summary> | |
| /// <param name="aacData">The AAC or M4A encoded audio data as a byte array.</param> | |
| /// <returns> | |
| /// A byte array containing the decoded PCM audio data in 16-bit, 16kHz, mono format (little-endian). | |
| /// </returns> | |
| /// <exception cref="InvalidOperationException"> | |
| /// Thrown when FFmpeg fails to decode the audio or is not installed on the system. | |
| /// </exception> |
| if (File.Exists(tempAacFile)) File.Delete(tempAacFile); | ||
| if (File.Exists(tempPcmFile)) File.Delete(tempPcmFile); |
There was a problem hiding this comment.
Temporary files created for FFmpeg processing may not be cleaned up if an exception occurs during file writing or before the FFmpeg process starts. The finally block only runs if execution reaches the try block successfully.
Consider wrapping file operations in try-catch or using a more robust cleanup approach:
var tempAacFile = Path.Combine(Path.GetTempPath(), $"temp_{Guid.NewGuid()}.m4a");
var tempPcmFile = Path.Combine(Path.GetTempPath(), $"temp_{Guid.NewGuid()}.pcm");
try
{
await File.WriteAllBytesAsync(tempAacFile, aacData);
// ... rest of the code
}
finally
{
try { if (File.Exists(tempAacFile)) File.Delete(tempAacFile); } catch { }
try { if (File.Exists(tempPcmFile)) File.Delete(tempPcmFile); } catch { }
}| if (File.Exists(tempAacFile)) File.Delete(tempAacFile); | |
| if (File.Exists(tempPcmFile)) File.Delete(tempPcmFile); | |
| try { if (File.Exists(tempAacFile)) File.Delete(tempAacFile); } catch { } | |
| try { if (File.Exists(tempPcmFile)) File.Delete(tempPcmFile); } catch { } |
| builder.Services.AddSignalR(options => { options.DisableImplicitFromServicesParameters = true; }); | ||
|
|
||
| // Registrar DbContext no DI container | ||
| builder.Services.AddDbContext<AppDbContext>(); |
There was a problem hiding this comment.
DbContext is registered in DI but most of the codebase still creates instances directly using new AppDbContext(). This leads to:
- Inconsistent usage patterns
- Potential connection leaks
- The DI registration having no effect except for the migration code
Either:
- Inject
AppDbContextwherever it's needed (recommended for proper DI and lifecycle management), OR - Remove the DI registration if you're not planning to use it consistently
Examples of direct instantiation can be found in:
HolidayService.csline 8TaskListService.csline 13DatabaseFeeder.cslines 16, 95, 107ConversationHistoryOllamaTool.csline 20
| builder.Services.AddDbContext<AppDbContext>(); | |
| // builder.Services.AddDbContext<AppDbContext>(); |
| var edgeTts = new EdgeTTSNet(); | ||
|
|
||
| var voices = await edgeTts.GetVoices(); | ||
| var cnVoice = voices.FirstOrDefault(v => v.ShortName == "pt-BR-FranciscaNeural"); | ||
| var options = new TTSOption | ||
| ( | ||
| voice: cnVoice!.Name, | ||
| pitch: "+0Hz", | ||
| rate: "+25%", | ||
| volume: "+0%" | ||
| ); | ||
|
|
||
| Log.Information("Assistente: {0}", melissaReply); | ||
|
|
||
|
|
||
| edgeTts = new EdgeTTSNet(options); |
There was a problem hiding this comment.
EdgeTTSNet is instantiated twice - once to get voices (line 84) and again with options (line 99). The first instance is never disposed, potentially leaking resources.
Refactor to use a single instance:
var voices = await EdgeTTSNet.GetVoicesAsync(); // If static method exists
// OR
using var edgeTtsForVoices = new EdgeTTSNet();
var voices = await edgeTtsForVoices.GetVoices();
var cnVoice = voices.FirstOrDefault(v => v.ShortName == "pt-BR-FranciscaNeural");
// ... null check ...
var options = new TTSOption(
voice: cnVoice.Name,
pitch: "+0Hz",
rate: "+25%",
volume: "+0%"
);
using var edgeTts = new EdgeTTSNet(options);This issue appears in both audio methods.
| var wavStream = new MemoryStream(wavBytes); | ||
| wavStream.Seek(0, SeekOrigin.Begin); | ||
|
|
||
| Log.Information("Iniciando transcrição de áudio..."); | ||
|
|
||
| var msgBuilder = new StringBuilder(); | ||
| await foreach (var result in processor.ProcessAsync(wavStream, cancellationToken)) | ||
| { | ||
| msgBuilder.Append(result.Text); |
There was a problem hiding this comment.
Disposable 'MemoryStream' is created but not disposed.
| var wavStream = new MemoryStream(wavBytes); | |
| wavStream.Seek(0, SeekOrigin.Begin); | |
| Log.Information("Iniciando transcrição de áudio..."); | |
| var msgBuilder = new StringBuilder(); | |
| await foreach (var result in processor.ProcessAsync(wavStream, cancellationToken)) | |
| { | |
| msgBuilder.Append(result.Text); | |
| using (var wavStream = new MemoryStream(wavBytes)) | |
| { | |
| wavStream.Seek(0, SeekOrigin.Begin); | |
| Log.Information("Iniciando transcrição de áudio..."); | |
| var msgBuilder = new StringBuilder(); | |
| await foreach (var result in processor.ProcessAsync(wavStream, cancellationToken)) | |
| { | |
| msgBuilder.Append(result.Text); | |
| } |
| if (isAvailable) | ||
| melissaReply = | ||
| await MelissaHub.SafeAskMelissaWithErrorHandlingAndRetry(melissa, question, cancellationToken); | ||
| else | ||
| melissaReply = statusMessage; |
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 (isAvailable) | |
| melissaReply = | |
| await MelissaHub.SafeAskMelissaWithErrorHandlingAndRetry(melissa, question, cancellationToken); | |
| else | |
| melissaReply = statusMessage; | |
| melissaReply = isAvailable | |
| ? await MelissaHub.SafeAskMelissaWithErrorHandlingAndRetry(melissa, question, cancellationToken) | |
| : statusMessage; |
…assistente Melissa
Esse PR corrige problemas com o deploy usando docker
Tambem habilita o container do ollama a usar GPU Nvidia (nao testado para AMD)