Feature: recode voice #5
Conversation
Walkthrough오디오 녹음기가 마이크 장치 선택, 진행률 이벤트, 실제 녹음 길이 기반 트리밍과 선택적 잡음 제거를 적용해 WAV로 변환·저장하도록 확장되었고, UI는 진행막대를 표시하도록 변경되었으며, 네트워크는 JSON 우선·폼데이터 업로드와 파일 크기 검사 추가, STT 서비스는 인터페이스 제거 후 UniTask 기반으로 변경됨. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VoiceInputView
participant AudioRecorder
participant WavEncoder
participant STTService
participant HttpApiClient
participant Server
User->>VoiceInputView: 녹음 시작 요청
VoiceInputView->>AudioRecorder: StartRecording()
AudioRecorder-->>VoiceInputView: OnRecordingProgress(progress) [periodic]
User->>VoiceInputView: 녹음 중지
VoiceInputView->>AudioRecorder: StopRecording()
AudioRecorder->>AudioRecorder: ProcessRecordingClip(actualDuration)
AudioRecorder-->>VoiceInputView: 녹음 완료(clip)
VoiceInputView->>WavEncoder: FromAudioClip(clip)
WavEncoder-->>VoiceInputView: wavBytes
VoiceInputView->>STTService: ConvertSpeechToTextAsync(wavBytes, cancellationToken)
STTService->>HttpApiClient: PostFormDataAsync(endpoint, formData, fileNames, cancellationToken)
HttpApiClient->>Server: HTTP 요청 (JSON or multipart/form-data)
Server-->>HttpApiClient: 응답(JSON)
HttpApiClient-->>STTService: 파싱된 결과
STTService-->>VoiceInputView: 텍스트 반환
VoiceInputView-->>User: 결과 표시
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (9)
Assets/Infrastructure/Network/Configs/NetworkConfig.cs (3)
28-32: Harden inspector inputs for upload settings (add Min/Tooltip).Prevent invalid values and improve UX by constraining and documenting the fields in the Inspector.
Apply this diff:
[Header("File Upload Settings")] -[SerializeField] private int maxFileSize = 10485760; // 10MB (bytes) -[SerializeField] private float uploadTimeout = 60f; // 파일 업로드용 더 긴 타임아웃 -[SerializeField] private bool enableFileSizeCheck = true; +[Tooltip("Maximum allowed file size in bytes (default 10MB).")] +[Min(0)] +[SerializeField] private int maxFileSize = 10485760; // 10MB (bytes) +[Tooltip("Upload timeout in seconds (applies to form-data uploads).")] +[Min(0.1f)] +[SerializeField] private float uploadTimeout = 60f; // 파일 업로드용 더 긴 타임아웃 +[Tooltip("If true, client validates each file against MaxFileSize before upload.")] +[SerializeField] private bool enableFileSizeCheck = true;
186-190: Expose units/semantics in XML docs for clarity.Consider adding XML docs so call sites know units and behavior (bytes, seconds).
Apply this diff:
-// File Upload Settings +// File Upload Settings +/// <summary>Maximum allowed upload file size in bytes (per file).</summary> public static int MaxFileSize => Instance.maxFileSize; +/// <summary>Timeout in seconds for form-data uploads.</summary> public static float UploadTimeout => Instance.uploadTimeout; +/// <summary>When true, client-side enforcement of MaxFileSize is enabled.</summary> public static bool EnableFileSizeCheck => Instance.enableFileSizeCheck;
247-249: Defaults look sane. Consider using long if larger uploads are expected.10MB/60s defaults are reasonable for voice. If future uploads may exceed 2GB, switch to long for file size.
Assets/Infrastructure/Audio/WavEncoder.cs (2)
14-39: AudioClip → PCM16 conversion is correct; consider edge-case refinements.The implementation is solid. Two refinements to consider:
- Handle pathological inputs (NaN/Inf) defensively.
- Map -1.0f deterministically to short.MinValue (-32768) for symmetry.
Apply this diff:
- byte[] pcm16 = new byte[sampleCount * 2]; + byte[] pcm16 = new byte[sampleCount * 2]; int pcmIndex = 0; for (int i = 0; i < sampleCount; i++) { - float clamped = Mathf.Clamp(samples[i], -1f, 1f); - short s = (short)Mathf.RoundToInt(clamped * short.MaxValue); + var sample = samples[i]; + if (float.IsNaN(sample) || float.IsInfinity(sample)) + sample = 0f; + float clamped = Mathf.Clamp(sample, -1f, 1f); + // Map [-1, 1] to [-32768, 32767] with symmetric handling at -1 + int scaled = (int)Mathf.Round(clamped * 32767f); + if (clamped <= -1f) scaled = short.MinValue; + short s = (short)scaled; pcm16[pcmIndex++] = (byte)(s & 0xFF); pcm16[pcmIndex++] = (byte)((s >> 8) & 0xFF); }
44-91: WAV header construction is correct; add basic parameter validation.Guard against invalid channels/sampleRate to avoid malformed headers from upstream misuse.
Apply this diff:
public static byte[] WrapPcm16ToWav(byte[] pcm16Data, int channels, int sampleRate) { - if (pcm16Data == null || pcm16Data.Length == 0) + if (pcm16Data == null || pcm16Data.Length == 0) { return Array.Empty<byte>(); } + if (channels <= 0) throw new ArgumentOutOfRangeException(nameof(channels), "Channels must be > 0."); + if (sampleRate <= 0) throw new ArgumentOutOfRangeException(nameof(sampleRate), "SampleRate must be > 0.");Assets/Domain/Chat/View/VoiceInputView.cs (2)
301-308: Clamp progress and avoid toggling visibility on exact zero due to float jitter.Small robustness tweak to keep UI clean and avoid flicker around 0.
Apply this diff:
private void UpdateProgressBar(float progress) { if (_progressBar != null) { - _progressBar.value = progress; - _progressBar.gameObject.SetActive(progress > 0f); + var clamped = Mathf.Clamp01(progress); + _progressBar.value = clamped; + _progressBar.gameObject.SetActive(clamped > 0f); } }
243-251: Prefer dependency injection for STTService to keep the view testable.Direct instantiation ties the view to a concrete implementation. Consider allowing a setter/injector to pass a mock or alternative implementation.
Apply this diff to enable an override while preserving current behavior:
- if (_sttService == null) - { - _sttService = new STTService(); - if (_sttService == null) - { - Debug.LogError("[VoiceInputView] STTService를 생성할 수 없습니다."); - } - } + if (_sttService == null) + { + _sttService = new STTService(); + if (_sttService == null) + { + Debug.LogError("[VoiceInputView] STTService를 생성할 수 없습니다."); + } + }And add this method outside the shown range (to support DI without breaking current calls):
public void SetSTTService(STTService sttService) { _sttService = sttService; }Assets/Infrastructure/Network/Services/STTService.cs (1)
135-148: Unused DTOs taking up spaceThe
STTRequestclass is defined but never used in the implementation, as the service uses form data instead of JSON serialization.Remove unused code:
-/// <summary> -/// STT 요청 데이터 구조 -/// </summary> -[Serializable] -public class STTRequest -{ - [JsonProperty("file")] - public byte[] AudioData { get; set; } = new byte[0]; - - [JsonProperty("filename")] - public string Filename { get; set; } = "recording.wav"; - - [JsonProperty("content_type")] - public string ContentType { get; set; } = "audio/wav"; - - [JsonProperty("language")] - public string Language { get; set; } = "ko"; -}Assets/Infrastructure/Network/Http/HttpApiClient.cs (1)
518-520: Debug logging in production codeExtensive debug logging is left in production code paths, which can impact performance and expose sensitive information.
Use conditional compilation or a logging level system:
+#if UNITY_EDITOR || DEVELOPMENT_BUILD // 디버깅: Content-Type 헤더 확인 string contentType = request.GetRequestHeader("Content-Type"); Debug.Log($"[HttpApiClient] 요청 헤더 설정 완료 - Content-Type: {contentType}"); +#endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Assets/UI/Prefabs/AudioInputView.prefabis excluded by!**/*.prefab
📒 Files selected for processing (11)
Assets/Core/Audio/AudioRecorder.cs(6 hunks)Assets/Core/Chat.meta(1 hunks)Assets/Domain/Chat/View/VoiceInputView.cs(11 hunks)Assets/Infrastructure/Audio.meta(1 hunks)Assets/Infrastructure/Audio/WavEncoder.cs(1 hunks)Assets/Infrastructure/Audio/WavEncoder.cs.meta(1 hunks)Assets/Infrastructure/Network/Configs/NetworkConfig.cs(3 hunks)Assets/Infrastructure/Network/Http/HttpApiClient.cs(8 hunks)Assets/Infrastructure/Network/Services/ISTTService.cs(0 hunks)Assets/Infrastructure/Network/Services/ISTTService.cs.meta(0 hunks)Assets/Infrastructure/Network/Services/STTService.cs(3 hunks)
💤 Files with no reviewable changes (2)
- Assets/Infrastructure/Network/Services/ISTTService.cs.meta
- Assets/Infrastructure/Network/Services/ISTTService.cs
🧰 Additional context used
🧬 Code Graph Analysis (5)
Assets/Infrastructure/Audio/WavEncoder.cs (1)
Assets/Core/Audio/AudioRecorder.cs (2)
AudioClip(123-159)AudioClip(268-312)
Assets/Infrastructure/Network/Services/STTService.cs (3)
Assets/Domain/Chat/View/VoiceInputView.cs (1)
System(310-328)Assets/Infrastructure/Network/Http/HttpApiClient.cs (18)
HttpApiClient(17-565)UniTask(56-60)UniTask(62-67)UniTask(69-74)UniTask(76-80)UniTask(82-86)UniTask(88-92)UniTask(94-116)UniTask(185-218)UniTask(220-253)UniTask(255-293)UniTask(295-344)UniTask(346-401)UniTask(408-420)UniTask(422-431)UniTask(433-445)UniTask(447-456)CancellationToken(403-406)Assets/Infrastructure/Audio/WavEncoder.cs (2)
WavEncoder(9-92)WrapPcm16ToWav(44-91)
Assets/Domain/Chat/View/VoiceInputView.cs (2)
Assets/Core/Audio/AudioRecorder.cs (4)
AudioRecorder(13-329)AudioClip(123-159)AudioClip(268-312)AudioClipToWavBytes(164-177)Assets/Infrastructure/Network/Services/STTService.cs (3)
STTService(18-129)STTService(25-32)GenerateTestAudioData(100-128)
Assets/Core/Audio/AudioRecorder.cs (4)
Assets/Core/Utils/Singleton.cs (1)
Singleton(3-36)Assets/Domain/Chat/Model/ChatMessage.cs (1)
AudioClip(40-40)Assets/Domain/Chat/View/VoiceInputView.cs (2)
OnRecordingProgress(379-382)Update(46-53)Assets/Infrastructure/Audio/WavEncoder.cs (2)
WavEncoder(9-92)FromAudioClip(14-39)
Assets/Infrastructure/Network/Http/HttpApiClient.cs (4)
Assets/Infrastructure/Network/Services/STTService.cs (1)
UniTask(41-95)Assets/Infrastructure/Network/Services/CharacterApiService.cs (2)
UniTask(30-39)UniTask(47-56)Assets/Infrastructure/Network/Services/ChatApiService.cs (2)
UniTask(32-41)UniTask(52-61)Assets/Infrastructure/Network/Configs/NetworkConfig.cs (1)
NetworkConfig(221-254)
🔇 Additional comments (7)
Assets/Core/Chat.meta (1)
2-2: No remaining references to the old folder GUIDRan a repository-wide search for the previous GUID:
rg -n "84b7758c3c4878646b31785d9a1c3f22"No occurrences were found, confirming there are no stale references to the old folder GUID.
Assets/Infrastructure/Audio.meta (1)
1-9: LGTM: New folder asset metadata is correct.Defaults and structure are standard and consistent. No action needed.
Assets/Infrastructure/Audio/WavEncoder.cs.meta (1)
1-2: LGTM: Meta for new script added.No issues.
Assets/Infrastructure/Audio/WavEncoder.cs (1)
1-39: Overall: Useful, self-contained WAV encoder.The encoder is straightforward, dependency-light, and suits the audio pipeline changes. Good addition.
Assets/Domain/Chat/View/VoiceInputView.cs (3)
141-141: Nice UX touch resetting the progress bar at record start.This keeps the UI consistent at the beginning of a new recording session.
178-179: Good switch to WAV export before STT.Aligns with the new WavEncoder utility and typical STT service expectations.
48-53: AudioRecorder Auto-Stops at Configured Max DurationVerified in Assets/Core/Audio/AudioRecorder.cs (Update method, lines 53–63):
- Invokes OnRecordingProgress each frame.
- Checks RecordingDuration against _maxRecordingLength and calls StopRecording() when reached.
- Ensures OnRecordingStopped/OnRecordingCompleted are raised.
No further changes needed in VoiceInputView; the guard can remain removed.
| if (_isRecording) | ||
| { | ||
| StopRecording(); | ||
| // 녹음 진행률 이벤트 발생 | ||
| OnRecordingProgress?.Invoke(RecordingProgress); | ||
|
|
||
| // 최대 녹음 시간 체크 | ||
| if (RecordingDuration >= _maxRecordingLength) | ||
| { | ||
| StopRecording(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update() method performs unnecessary work every frame
The Update() method fires the progress event and checks duration every frame when recording, which is inefficient for UI updates that don't need 60+ FPS refresh rates.
Throttle the update frequency:
+private float _lastProgressUpdate = 0f;
+private const float PROGRESS_UPDATE_INTERVAL = 0.1f; // Update 10 times per second
private void Update()
{
if (_isRecording)
{
- // 녹음 진행률 이벤트 발생
- OnRecordingProgress?.Invoke(RecordingProgress);
+ // Throttle progress updates
+ if (Time.time - _lastProgressUpdate >= PROGRESS_UPDATE_INTERVAL)
+ {
+ OnRecordingProgress?.Invoke(RecordingProgress);
+ _lastProgressUpdate = Time.time;
+ }
// 최대 녹음 시간 체크
if (RecordingDuration >= _maxRecordingLength)
{
StopRecording();
}
}
}📝 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.
| if (_isRecording) | |
| { | |
| StopRecording(); | |
| // 녹음 진행률 이벤트 발생 | |
| OnRecordingProgress?.Invoke(RecordingProgress); | |
| // 최대 녹음 시간 체크 | |
| if (RecordingDuration >= _maxRecordingLength) | |
| { | |
| StopRecording(); | |
| } | |
| } | |
| } | |
| // Add these at class scope | |
| private float _lastProgressUpdate = 0f; | |
| private const float PROGRESS_UPDATE_INTERVAL = 0.1f; // Update 10 times per second | |
| private void Update() | |
| { | |
| if (_isRecording) | |
| { | |
| // Throttle progress updates | |
| if (Time.time - _lastProgressUpdate >= PROGRESS_UPDATE_INTERVAL) | |
| { | |
| OnRecordingProgress?.Invoke(RecordingProgress); | |
| _lastProgressUpdate = Time.time; | |
| } | |
| // 최대 녹음 시간 체크 | |
| if (RecordingDuration >= _maxRecordingLength) | |
| { | |
| StopRecording(); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In Assets/Core/Audio/AudioRecorder.cs around lines 53 to 64, the Update() method
currently invokes OnRecordingProgress and checks RecordingDuration every frame
while _isRecording; throttle these operations by accumulating Time.deltaTime and
only invoking the progress event and duration check at a lower frequency (e.g.,
every 0.1s or configurable interval). Add a private float accumulator and a
configurable float progressUpdateInterval, increment the accumulator with
Time.deltaTime each Update, and when it exceeds the interval reset the
accumulator, invoke OnRecordingProgress with current RecordingProgress, and then
perform the RecordingDuration >= _maxRecordingLength check and StopRecording()
if needed. Ensure the default interval is small enough for UI responsiveness but
avoids per-frame updates.
| public bool IsConnected => true; // 항상 연결 가능하다고 가정 | ||
| public bool IsAvailable => _httpClient != null; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Properties always return fixed values
The IsConnected and IsAvailable properties aren't providing meaningful state information. IsConnected always returns true, and IsAvailable only checks for null without considering actual connectivity or initialization state.
Consider implementing proper state management:
-public bool IsConnected => true; // 항상 연결 가능하다고 가정
-public bool IsAvailable => _httpClient != null;
+public bool IsConnected => _httpClient?.IsConnected ?? false;
+public bool IsAvailable => _httpClient != null && _httpClient.IsInitialized;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Assets/Infrastructure/Network/Services/STTService.cs around lines 22-23, the
IsConnected and IsAvailable properties return fixed/meaningless values; replace
them with real state checks by introducing a private backing field (e.g.,
_isConnected) that you set true/false during proper
initialization/Connect/Disconnect/Dispose flows, and change IsConnected to
return that field; change IsAvailable to return a meaningful check such as
(_httpClient != null && _isConnected) or perform a lightweight health check
(HEAD/GET to the service or a cached successful probe result) in an async
EnsureAvailable method and base IsAvailable on that probe or initialization
flag; ensure updates to the state are thread-safe (lock/volatile) and update all
startup/teardown code paths to set the backing field accordingly.
| string forcedLanguage = "ko"; | ||
| string endpoint = $"stt/transcribe?language={forcedLanguage}"; |
There was a problem hiding this comment.
Language parameter is ignored despite being in the method signature
The method accepts a language parameter but forces it to "ko" internally, which is misleading for callers who might expect their language preference to be honored.
Either remove the parameter or properly use it:
-// 서버 API에 맞게 language 파라미터만 사용
-string forcedLanguage = "ko";
-string endpoint = $"stt/transcribe?language={forcedLanguage}";
+// Use the provided language parameter
+string endpoint = $"stt/transcribe?language={language}";Or if the API only supports Korean, remove the parameter:
-public async UniTask<string> ConvertSpeechToTextAsync(byte[] audioData, string audioFormat = "wav", string language = "ko", CancellationToken cancellationToken = default)
+public async UniTask<string> ConvertSpeechToTextAsync(byte[] audioData, string audioFormat = "wav", CancellationToken cancellationToken = default)📝 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.
| string forcedLanguage = "ko"; | |
| string endpoint = $"stt/transcribe?language={forcedLanguage}"; | |
| // Use the provided language parameter | |
| string endpoint = $"stt/transcribe?language={language}"; |
🤖 Prompt for AI Agents
In Assets/Infrastructure/Network/Services/STTService.cs around lines 67-68, the
method ignores the incoming language parameter and hardcodes forcedLanguage =
"ko"; fix by either using the method parameter (replace forcedLanguage with the
language argument, validate/normalize it if needed, and construct endpoint =
$"stt/transcribe?language={language}") or if the API truly only supports Korean,
remove the language parameter from the method signature and all callers and keep
the endpoint hardcoded; ensure any unit tests or call sites are updated
accordingly.
| public byte[] GenerateTestAudioData() | ||
| { | ||
| int sampleRate = 22050; | ||
| int duration = 1; // 1초 | ||
| int samples = sampleRate * duration; | ||
|
|
||
| // 440Hz 사인파 생성 | ||
| float frequency = 440f; | ||
| float[] audioData = new float[samples]; | ||
|
|
||
| for (int i = 0; i < samples; i++) | ||
| { | ||
| audioData[i] = Mathf.Sin(2f * Mathf.PI * frequency * i / sampleRate) * 0.5f; | ||
| } | ||
|
|
||
| // WAV로 변환 | ||
| byte[] pcm16 = new byte[samples * 2]; | ||
| int pcmIndex = 0; | ||
| for (int i = 0; i < samples; i++) | ||
| { | ||
| float clamped = Mathf.Clamp(audioData[i], -1f, 1f); | ||
| short s = (short)Mathf.RoundToInt(clamped * short.MaxValue); | ||
| pcm16[pcmIndex++] = (byte)(s & 0xFF); | ||
| pcm16[pcmIndex++] = (byte)((s >> 8) & 0xFF); | ||
| } | ||
|
|
||
| // WAV 헤더 추가 | ||
| return ProjectVG.Infrastructure.Audio.WavEncoder.WrapPcm16ToWav(pcm16, 1, sampleRate); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Test method generates audio data that should be in a test class
The GenerateTestAudioData method is test-specific code that shouldn't be in production service classes. This violates separation of concerns and could accidentally be used in production.
Move this to a test utility class or use conditional compilation:
+#if UNITY_EDITOR || DEVELOPMENT_BUILD
/// <summary>
/// 테스트용 더미 음성 데이터 생성 (1초, 22050Hz, 사인파)
/// </summary>
public byte[] GenerateTestAudioData()
{
// ... existing implementation ...
}
+#endif📝 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 byte[] GenerateTestAudioData() | |
| { | |
| int sampleRate = 22050; | |
| int duration = 1; // 1초 | |
| int samples = sampleRate * duration; | |
| // 440Hz 사인파 생성 | |
| float frequency = 440f; | |
| float[] audioData = new float[samples]; | |
| for (int i = 0; i < samples; i++) | |
| { | |
| audioData[i] = Mathf.Sin(2f * Mathf.PI * frequency * i / sampleRate) * 0.5f; | |
| } | |
| // WAV로 변환 | |
| byte[] pcm16 = new byte[samples * 2]; | |
| int pcmIndex = 0; | |
| for (int i = 0; i < samples; i++) | |
| { | |
| float clamped = Mathf.Clamp(audioData[i], -1f, 1f); | |
| short s = (short)Mathf.RoundToInt(clamped * short.MaxValue); | |
| pcm16[pcmIndex++] = (byte)(s & 0xFF); | |
| pcm16[pcmIndex++] = (byte)((s >> 8) & 0xFF); | |
| } | |
| // WAV 헤더 추가 | |
| return ProjectVG.Infrastructure.Audio.WavEncoder.WrapPcm16ToWav(pcm16, 1, sampleRate); | |
| } | |
| #if UNITY_EDITOR || DEVELOPMENT_BUILD | |
| public byte[] GenerateTestAudioData() | |
| { | |
| int sampleRate = 22050; | |
| int duration = 1; // 1초 | |
| int samples = sampleRate * duration; | |
| // 440Hz 사인파 생성 | |
| float frequency = 440f; | |
| float[] audioData = new float[samples]; | |
| for (int i = 0; i < samples; i++) | |
| { | |
| audioData[i] = Mathf.Sin(2f * Mathf.PI * frequency * i / sampleRate) * 0.5f; | |
| } | |
| // WAV로 변환 | |
| byte[] pcm16 = new byte[samples * 2]; | |
| int pcmIndex = 0; | |
| for (int i = 0; i < samples; i++) | |
| { | |
| float clamped = Mathf.Clamp(audioData[i], -1f, 1f); | |
| short s = (short)Mathf.RoundToInt(clamped * short.MaxValue); | |
| pcm16[pcmIndex++] = (byte)(s & 0xFF); | |
| pcm16[pcmIndex++] = (byte)((s >> 8) & 0xFF); | |
| } | |
| // WAV 헤더 추가 | |
| return ProjectVG.Infrastructure.Audio.WavEncoder.WrapPcm16ToWav(pcm16, 1, sampleRate); | |
| } | |
| #endif |
🤖 Prompt for AI Agents
In Assets/Infrastructure/Network/Services/STTService.cs around lines 100-128,
the GenerateTestAudioData method is test-only logic that must be removed from
the production service; extract this method into a dedicated test utility (e.g.,
Tests/Utils/TestAudioGenerator.cs or a test-only namespace) or guard it with
conditional compilation (e.g., #if UNITY_EDITOR || DEBUG ... #endif) so it is
not included in production builds, update any call sites to reference the new
test utility, adjust namespaces and access modifiers accordingly, and remove the
method from STTService to restore separation of concerns.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.coderabbit.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.coderabbit.yaml
[error] 5-5: trailing spaces
(trailing-spaces)
[error] 8-8: trailing spaces
(trailing-spaces)
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.coderabbit.yaml (1)
2-14: 구성 스키마와 일치합니다
- language 값(ko-KR), reviews/chat 구조, reviews.auto_review.enabled/drafts 필드 모두 스키마와 정합합니다.
- 불필요한 옵션 추가 없이 목적에 맞게 최소 구성되어 있습니다.
| language: "ko-KR" | ||
| early_access: false | ||
| reviews: | ||
| profile: "chill" |
There was a problem hiding this comment.
YAMLlint 오류: 후행 공백 제거 필요
Line 5 끝에 공백이 있어 linter가 에러를 보고합니다. 아래처럼 공백을 제거하세요.
- profile: "chill"
+ profile: "chill"📝 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.
| profile: "chill" | |
| profile: "chill" |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .coderabbit.yaml around line 5, the value line 'profile: "chill" ' contains a
trailing space that triggers YAML lint errors; remove the trailing whitespace at
the end of line 5 so it becomes 'profile: "chill"' and save the file to clear
the linter error.
| profile: "chill" | ||
| request_changes_workflow: false | ||
| high_level_summary: true | ||
| poem: true |
There was a problem hiding this comment.
YAMLlint 오류: 후행 공백 제거 필요
Line 8 끝에 공백이 있어 linter가 에러를 보고합니다. 아래처럼 공백을 제거하세요.
- poem: true
+ poem: true📝 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.
| poem: true | |
| poem: true |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 8-8: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
.coderabbit.yaml around line 8: there's a trailing space after "poem: true"
causing YAML linter error; remove the trailing whitespace at the end of line 8
so the line reads without any spaces after "true".
| enabled: true | ||
| drafts: false | ||
| chat: | ||
| auto_reply: true No newline at end of file |
There was a problem hiding this comment.
YAMLlint 오류: 파일 끝 개행 추가 필요
파일 끝에 개행(newline)이 없어 linter가 에러를 보고합니다. 개행을 하나 추가해 주세요.
- auto_reply: true
+ auto_reply: true
+📝 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.
| auto_reply: true | |
| auto_reply: true | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
.coderabbit.yaml around line 15: the file is missing a trailing newline at EOF
which triggers YAMLlint; add a single newline character at the end of the file
(ensure the file ends with a blank line) and save to satisfy the linter.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
Assets/Core/Audio/AudioRecorder.cs (2)
237-241: 녹음 중 마이크 변경 방지 구현됨이전 리뷰에서 지적된 녹음 중 마이크 변경 문제가 올바르게 해결되었습니다.
53-64: Update() 메서드에서 매 프레임마다 불필요한 작업 수행매 프레임마다 진행률 이벤트를 발생시키고 지속 시간을 체크하는 것은 비효율적입니다. UI 업데이트는 60+ FPS의 새로고침 빈도가 필요하지 않습니다.
업데이트 빈도를 제한하는 방식으로 개선하세요:
+private float _lastProgressUpdate = 0f; +private const float PROGRESS_UPDATE_INTERVAL = 0.1f; // 초당 10회 업데이트 private void Update() { if (_isRecording) { - // 녹음 진행률 이벤트 발생 - OnRecordingProgress?.Invoke(RecordingProgress); + // 진행률 업데이트 제한 + if (Time.time - _lastProgressUpdate >= PROGRESS_UPDATE_INTERVAL) + { + OnRecordingProgress?.Invoke(RecordingProgress); + _lastProgressUpdate = Time.time; + } // 최대 녹음 시간 체크 if (RecordingDuration >= _maxRecordingLength) { StopRecording(); } } }Assets/Infrastructure/Network/Http/HttpApiClient.cs (2)
297-353: SendFormDataRequestAsync가 여전히 중복 구현을 포함하고 있습니다이 메서드는 이전 리뷰에서 지적된 대로
fileNames매개변수 처리만 다르고 나머지 로직이 동일합니다.기본값 처리로 통합하는 현재 접근 방식(Line 299)이 올바른 방향이지만, 다른 오버로드를 제거해야 합니다. Line 88-92의 오버로드를 다음과 같이 수정하세요:
public async UniTask<T> PostFormDataAsync<T>(string endpoint, Dictionary<string, object> formData, Dictionary<string, string> headers = null, CancellationToken cancellationToken = default) { var url = IsFullUrl(endpoint) ? endpoint : GetFullUrl(endpoint); - return await SendFormDataRequestAsync<T>(url, formData, null, headers, cancellationToken); + return await PostFormDataAsync<T>(endpoint, formData, null, headers, cancellationToken); }
56-80: 중복된 요청 처리 로직을 통합하세요
SendJsonRequestAsync와 기존SendRequestAsync메서드가 재시도 로직, 에러 처리, 요청 처리 코드를 중복으로 구현하고 있습니다. 이는 유지보수 부담을 증가시킵니다.단일 요청 핸들러로 통합하여 코드 중복을 제거하세요:
-private async UniTask<T> SendJsonRequestAsync<T>(string url, string method, string jsonData, Dictionary<string, string> headers, CancellationToken cancellationToken) -{ - // ... 중복 구현 -} - -private async UniTask<T> SendRequestAsync<T>(string url, string method, string jsonData, Dictionary<string, string> headers, CancellationToken cancellationToken) -{ - // ... 중복 구현 -} +private async UniTask<T> SendRequestAsync<T>(string url, string method, string jsonData, Dictionary<string, string> headers, bool useJsonContentType, CancellationToken cancellationToken) +{ + var combinedCancellationToken = CreateCombinedCancellationToken(cancellationToken); + + for (int attempt = 0; attempt <= NetworkConfig.MaxRetryCount; attempt++) + { + try + { + using var request = useJsonContentType + ? CreateJsonRequest(url, method, jsonData, headers) + : CreateRequest(url, method, jsonData, headers); + + var operation = request.SendWebRequest(); + await operation.WithCancellation(combinedCancellationToken); + + if (request.result == UnityWebRequest.Result.Success) + { + return ParseResponse<T>(request); + } + else + { + await HandleRequestFailure(request, attempt, combinedCancellationToken); + } + } + catch (OperationCanceledException) + { + throw; + } + catch (Exception ex) when (ex is not ApiException) + { + await HandleRequestException(ex, attempt, combinedCancellationToken); + } + } + + throw new ApiException($"{NetworkConfig.MaxRetryCount + 1}번 시도 후 요청 실패", 0, "최대 재시도 횟수 초과"); +}그리고 호출 부분을 수정하세요:
public async UniTask<T> GetAsync<T>(string endpoint, Dictionary<string, string> headers = null, CancellationToken cancellationToken = default) { var url = IsFullUrl(endpoint) ? endpoint : GetFullUrl(endpoint); - return await SendJsonRequestAsync<T>(url, UnityWebRequest.kHttpVerbGET, null, headers, cancellationToken); + return await SendRequestAsync<T>(url, UnityWebRequest.kHttpVerbGET, null, headers, true, cancellationToken); }Also applies to: 185-218
🧹 Nitpick comments (5)
Assets/Core/Audio/AudioRecorder.cs (3)
103-103: null 안전성 개선 필요
_currentDevice가 null일 수 있는데Microphone.Start에 빈 문자열을 전달하는 것은 명확하지 않습니다. Unity의 Microphone API는 null이나 빈 문자열을 기본 디바이스로 처리하지만, 이를 명시적으로 처리하는 것이 좋습니다.-_recordingClip = Microphone.Start(_currentDevice ?? string.Empty, false, _maxRecordingLength, _sampleRate); +// null 또는 빈 문자열은 기본 마이크 사용을 의미 +_recordingClip = Microphone.Start(_currentDevice, false, _maxRecordingLength, _sampleRate);
137-137: 일관성 있는 null 처리 패턴 사용Line 103에서와 동일하게
_currentDevice에 대한 null 병합 연산자 사용이 불필요합니다.-Microphone.End(_currentDevice ?? string.Empty); +Microphone.End(_currentDevice);
319-322: DestroyImmediate 사용 시 주의사항
DestroyImmediate는 에디터와 런타임에서 모두 작동하지만, 런타임에서는Destroy를 사용하는 것이 일반적입니다.런타임과 에디터 컨텍스트를 구분하여 처리:
-if (_recordingClip != null) -{ - DestroyImmediate(_recordingClip); -} +if (_recordingClip != null) +{ + #if UNITY_EDITOR + DestroyImmediate(_recordingClip); + #else + Destroy(_recordingClip); + #endif +}Assets/Infrastructure/Network/Http/HttpApiClient.cs (2)
427-443: CreateJsonRequest와 CreateRequest의 중복 코드두 메서드의 유일한 차이점은
Content-Type헤더 설정뿐입니다. 나머지 로직은 동일합니다.단일 메서드로 통합하세요:
-private UnityWebRequest CreateRequest(string url, string method, string jsonData, Dictionary<string, string> headers) -{ - var request = new UnityWebRequest(url, method); - - if (!string.IsNullOrEmpty(jsonData)) - { - var bodyRaw = Encoding.UTF8.GetBytes(jsonData); - request.uploadHandler = new UploadHandlerRaw(bodyRaw); - } - - request.downloadHandler = new DownloadHandlerBuffer(); - SetupRequest(request, headers); - request.timeout = (int)NetworkConfig.HttpTimeout; - - return request; -} - -private UnityWebRequest CreateJsonRequest(string url, string method, string jsonData, Dictionary<string, string> headers) -{ - var request = new UnityWebRequest(url, method); - - if (!string.IsNullOrEmpty(jsonData)) - { - var bodyRaw = Encoding.UTF8.GetBytes(jsonData); - request.uploadHandler = new UploadHandlerRaw(bodyRaw); - request.SetRequestHeader("Content-Type", "application/json"); - } - - request.downloadHandler = new DownloadHandlerBuffer(); - SetupRequest(request, headers); - request.timeout = (int)NetworkConfig.HttpTimeout; - - return request; -} +private UnityWebRequest CreateRequest(string url, string method, string jsonData, Dictionary<string, string> headers, bool setJsonContentType = false) +{ + var request = new UnityWebRequest(url, method); + + if (!string.IsNullOrEmpty(jsonData)) + { + var bodyRaw = Encoding.UTF8.GetBytes(jsonData); + request.uploadHandler = new UploadHandlerRaw(bodyRaw); + if (setJsonContentType) + { + request.SetRequestHeader("Content-Type", "application/json"); + } + } + + request.downloadHandler = new DownloadHandlerBuffer(); + SetupRequest(request, headers); + request.timeout = (int)NetworkConfig.HttpTimeout; + + return request; +}
307-309: 프로덕션 환경에서 과도한 디버그 로깅디버그 로그가 매 요청마다 출력되어 성능에 영향을 줄 수 있습니다.
조건부 로깅을 추가하거나 개발 환경에서만 활성화하세요:
+#if UNITY_EDITOR || DEVELOPMENT_BUILD Debug.Log($"[HttpApiClient] 폼 데이터 전송 시작 - URL: {url}"); Debug.Log($"[HttpApiClient] 실제 전송 URL: {url}"); +#endifAlso applies to: 470-472
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Assets/Core/Audio/AudioRecorder.cs(6 hunks)Assets/Infrastructure/Network/Http/HttpApiClient.cs(9 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
Assets/Core/Audio/AudioRecorder.cs (3)
Assets/Core/Utils/Singleton.cs (1)
Singleton(3-36)Assets/Domain/Chat/View/VoiceInputView.cs (6)
OnRecordingStarted(364-367)OnRecordingStopped(369-372)OnRecordingCompleted(374-377)OnRecordingProgress(379-382)Update(46-53)Start(41-44)Assets/Infrastructure/Audio/WavEncoder.cs (2)
WavEncoder(9-92)FromAudioClip(14-39)
Assets/Infrastructure/Network/Http/HttpApiClient.cs (2)
Assets/Infrastructure/Network/Services/STTService.cs (1)
UniTask(41-95)Assets/Infrastructure/Network/Configs/NetworkConfig.cs (1)
NetworkConfig(221-254)
🔇 Additional comments (4)
Assets/Core/Audio/AudioRecorder.cs (2)
261-273: 마이크 초기화 로직 개선초기화 시 디바이스 목록을 확인하지만
_currentDevice를 null로 초기화하는 것도 고려해볼 수 있습니다. Unity의 Microphone API는 null을 기본 디바이스로 처리합니다.현재 구현이 적절합니다. 첫 번째 사용 가능한 디바이스를 선택하는 것은 좋은 기본 동작입니다.
278-329: 처리된 클립 반환 및 메모리 관리 개선됨이전 리뷰에서 지적된 문제들이 잘 해결되었습니다:
- 처리된 AudioClip을 올바르게 반환
DestroyImmediate를 사용하여 원본 클립 메모리 정리메모리 누수 방지와 올바른 클립 반환이 구현되어 있습니다.
Assets/Infrastructure/Network/Http/HttpApiClient.cs (2)
328-328: UploadTimeout 사용이 적절합니다파일 업로드 시 전용 타임아웃 설정을 사용하는 것은 좋은 접근입니다.
532-543: FileSizeExceededException 구현이 잘 되었습니다파일 크기 초과에 대한 전용 예외 클래스 구현이 적절하며, HTTP 413 상태 코드 사용도 올바릅니다.
| public async UniTask<T> PostFormDataAsync<T>(string endpoint, Dictionary<string, object> formData, Dictionary<string, string> fileNames, Dictionary<string, string> headers = null, CancellationToken cancellationToken = default) | ||
| { | ||
| var url = IsFullUrl(endpoint) ? endpoint : GetFullUrl(endpoint); | ||
|
|
||
| // 파일 크기 검사 | ||
| if (NetworkConfig.EnableFileSizeCheck) | ||
| { | ||
| foreach (var kvp in formData) | ||
| { | ||
| if (kvp.Value is byte[] byteData) | ||
| { | ||
| if (byteData.Length > NetworkConfig.MaxFileSize) | ||
| { | ||
| var fileSizeMB = byteData.Length / 1024.0 / 1024.0; | ||
| var maxSizeMB = NetworkConfig.MaxFileSize / 1024.0 / 1024.0; | ||
| throw new FileSizeExceededException(fileSizeMB, maxSizeMB); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return await SendFormDataRequestAsync<T>(url, formData, fileNames, headers, cancellationToken); | ||
| } |
There was a problem hiding this comment.
파일 크기 검증이 PostFormDataAsync 오버로드에만 구현되어 있습니다
파일 크기 검증 로직이 한 오버로드에만 있어서 다른 파일 업로드 메서드들과 일관성이 없습니다. UploadFileAsync와 다른 PostFormDataAsync 오버로드도 동일한 검증이 필요합니다.
파일 크기 검증을 별도 메서드로 추출하고 모든 파일 업로드 메서드에서 호출하세요:
+private void ValidateFileSize(byte[] fileData, string fieldName = "file")
+{
+ if (!NetworkConfig.EnableFileSizeCheck) return;
+
+ if (fileData.Length > NetworkConfig.MaxFileSize)
+ {
+ var fileSizeMB = fileData.Length / 1024.0 / 1024.0;
+ var maxSizeMB = NetworkConfig.MaxFileSize / 1024.0 / 1024.0;
+ throw new FileSizeExceededException(fileSizeMB, maxSizeMB);
+ }
+}
public async UniTask<T> UploadFileAsync<T>(string endpoint, byte[] fileData, string fileName, string fieldName = "file", Dictionary<string, string> headers = null, CancellationToken cancellationToken = default)
{
+ ValidateFileSize(fileData, fieldName);
var url = GetFullUrl(endpoint);
return await SendFileRequestAsync<T>(url, fileData, fileName, fieldName, headers, cancellationToken);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Assets/Infrastructure/Network/Http/HttpApiClient.cs around lines 94-116, the
file-size validation is implemented only inside this PostFormDataAsync overload;
extract that validation into a private helper method (e.g.,
ValidateFileSizes(Dictionary<string, object> formData) or
ValidateFileSizesFromBytes(IEnumerable<byte[]> files)) that checks byte[]
lengths against NetworkConfig.EnableFileSizeCheck and NetworkConfig.MaxFileSize
and throws FileSizeExceededException with calculated MB values, then replace the
inlined loop with a call to that helper here and invoke the same helper from the
other file upload paths (other PostFormDataAsync overloads and UploadFileAsync)
before sending requests so all upload methods share the same validation logic.
Summary by CodeRabbit
New Features
UI
Tests
Chores