feat(sources): add frame inspection and evidence tracking - #164
Open
DonIsmaelito wants to merge 1 commit into
Open
DonIsmaelito wants to merge 1 commit into
DonIsmaelito wants to merge 1 commit into
Conversation
DonIsmaelito
changed the title
DonIsmaelito
marked this pull request as ready for review
September 17, 2026 19:33
This was referenced Sep 17, 2026
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
19 issues found across 11 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="helpers/project_state.py">
<violation number="1" location="helpers/project_state.py:57">
P2: When an artifact path escapes the context directory, `record()` and `view()` fingerprint the external file anyway. Reject resolved paths outside `context.parent` in both calls before recording or reporting provenance.</violation>
<violation number="2" location="helpers/project_state.py:62">
P2: When `depends_on` names an artifact that has not been recorded yet, `record` raises an uncaught `KeyError` before graph validation runs. Validate dependency IDs before building `dependency_hashes` so the CLI reports the intended actionable validation error.</violation>
</file>
<file name="helpers/edit_io.py">
<violation number="1" location="helpers/edit_io.py:93">
P3: `last_json` is never called anywhere in the repository, so this helper adds an untested maintenance surface without affecting any workflow. Remove it until a real caller requires it.</violation>
<violation number="2" location="helpers/edit_io.py:95">
P2: When the final JSON measurement contains nested data, `last_json` returns the innermost object instead of the complete measurement. Track top-level object boundaries before selecting the last object.</violation>
</file>
<file name="helpers/find_shot.py">
<violation number="1" location="helpers/find_shot.py:36">
P2: When homography estimation returns no inliers, this mean is computed over an empty array and the search can fail during JSON serialization. Return the existing zero-inlier result when `valid.any()` is false.</violation>
<violation number="2" location="helpers/find_shot.py:50">
P2: When callers pass a non-finite `--every` value such as `nan`, this guard accepts it and the search silently samples only the first frame. Reject non-finite intervals before sampling.</violation>
<violation number="3" location="helpers/find_shot.py:72">
P2: The match report does not identify its query image, so a changed screenshot cannot be distinguished from the report that it generated. Include the query path and a SHA-256 fingerprint in the result metadata.</violation>
<violation number="4" location="helpers/find_shot.py:74">
P3: A zero reprojection error is treated as missing by `or 1e9`, which can mis-rank exact matches. Check explicitly for `None` instead.</violation>
<violation number="5" location="helpers/find_shot.py:89">
P1: When `--out` is an existing hard link to the query or source, this check passes and `save_json` truncates the input file. Reject outputs that refer to an input with `Path.samefile()` as well as resolved-path equality.</violation>
</file>
<file name="helpers/edit_clock.py">
<violation number="1" location="helpers/edit_clock.py:25">
P2: When `mode` is `"nearest"` and `fps` is zero, `seconds_to_frame` returns `0` instead of rejecting an invalid frame rate, silently producing a bogus boundary. Validate `fps > 0` before this multiplication, consistently with `render.parse_fps`.</violation>
<violation number="2" location="helpers/edit_clock.py:38">
P2: `frame_to_sample` raises `TypeError` when callers provide a float sample rate or rational frame. Convert `frame` and `rate` through `fraction()` before dividing by `fps`.</violation>
<violation number="3" location="helpers/edit_clock.py:57">
P3: When `total` is a non-integer, `allocate_frames` raises a `TypeError` while slicing the remainder count. Validate that the frame budget is an integer before allocating.</violation>
<violation number="4" location="helpers/edit_clock.py:86">
P3: `check_partition` accepts a non-integer total such as `10.0` while requiring integer shot boundaries. Reject non-integer totals so malformed frame partitions cannot pass validation.</violation>
</file>
<file name="helpers/prepare_source.py">
<violation number="1" location="helpers/prepare_source.py:13">
P2: When only the derivative’s `.log` or `.json` sidecar already exists, this guard allows the run and overwrites that existing provenance record. Check both sidecar paths alongside `out` before creating the derivative.</violation>
<violation number="2" location="helpers/prepare_source.py:25">
P3: When callers pass an empty crop sequence, `if crop` silently treats malformed input as no crop and creates an uncropped derivative. Check `crop is not None` before validating it.</violation>
<violation number="3" location="helpers/prepare_source.py:53">
P2: For sources with a non-zero start timestamp, this invocation shifts PTS because it omits `-copyts`; `-fps_mode passthrough` does not preserve the original timestamp origin. Add `-copyts` so prepared frames remain aligned with the native PTS catalog.</violation>
<violation number="4" location="helpers/prepare_source.py:83">
P2: When ffmpeg fails after creating the target, `run` leaves a partial `out`, and the next invocation refuses to retry because it treats that file as an existing derivative. Remove failed outputs or write to a temporary path and publish it only after ffmpeg succeeds.</violation>
</file>
<file name="helpers/source_scan.py">
<violation number="1" location="helpers/source_scan.py:78">
P2: When a source contains multiple video streams, `selected_frames()` can decode a different stream than `catalog()` indexed. Map the decoder explicitly to `0:v:0` so frame indices, PTS, and pixels refer to the same stream.</violation>
<violation number="2" location="helpers/source_scan.py:142">
P1: When `--out` is a hard link to the source, this path comparison misses the shared inode and `save_json` truncates the source. Reject existing output aliases with `Path.samefile` before writing.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| 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 an existing hard link to the query or source, this check passes and save_json truncates the input file. Reject outputs that refer to an input with Path.samefile() as well as resolved-path equality.
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 an existing hard link to the query or source, this check passes and `save_json` truncates the input file. Reject outputs that refer to an input with `Path.samefile()` as well as resolved-path equality.</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()): | |
| if Path(a.out).resolve() in (Path(a.query).resolve(), Path(a.source).resolve()) or ( | |
| Path(a.out).exists() | |
| and any( | |
| Path(a.out).samefile(Path(input_path)) | |
| for input_path in (a.query, a.source) | |
| ) | |
| ): |
| 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, this path comparison misses the shared inode and save_json truncates the source. Reject existing output aliases with Path.samefile before writing.
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, this path comparison misses the shared inode and `save_json` truncates the source. Reject existing output aliases with `Path.samefile` before writing.</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>
Suggested change
| if Path(args.source).resolve() == Path(args.out).resolve(): | |
| if Path(args.source).resolve() == Path(args.out).resolve() or ( | |
| Path(args.out).exists() and Path(args.source).samefile(args.out) | |
| ): |
Comment on lines
+62
to
+64
| entry["dependency_hashes"] = { | ||
| dep: data["artifacts"][dep]["sha256"] for dep in entry.get("depends_on", []) | ||
| } |
There was a problem hiding this comment.
P2: When depends_on names an artifact that has not been recorded yet, record raises an uncaught KeyError before graph validation runs. Validate dependency IDs before building dependency_hashes so the CLI reports the intended actionable validation error.
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 `depends_on` names an artifact that has not been recorded yet, `record` raises an uncaught `KeyError` before graph validation runs. Validate dependency IDs before building `dependency_hashes` so the CLI reports the intended actionable validation error.</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", []) | |
| } | |
| for dep in entry.get("depends_on", []): | |
| if dep not in data["artifacts"]: | |
| raise ValueError(f"unknown context dependency {dep}") | |
| entry["dependency_hashes"] = { | |
| dep: data["artifacts"][dep]["sha256"] for dep in entry.get("depends_on", []) | |
| } |
| # rank sampled source frames against a query image for later visual review | ||
| def search(query, source, every=1, limit=12): | ||
| """Rank sampled source frames against a query image for later visual review.""" | ||
| if every <= 0: |
There was a problem hiding this comment.
P2: When callers pass a non-finite --every value such as nan, this guard accepts it and the search silently samples only the first frame. Reject non-finite intervals before sampling.
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 50:
<comment>When callers pass a non-finite `--every` value such as `nan`, this guard accepts it and the search silently samples only the first frame. Reject non-finite intervals before sampling.</comment>
<file context>
@@ -0,0 +1,95 @@
+# rank sampled source frames against a query image for later visual review
+def search(query, source, every=1, limit=12):
+ """Rank sampled source frames against a query image for later visual review."""
+ if every <= 0:
+ raise ValueError("sampling interval must be positive")
+ index = catalog(source)
</file context>
Suggested change
| if every <= 0: | |
| if not np.isfinite(every) or every <= 0: |
| if matrix is None or mask is None: | ||
| return {"inliers": 0, "matches": len(matches), "reprojection_px": None} | ||
| projected = cv2.perspectiveTransform(a[:, None, :], matrix)[:, 0, :] | ||
| valid = mask[:, 0].astype(bool) |
There was a problem hiding this comment.
P2: When homography estimation returns no inliers, this mean is computed over an empty array and the search can fail during JSON serialization. Return the existing zero-inlier result when valid.any() is false.
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 36:
<comment>When homography estimation returns no inliers, this mean is computed over an empty array and the search can fail during JSON serialization. Return the existing zero-inlier result when `valid.any()` is false.</comment>
<file context>
@@ -0,0 +1,95 @@
+ if matrix is None or mask is None:
+ return {"inliers": 0, "matches": len(matches), "reprojection_px": None}
+ projected = cv2.perspectiveTransform(a[:, None, :], matrix)[:, 0, :]
+ valid = mask[:, 0].astype(bool)
+ return {
+ "inliers": int(valid.sum()),
</file context>
Suggested change
| valid = mask[:, 0].astype(bool) | |
| valid = mask[:, 0].astype(bool) | |
| if not valid.any(): | |
| return {"inliers": 0, "matches": len(matches), "reprojection_px": None} |
|
|
||
|
|
||
| # recover the last decodable JSON object from command output | ||
| def last_json(text): |
There was a problem hiding this comment.
P3: last_json is never called anywhere in the repository, so this helper adds an untested maintenance surface without affecting any workflow. Remove it until a real caller requires it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/edit_io.py, line 93:
<comment>`last_json` is never called anywhere in the repository, so this helper adds an untested maintenance surface without affecting any workflow. Remove it until a real caller requires it.</comment>
<file context>
@@ -0,0 +1,100 @@
+
+
+# recover the last decodable JSON object from command output
+def last_json(text):
+ """Recover the last decodable JSON object from command output."""
+ for start in reversed([i for i, c in enumerate(text) if c == "{"]):
</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.
P3: When callers pass an empty crop sequence, if crop silently treats malformed input as no crop and creates an uncropped derivative. Check crop is not None before validating it.
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 empty crop sequence, `if crop` silently treats malformed input as no crop and creates an uncropped derivative. Check `crop is not None` before validating it.</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: |
| f'non-contiguous or invalid shot: {shot.get("id", "unknown")}' | ||
| ) | ||
| cursor = end | ||
| if cursor != total: |
There was a problem hiding this comment.
P3: check_partition accepts a non-integer total such as 10.0 while requiring integer shot boundaries. Reject non-integer totals so malformed frame partitions cannot pass validation.
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 86:
<comment>`check_partition` accepts a non-integer total such as `10.0` while requiring integer shot boundaries. Reject non-integer totals so malformed frame partitions cannot pass validation.</comment>
<file context>
@@ -0,0 +1,87 @@
+ f'non-contiguous or invalid shot: {shot.get("id", "unknown")}'
+ )
+ cursor = end
+ if cursor != total:
+ raise ValueError(f"shots cover {cursor} frames, expected {total}")
</file context>
| # distribute a fixed frame budget across positive shot weights | ||
| def allocate_frames(total, weights): | ||
| """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): |
There was a problem hiding this comment.
P3: When total is a non-integer, allocate_frames raises a TypeError while slicing the remainder count. Validate that the frame budget is an integer before allocating.
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 57:
<comment>When `total` is a non-integer, `allocate_frames` raises a `TypeError` while slicing the remainder count. Validate that the frame budget is an integer before allocating.</comment>
<file context>
@@ -0,0 +1,87 @@
+# distribute a fixed frame budget across positive shot weights
+def allocate_frames(total, weights):
+ """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]
</file context>
| return { | ||
| "source_sha256": index["sha256"], | ||
| "candidates": sorted( | ||
| results, key=lambda r: (-r["inliers"], r["reprojection_px"] or 1e9) |
There was a problem hiding this comment.
P3: A zero reprojection error is treated as missing by or 1e9, which can mis-rank exact matches. Check explicitly for None instead.
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 74:
<comment>A zero reprojection error is treated as missing by `or 1e9`, which can mis-rank exact matches. Check explicitly for `None` instead.</comment>
<file context>
@@ -0,0 +1,95 @@
+ return {
+ "source_sha256": index["sha256"],
+ "candidates": sorted(
+ results, key=lambda r: (-r["inliers"], r["reprojection_px"] or 1e9)
+ )[:limit],
+ "limit": "Visual candidates need native-frame and semantic review; coarse sampling cannot certify exact action timing",
</file context>
Suggested change
| results, key=lambda r: (-r["inliers"], r["reprojection_px"] or 1e9) | |
| + results, key=lambda r: (-r["inliers"], r["reprojection_px"] if r["reprojection_px"] is not None else 1e9) |
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
Basic metadata and timeline filmstrips did not provide reusable tools for locating a screenshot in supplied footage, inspecting exact native frames, or tracking evidence after source files changed. Agents needed custom commands and separate bookkeeping, making source selection and working-copy preparation harder to reproduce.
Changes
source_scan.pyfor native frame timestamps and scene-change candidates, andfind_shot.pyfor ranked screenshot matches.prepare_source.pyfor separate cropped or HDR-converted working copies with recorded transformations and file fingerprints.project_state.pyto flag recorded evidence and its dependents as stale when their inputs change.edit_clock.pyandedit_io.py.Limits