feat: provenance-aware TMK/VAD segmentation and GPU transcription reconciliation - #396
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
rust-core/src/lib.rs (2)
2243-2246: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win실제 partial 파일명을 검사하도록 테스트를 수정하세요.
stage_relative는.codec-carver-{pid}-{nonce}.{extension}.partial형식의 파일을 생성합니다. 현재 테스트는linked.wav.partial만 확인합니다. 실제 partial 파일이 남아도 assertion이 통과할 수 있습니다.read_dir로 staging 디렉터리의 실제.partial항목을 검사하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust-core/src/lib.rs` around lines 2243 - 2246, Update the cleanup assertion around stage_relative to inspect the staging directory with read_dir and verify that no actual entry matching the generated .codec-carver-{pid}-{nonce}.{extension}.partial pattern remains, rather than checking only linked.wav.partial. Preserve the existing staging-directory removal behavior.
851-865: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
fileproviderctl호출에 시간 제한과 성공 상태 검사를 추가하세요.
Command::output()은 자식 프로세스가 종료될 때까지 동기적으로 기다립니다. 현재 호출에는 시간 제한, kill 처리,output.status.success()검사가 없습니다. File Provider가 응답하지 않으면 dataless staging이 무기한 대기할 수 있습니다. 비정상 종료 후에도 파싱 가능한 출력이 반환되면 잘못된 상태를 사용합니다. 시간 제한 또는 non-zero exit에서는 fail closed 하도록 수정하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust-core/src/lib.rs` around lines 851 - 865, Update the fileproviderctl command flow around Command::output to enforce a bounded execution timeout, terminate the child process when the timeout expires, and return None on timeout. Also require output.status.success() before parsing stdout and stderr, returning None for non-zero or abnormal exits so the staging state fails closed.
🧹 Nitpick comments (1)
rust-core/src/lib.rs (1)
2833-2843: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftFile Provider 상태 조회를 주입 가능한 테스트로 분리하세요.
이 테스트는 일반 로컬 파일을 사용합니다.
provider_reports_downloaded가 실패하거나None을 반환해도fs::metadatafallback이 직접 읽기를 허용할 수 있습니다. 따라서DirectReadStaleDatalessFlagassertion이fileproviderctl파싱과 stale dataless 경로를 검증하지 못합니다. 상태 조회 함수를 분리하거나 주입하고, downloaded/latest/not-downloading 및 실패 응답을 fixture로 테스트하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust-core/src/lib.rs` around lines 2833 - 2843, Refactor copy_and_hash_staged_source to use an injectable File Provider status query instead of relying on the real provider_reports_downloaded path during tests. Add fixtures covering downloaded, latest, not-downloading, and failed status responses, and assert the corresponding StageReadMode outcomes. Ensure the tests cannot pass through fs::metadata fallback when validating the stale dataless path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/benchmark_segmentation.py`:
- Around line 133-136: Validate args.duration_seconds as a finite positive value
before invoking _measure_planner in the benchmark flow. Reject zero, negative,
NaN, and infinite values through the existing intended termination path so
_measure_planner never receives an invalid duration and the user-facing exit
message remains effective.
---
Outside diff comments:
In `@rust-core/src/lib.rs`:
- Around line 2243-2246: Update the cleanup assertion around stage_relative to
inspect the staging directory with read_dir and verify that no actual entry
matching the generated .codec-carver-{pid}-{nonce}.{extension}.partial pattern
remains, rather than checking only linked.wav.partial. Preserve the existing
staging-directory removal behavior.
- Around line 851-865: Update the fileproviderctl command flow around
Command::output to enforce a bounded execution timeout, terminate the child
process when the timeout expires, and return None on timeout. Also require
output.status.success() before parsing stdout and stderr, returning None for
non-zero or abnormal exits so the staging state fails closed.
---
Nitpick comments:
In `@rust-core/src/lib.rs`:
- Around line 2833-2843: Refactor copy_and_hash_staged_source to use an
injectable File Provider status query instead of relying on the real
provider_reports_downloaded path during tests. Add fixtures covering downloaded,
latest, not-downloading, and failed status responses, and assert the
corresponding StageReadMode outcomes. Ensure the tests cannot pass through
fs::metadata fallback when validating the stale dataless path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d39e22e4-ef1e-4429-9c7a-b90c925aee84
📒 Files selected for processing (10)
audio_library.pydocs/architecture/gpu-transcription-rust-backend.mddocs/architecture/segmentation-reconciliation.mdrust-core/src/lib.rsrust-core/src/main.rsrust-toolchain.tomlscripts/benchmark_segmentation.pytests/test_audio_library.pytests/test_macos_gpu_bootstrap.pytests/test_saas_web.py
🚧 Files skipped from review as they are similar to previous changes (5)
- rust-core/src/main.rs
- rust-toolchain.toml
- tests/test_macos_gpu_bootstrap.py
- docs/architecture/gpu-transcription-rust-backend.md
- docs/architecture/segmentation-reconciliation.md
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust-core/src/lib.rs (1)
21-21: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClippy가 보고한 미사용 import를 제거하세요.
Clippy가 Line 21의 import 선언을 사용되지 않은 import로 보고했습니다. 경고에 표시된 심볼만 제거하세요. 조건부 컴파일에서만 필요한 심볼이면 해당
#[cfg]범위에 두세요.정적 분석 경고를 근거로 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust-core/src/lib.rs` at line 21, rust-core/src/lib.rs의 std::time import에서 Clippy가 미사용으로 지목한 심볼만 제거하세요. 해당 심볼이 조건부 컴파일에서만 사용된다면 관련 #[cfg] 범위로 이동하고, 나머지 Duration, Instant, SystemTime, UNIX_EPOCH import는 유지하세요.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/gpu-transcription-rust-backend.md`:
- Around line 70-74: Update provider_reports_downloaded() so spawn errors,
timeouts, abnormal exits, and non-zero statuses remain explicitly unknown and
always select the Foundation-coordinated path instead of being re-evaluated via
fs::metadata() or permitting direct reads. Preserve successful evaluate parsing,
and add regression coverage for each failure type.
---
Outside diff comments:
In `@rust-core/src/lib.rs`:
- Line 21: rust-core/src/lib.rs의 std::time import에서 Clippy가 미사용으로 지목한 심볼만 제거하세요.
해당 심볼이 조건부 컴파일에서만 사용된다면 관련 #[cfg] 범위로 이동하고, 나머지 Duration, Instant, SystemTime,
UNIX_EPOCH import는 유지하세요.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e4e4449e-6d52-4ac2-8150-c976ff2c5a9a
📒 Files selected for processing (5)
audio_library.pydocs/architecture/gpu-transcription-rust-backend.mdrust-core/src/lib.rsscripts/benchmark_segmentation.pytests/test_audio_library.py
|
@opencode-agent @cwl-noema-review Formal review-only request for exact current head |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head3c93fa9fef1d96b33da90df65670dd3adbd9e79e. -
Head SHA:
3c93fa9fef1d96b33da90df65670dd3adbd9e79e -
Workflow run: 31498284478
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (13 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (13 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Workflow: ci.yml"]
S2 --> I2["GitHub Actions review job"]
I2 --> R2["Review risk: Workflow: ci.yml"]
R2 --> V2["actionlint plus required checks"]
Evidence --> S3["Docs (4 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (4 files)"]
R3 --> V3["docs review"]
Evidence --> S4["Test (7 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (7 files)"]
R4 --> V4["targeted test run"]
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
audio_library.py (1)
7030-7035: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift캐시 적중 경로에서 prefetch Future를 추적하고 정리하세요.
Future가 이미 시작되면cancel()은False를 반환합니다. 현재 코드는 해당 Future를prefetch_futures에서 제거하므로await_pending_prefetches()와 eviction 판단에서 제외합니다.backend.stage가 완료되어도verify_staged_artifact()를 호출하지 않으므로 staged artifact가 남습니다. 실행 중인 작업은 staging 디렉터리의weakref.finalize정리와 경합할 수 있습니다.캐시 적중 여부를 prefetch 제출 전에 확인하여 후보에서 제외하세요. 이미 제출한 경우 Future를 drain 목록에 유지하고, 완료 후 반환된
staged_path를 안전하게 삭제하세요. 그 다음 livematerialized상태를 갱신하고 eviction을 수행하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@audio_library.py` around lines 7030 - 7035, 캐시 적중 경로에서 prefetch Future를 무조건 제거·취소하지 않도록 수정하세요. prefetch 제출 전에 cache hit를 확인해 후보에서 제외하고, 이미 제출된 Future는 await_pending_prefetches()가 drain하도록 추적을 유지하며 완료 후 반환된 staged_path를 안전하게 삭제하세요. 이후 live materialized 상태를 갱신한 뒤 eviction을 수행하도록 해당 cache-hit 처리 흐름을 조정하세요.
🧹 Nitpick comments (1)
audio_library.py (1)
7446-7453: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
is_icloud_datalessprobe 실패를 구분하세요.
is_icloud_dataless()는FileNotFoundError만 처리합니다. 현재 블록은PermissionError등OSError와 예상 밖 예외를 모두materialized=False로 저장합니다. 이 경우 probe 실패가 실제 dataless 상태로 처리되고 eviction 판단에서 제외됩니다. 예상 가능한OSError만 처리하거나, fail-closed 정책과 별도 진단 정보를 추가하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@audio_library.py` around lines 7446 - 7453, Update the exception handling around the is_icloud_dataless probe so expected OSError failures are handled deliberately, while unexpected exceptions are not silently converted into materialized=False. Preserve the fail-closed behavior only when explicitly intended, and record separate diagnostic information for probe failures so eviction logic can distinguish them from confirmed dataless state.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/gpu-transcription-rust-backend.md`:
- Around line 49-55: Update stream_transcribe() so a was_dataless cache hit
requires sha256_verified to be computed from the current bytes opened through a
no-follow descriptor, not merely a stored SHA with sha256_source == "content".
If current bytes cannot be inspected, continue through Rust staging and
byte/hash verification instead of reusing the transcript sidecar. Add a
regression test proving remote byte changes invalidate the previous SHA and
cache.
---
Outside diff comments:
In `@audio_library.py`:
- Around line 7030-7035: 캐시 적중 경로에서 prefetch Future를 무조건 제거·취소하지 않도록 수정하세요.
prefetch 제출 전에 cache hit를 확인해 후보에서 제외하고, 이미 제출된 Future는
await_pending_prefetches()가 drain하도록 추적을 유지하며 완료 후 반환된 staged_path를 안전하게 삭제하세요.
이후 live materialized 상태를 갱신한 뒤 eviction을 수행하도록 해당 cache-hit 처리 흐름을 조정하세요.
---
Nitpick comments:
In `@audio_library.py`:
- Around line 7446-7453: Update the exception handling around the
is_icloud_dataless probe so expected OSError failures are handled deliberately,
while unexpected exceptions are not silently converted into materialized=False.
Preserve the fail-closed behavior only when explicitly intended, and record
separate diagnostic information for probe failures so eviction logic can
distinguish them from confirmed dataless state.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d5ea1e8-2227-42e2-af83-de729eef2e4c
📒 Files selected for processing (5)
README.mdaudio_library.pydocs/architecture/gpu-transcription-rust-backend.mdrust-core/src/lib.rstests/test_audio_library.py
Dismissed stale OpenCode request: the cited review commit is not the current PR head; current-head Checks and unresolved threads will be revalidated before approval.
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headda969147b8039c3f8dd78658fb2dab1543c06bbf. -
Head SHA:
da969147b8039c3f8dd78658fb2dab1543c06bbf -
Workflow run: 31508885233
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (13 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (13 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Workflow: ci.yml"]
S2 --> I2["GitHub Actions review job"]
I2 --> R2["Review risk: Workflow: ci.yml"]
R2 --> V2["actionlint plus required checks"]
Evidence --> S3["Docs (4 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (4 files)"]
R3 --> V3["docs review"]
Evidence --> S4["Test (7 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (7 files)"]
R4 --> V4["targeted test run"]
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
audio_library.py (2)
7412-7422: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win성공한 provider 상태 조회 뒤에도 이전 probe 오류를 삭제하세요.
현재
record.pop("materialization_probe_error", None)는 전체 전사 성공 경로에서만 실행됩니다. 이전 실행이 probe 오류를 기록한 뒤, 다음 실행이 provider 상태 조회에는 성공하지만 GPU 전사에서 실패하면 오래된materialization_probe_error가 남습니다. 운영자가 현재 전사 실패를 provider 오류로 오인할 수 있습니다.
is_icloud_dataless(audio_path)조회가 성공한 직후에도 해당 필드를 삭제하세요. 새 probe 예외가 발생한 경우에만 새 오류를 기록해야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@audio_library.py` around lines 7412 - 7422, Update the failure-handling path around is_icloud_dataless(audio_path) so that, after the provider-state lookup succeeds, it removes the stale materialization_probe_error field before setting record["materialized"]. Preserve recording a new probe error only when the lookup itself raises an exception, while keeping the existing transcription failure handling unchanged.
7423-7436: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win현재 바이트 검증 전에 실패하면
sha256_verified를 무효화하세요.
backend.stage()또는backend.inspect()가 검증 완료 전에 실패하면 이 예외 처리 블록은materialized만 갱신합니다. 기존 inventory의sha256_verified=True와sha256_source="content"가 그대로 저장될 수 있습니다. 현재 File Provider 바이트를 확인하지 않았는데도 SHA-256 증거가 검증된 상태로 남습니다.검증 완료 여부를 별도 상태로 추적하세요. 검증 전에 실패하면
record_sha_is_verified()가False를 반환하도록sha256_verified를 해제하세요. 검증된 staged bytes 이후 GPU 전사만 실패한 경우에는 기존 검증 상태를 유지해야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@audio_library.py` around lines 7423 - 7436, Track whether File Provider bytes were successfully validated separately from later GPU transcription failures. In the exception paths around backend.stage() and backend.inspect(), clear the existing SHA-256 verification state through record_sha_is_verified() when failure occurs before validation, including sha256_source="content" as appropriate. Preserve the existing SHA-256 state when staged bytes were already validated and only GPU transcription fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@audio_library.py`:
- Around line 7412-7422: Update the failure-handling path around
is_icloud_dataless(audio_path) so that, after the provider-state lookup
succeeds, it removes the stale materialization_probe_error field before setting
record["materialized"]. Preserve recording a new probe error only when the
lookup itself raises an exception, while keeping the existing transcription
failure handling unchanged.
- Around line 7423-7436: Track whether File Provider bytes were successfully
validated separately from later GPU transcription failures. In the exception
paths around backend.stage() and backend.inspect(), clear the existing SHA-256
verification state through record_sha_is_verified() when failure occurs before
validation, including sha256_source="content" as appropriate. Preserve the
existing SHA-256 state when staged bytes were already validated and only GPU
transcription fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a60a0464-3e15-4a7a-a372-3bccc73bb37c
📒 Files selected for processing (5)
README.mdaudio_library.pydocs/architecture/gpu-transcription-rust-backend.mdtests/test_audio_library.pytests/test_saas_web.py
💤 Files with no reviewable changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_saas_web.py
- docs/architecture/gpu-transcription-rust-backend.md
|
@opencode-agent Please re-review exact current head |
1 participant
Summary
tmk_pending_materializationinstead of fabricating markers.Validation
-D warningsand format check passed.Supersedes the closed/conflicting #250 snapshot without changing that PR's state.
Summary by CodeRabbit
새로운 기능
문서
품질 개선