feat(audio): add independent track mixing and transcript timing - #167
Open
DonIsmaelito wants to merge 5 commits into
Open
DonIsmaelito wants to merge 5 commits into
DonIsmaelito wants to merge 5 commits into
Conversation
DonIsmaelito
marked this pull request as ready for review
September 17, 2026 20:12
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
13 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pyproject.toml">
<violation number="1" location="pyproject.toml:28">
P2: With pytest older than 7, this `pythonpath` option is ignored and the documented test commands fail on bare helper imports such as `from mix_audio import ...`. Declare a pytest 7+ test requirement or provide a version-independent helper import setup.</violation>
</file>
<file name="references/sources.md">
<violation number="1" location="references/sources.md:23">
P2: The catalog does not guarantee original source PTS because `source_scan.py` records FFmpeg best-effort timestamps. Change this wording to “best-effort presentation timestamps” or update the helper to emit original `pts_time` values before consumers rely on them for exact alignment.</violation>
</file>
<file name="helpers/map_transcript.py">
<violation number="1" location="helpers/map_transcript.py:26">
P3: When a word starts slightly before zero, `seconds_to_sample` rounds it to sample 0 and this check accepts it as valid. Validate the raw timestamps before converting them to samples.</violation>
</file>
<file name="helpers/find_shot.py">
<violation number="1" location="helpers/find_shot.py:50">
P2: When `--every nan` is supplied, this check accepts it and the sampler silently examines only the first frame. Reject non-finite intervals before sampling.</violation>
<violation number="2" location="helpers/find_shot.py:89">
P1: When `--out` is a hard link to the query or source, this check passes and `save_json` destroys the input file. Use `samefile()` for existing output aliases before writing.</violation>
</file>
<file name="helpers/project_state.py">
<violation number="1" location="helpers/project_state.py:47">
P2: When a persisted artifact row lacks `sha256`, `show` crashes with `KeyError` after `validate` accepts it. Require the generated fingerprint during validation before `view` indexes it.</violation>
<violation number="2" location="helpers/project_state.py:57">
P1: When an artifact path is the context itself or a filesystem alias, `record` hashes it and then overwrites the context via `save_json`. Reject the resolved path and `samefile` aliases before hashing.</violation>
<violation number="3" location="helpers/project_state.py:62">
P3: When a new artifact names a missing or cyclic dependency, this hash comprehension runs before `validate`, so `record` raises `KeyError` and bypasses dependency diagnostics. Insert the candidate row and validate it before computing dependency hashes.</violation>
</file>
<file name="helpers/source_scan.py">
<violation number="1" location="helpers/source_scan.py:80">
P1: When an input contains multiple video streams, FFmpeg can decode a different stream than the `v:0` stream cataloged above. Map `0:v:0` explicitly so selected frame indices and dimensions refer to the cataloged stream.</violation>
<violation number="2" location="helpers/source_scan.py:142">
P1: When `--out` is a hard link to the source, `resolve()` does not identify the alias and `save_json()` truncates the source file. Reject existing aliases with `samefile()` before writing the catalog.</violation>
</file>
<file name="helpers/edit_clock.py">
<violation number="1" location="helpers/edit_clock.py:59">
P2: When the frame budget is at least the number of positive-weight shots but a small quota floors to zero, `allocate_frames` raises instead of producing a valid partition. Reserve one frame per shot before applying largest-remainder allocation so valid short shots are not rejected.</violation>
</file>
<file name="helpers/prepare_source.py">
<violation number="1" location="helpers/prepare_source.py:13">
P2: When `<out>.log` or `<out>.json` already exists, this guard checks only the MKV, so preparation overwrites the prior artifacts. Reject all derivative artifact paths before creating the output.</violation>
<violation number="2" location="helpers/prepare_source.py:25">
P2: When callers pass an explicitly supplied empty crop, `if crop` skips validation and silently ignores it. Check `crop is not None` so every supplied crop is validated.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| """Save an artifact fingerprint and the dependency versions it was built from.""" | ||
| context = Path(context) | ||
| data = load_json(context) if context.exists() else {"version": 1, "artifacts": {}} | ||
| path = resolve(context.parent, entry["path"]) |
There was a problem hiding this comment.
P1: When an artifact path is the context itself or a filesystem alias, record hashes it and then overwrites the context via save_json. Reject the resolved path and samefile aliases before hashing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/project_state.py, line 57:
<comment>When an artifact path is the context itself or a filesystem alias, `record` hashes it and then overwrites the context via `save_json`. Reject the resolved path and `samefile` aliases before hashing.</comment>
<file context>
@@ -0,0 +1,126 @@
+ """Save an artifact fingerprint and the dependency versions it was built from."""
+ context = Path(context)
+ data = load_json(context) if context.exists() else {"version": 1, "artifacts": {}}
+ path = resolve(context.parent, entry["path"])
+ if not path.is_file():
+ raise FileNotFoundError(path)
</file context>
Suggested change
| path = resolve(context.parent, entry["path"]) | |
| path = resolve(context.parent, entry["path"]) | |
| if path == context.resolve() or ( | |
| path.exists() and context.exists() and path.samefile(context) | |
| ): | |
| raise ValueError("context cannot also be an artifact") |
| "2", | ||
| "-i", | ||
| str(path), | ||
| "-an", |
There was a problem hiding this comment.
P1: When an input contains multiple video streams, FFmpeg can decode a different stream than the v:0 stream cataloged above. Map 0:v:0 explicitly so selected frame indices and dimensions refer to the cataloged stream.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/source_scan.py, line 80:
<comment>When an input contains multiple video streams, FFmpeg can decode a different stream than the `v:0` stream cataloged above. Map `0:v:0` explicitly so selected frame indices and dimensions refer to the cataloged stream.</comment>
<file context>
@@ -0,0 +1,151 @@
+ "2",
+ "-i",
+ str(path),
+ "-an",
+ "-sn",
+ "-dn",
</file context>
| parser.add_argument("--out", required=True) | ||
| parser.add_argument("--scenes", action="store_true") | ||
| args = parser.parse_args() | ||
| if Path(args.source).resolve() == Path(args.out).resolve(): |
There was a problem hiding this comment.
P1: When --out is a hard link to the source, resolve() does not identify the alias and save_json() truncates the source file. Reject existing aliases with samefile() before writing the catalog.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/source_scan.py, line 142:
<comment>When `--out` is a hard link to the source, `resolve()` does not identify the alias and `save_json()` truncates the source file. Reject existing aliases with `samefile()` before writing the catalog.</comment>
<file context>
@@ -0,0 +1,151 @@
+ parser.add_argument("--out", required=True)
+ parser.add_argument("--scenes", action="store_true")
+ args = parser.parse_args()
+ if Path(args.source).resolve() == Path(args.out).resolve():
+ parser.error("output cannot replace source")
+ data = catalog(args.source)
</file context>
| p.add_argument("--every", type=float, default=1) | ||
| p.add_argument("--out", required=True) | ||
| a = p.parse_args() | ||
| if Path(a.out).resolve() in (Path(a.query).resolve(), Path(a.source).resolve()): |
There was a problem hiding this comment.
P1: When --out is a hard link to the query or source, this check passes and save_json destroys the input file. Use samefile() for existing output aliases before writing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/find_shot.py, line 89:
<comment>When `--out` is a hard link to the query or source, this check passes and `save_json` destroys the input file. Use `samefile()` for existing output aliases before writing.</comment>
<file context>
@@ -0,0 +1,95 @@
+ p.add_argument("--every", type=float, default=1)
+ p.add_argument("--out", required=True)
+ a = p.parse_args()
+ if Path(a.out).resolve() in (Path(a.query).resolve(), Path(a.source).resolve()):
+ p.error("output would overwrite input")
+ save_json(a.out, search(a.query, a.source, a.every))
</file context>
Suggested change
| if Path(a.out).resolve() in (Path(a.query).resolve(), Path(a.source).resolve()): | |
| output = Path(a.out).resolve() | |
| inputs = [Path(a.query).resolve(), Path(a.source).resolve()] | |
| if any( | |
| output == path | |
| or (output.exists() and path.exists() and output.samefile(path)) | |
| for path in inputs | |
| ): |
| py-modules = [] | ||
|
|
||
| [tool.pytest.ini_options] | ||
| pythonpath = [".", "helpers"] |
There was a problem hiding this comment.
P2: With pytest older than 7, this pythonpath option is ignored and the documented test commands fail on bare helper imports such as from mix_audio import .... Declare a pytest 7+ test requirement or provide a version-independent helper import setup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pyproject.toml, line 28:
<comment>With pytest older than 7, this `pythonpath` option is ignored and the documented test commands fail on bare helper imports such as `from mix_audio import ...`. Declare a pytest 7+ test requirement or provide a version-independent helper import setup.</comment>
<file context>
@@ -10,14 +10,20 @@ dependencies = [
py-modules = []
+
+[tool.pytest.ini_options]
+pythonpath = [".", "helpers"]
+testpaths = ["tests"]
</file context>
Comment on lines
+59
to
+66
| exact = [fraction(w) * total / sum(map(fraction, weights)) for w in weights] | ||
| counts = [math.floor(v) for v in exact] | ||
| for i in sorted( | ||
| range(len(weights)), key=lambda i: (exact[i] - counts[i], -i), reverse=True | ||
| )[: total - sum(counts)]: | ||
| counts[i] += 1 | ||
| if min(counts) < 1: | ||
| raise ValueError("duration is too short for these shot proportions") |
There was a problem hiding this comment.
P2: When the frame budget is at least the number of positive-weight shots but a small quota floors to zero, allocate_frames raises instead of producing a valid partition. Reserve one frame per shot before applying largest-remainder allocation so valid short shots are not rejected.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/edit_clock.py, line 59:
<comment>When the frame budget is at least the number of positive-weight shots but a small quota floors to zero, `allocate_frames` raises instead of producing a valid partition. Reserve one frame per shot before applying largest-remainder allocation so valid short shots are not rejected.</comment>
<file context>
@@ -0,0 +1,87 @@
+ """Largest-remainder allocation preserves total; it does not choose edit points."""
+ if not weights or total < len(weights) or any(fraction(w) <= 0 for w in weights):
+ raise ValueError("positive weights and at least one frame per shot required")
+ exact = [fraction(w) * total / sum(map(fraction, weights)) for w in weights]
+ counts = [math.floor(v) for v in exact]
+ for i in sorted(
</file context>
Suggested change
| exact = [fraction(w) * total / sum(map(fraction, weights)) for w in weights] | |
| counts = [math.floor(v) for v in exact] | |
| for i in sorted( | |
| range(len(weights)), key=lambda i: (exact[i] - counts[i], -i), reverse=True | |
| )[: total - sum(counts)]: | |
| counts[i] += 1 | |
| if min(counts) < 1: | |
| raise ValueError("duration is too short for these shot proportions") | |
| remaining = total - len(weights) | |
| exact = [fraction(w) * remaining / sum(map(fraction, weights)) for w in weights] | |
| floors = [math.floor(v) for v in exact] | |
| counts = [1 + floor for floor in floors] | |
| for i in sorted( | |
| range(len(weights)), key=lambda i: (exact[i] - floors[i], -i), reverse=True | |
| )[: remaining - sum(floors)]: | |
| counts[i] += 1 |
| """Create a new FFV1 working copy and record its explicit transformations.""" | ||
| source = Path(source).resolve() | ||
| out = Path(out).resolve() | ||
| if out.exists() or source == out: |
There was a problem hiding this comment.
P2: When <out>.log or <out>.json already exists, this guard checks only the MKV, so preparation overwrites the prior artifacts. Reject all derivative artifact paths before creating the output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/prepare_source.py, line 13:
<comment>When `<out>.log` or `<out>.json` already exists, this guard checks only the MKV, so preparation overwrites the prior artifacts. Reject all derivative artifact paths before creating the output.</comment>
<file context>
@@ -0,0 +1,111 @@
+ """Create a new FFV1 working copy and record its explicit transformations."""
+ source = Path(source).resolve()
+ out = Path(out).resolve()
+ if out.exists() or source == out:
+ raise FileExistsError("choose a new source derivative path")
+ if out.suffix.lower() != ".mkv":
</file context>
| if tonemap and not hdr: | ||
| raise ValueError("tonemap requires tagged PQ or HLG input") | ||
| filters = [] | ||
| if crop: |
There was a problem hiding this comment.
P2: When callers pass an explicitly supplied empty crop, if crop skips validation and silently ignores it. Check crop is not None so every supplied crop is validated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/prepare_source.py, line 25:
<comment>When callers pass an explicitly supplied empty crop, `if crop` skips validation and silently ignores it. Check `crop is not None` so every supplied crop is validated.</comment>
<file context>
@@ -0,0 +1,111 @@
+ if tonemap and not hdr:
+ raise ValueError("tonemap requires tagged PQ or HLG input")
+ filters = []
+ if crop:
+ if (
+ len(crop) != 4
</file context>
Suggested change
| if crop: | |
| if crop is not None: |
| continue | ||
| a = seconds_to_sample(word["start"]) | ||
| b = seconds_to_sample(word["end"]) | ||
| if a < 0 or b <= a: |
There was a problem hiding this comment.
P3: When a word starts slightly before zero, seconds_to_sample rounds it to sample 0 and this check accepts it as valid. Validate the raw timestamps before converting them to samples.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/map_transcript.py, line 26:
<comment>When a word starts slightly before zero, `seconds_to_sample` rounds it to sample 0 and this check accepts it as valid. Validate the raw timestamps before converting them to samples.</comment>
<file context>
@@ -0,0 +1,135 @@
+ continue
+ a = seconds_to_sample(word["start"])
+ b = seconds_to_sample(word["end"])
+ if a < 0 or b <= a:
+ raise ValueError(
+ "word timestamps must describe a positive nonnegative interval"
</file context>
Suggested change
| if a < 0 or b <= a: | |
| if word["start"] < 0 or word["end"] <= word["start"] or b <= a: |
Comment on lines
+62
to
+66
| entry["dependency_hashes"] = { | ||
| dep: data["artifacts"][dep]["sha256"] for dep in entry.get("depends_on", []) | ||
| } | ||
| data["artifacts"][ident] = entry | ||
| validate(data) |
There was a problem hiding this comment.
P3: When a new artifact names a missing or cyclic dependency, this hash comprehension runs before validate, so record raises KeyError and bypasses dependency diagnostics. Insert the candidate row and validate it before computing dependency hashes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/project_state.py, line 62:
<comment>When a new artifact names a missing or cyclic dependency, this hash comprehension runs before `validate`, so `record` raises `KeyError` and bypasses dependency diagnostics. Insert the candidate row and validate it before computing dependency hashes.</comment>
<file context>
@@ -0,0 +1,126 @@
+ raise FileNotFoundError(path)
+ entry = dict(entry)
+ entry["sha256"] = sha256(path)
+ entry["dependency_hashes"] = {
+ dep: data["artifacts"][dep]["sha256"] for dep in entry.get("depends_on", [])
+ }
</file context>
Suggested change
| entry["dependency_hashes"] = { | |
| dep: data["artifacts"][dep]["sha256"] for dep in entry.get("depends_on", []) | |
| } | |
| data["artifacts"][ident] = entry | |
| validate(data) | |
| data["artifacts"][ident] = entry | |
| validate(data) | |
| entry["dependency_hashes"] = { | |
| dep: data["artifacts"][dep]["sha256"] for dep in entry.get("depends_on", []) | |
| } |
This was referenced Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Music should keep playing smoothly when the picture cuts, and speech should stay lined up with its captions. These helpers let the agent control music, speech and sound effects separately, and keep word timestamps aligned with the edited audio.
Builds on #164 for shared media and timing helpers.
Changes
Four focused commits, with tests alongside each feature:
references/audio-mixing.mdand expose the helpers inSKILL.md.All 83 branch tests passed, including 47 mixing/transcript cases with real FFmpeg decoding, stereo sample checks and independent master loudness measurement. CLI, comment, skill-rule and lockfile checks passed.
Limits