Skip to content

feat(previews): isolate chapter renders and export layout bounds - #175

Open
DonIsmaelito wants to merge 17 commits into
browser-use:mainfrom
DonIsmaelito:submit/previews
Open

DonIsmaelito wants to merge 17 commits into
browser-use:mainfrom
DonIsmaelito:submit/previews

Conversation

@DonIsmaelito

@DonIsmaelito DonIsmaelito commented Sep 17, 2026

Copy link
Copy Markdown

Why

A failed chapter preview should not accidentally reuse an old video. Layout review also needs measurements that show when objects extend outside the frame.

Use this for Manim-based math, science and technical explainers that need chapter-by-chapter checks of animated diagrams, equations and text layouts.

Builds on #146.

Changes

  • Isolate each preview attempt by source path and a fresh output directory.
  • Expose FPS and quality controls, validate options, and reuse already decoded endpoint frames in contact sheets.
  • Export named object bounds in delivery pixels, including clipping and unrotated camera movement.
  • Two focused commits cover preview reliability and measured layout evidence. All 130 branch tests pass, including real Manim rendering.

Limits

Measured rectangles are evidence, not automatic layout approval. Rotated or perspective cameras and internal labels need separate review. Failed attempts remain on disk, and source path hashes are not content fingerprints or a render cache.

This is an incremental follow-up to #146. Measurements can feed the layout checks in #148 without requiring that PR to export them. Broader teaching helpers and workflow changes remain outside this draft.

@DonIsmaelito
DonIsmaelito marked this pull request as ready for review September 17, 2026 23:14

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

40 issues found across 48 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="skills/manim-video/assets/domains/_common.py">

<violation number="1" location="skills/manim-video/assets/domains/_common.py:111">
P2: When `start` or `end` is a Mobject, this helper draws from its center, so arrows overlap the source and destination objects. Compute boundary points along the center-to-center direction before constructing the arrow.</violation>

<violation number="2" location="skills/manim-video/assets/domains/_common.py:181">
P2: When callers pass a fractional index, `ensure_index` silently truncates it and can update the wrong element. Require an integer input before performing the range check.</violation>

<violation number="3" location="skills/manim-video/assets/domains/_common.py:189">
P1: When a transfer amount is NaN, `ensure_amount` accepts it and the domain transfer methods write NaN into both source and target balances. Reject non-finite amounts before the non-negative and availability checks.</violation>

<violation number="4" location="skills/manim-video/assets/domains/_common.py:211">
P2: When the two rows have different lengths, index-matched links are not aligned to matching columns after the rows are width-normalized. Reject unequal row lengths before creating links, or explicitly align the intended columns.</violation>
</file>

<file name="skills/manim-video/assets/teaching.py">

<violation number="1" location="skills/manim-video/assets/teaching.py:284">
P2: After `transform_object` adopts a semantic target with callable anchors, the anchors become fixed at the transform position. Preserve callables tied to the adopted children or rebind them structurally so anchors continue following later movement.</violation>

<violation number="2" location="skills/manim-video/assets/teaching.py:319">
P2: After clearing a shorthand binding, the next shorthand registration can reuse an existing auto-name and fail with `ValueError`. Generate the first unused `dependent_N` name instead.</violation>

<violation number="3" location="skills/manim-video/assets/teaching.py:473">
P2: When `focus_on` receives a nested part, it drops the entire containing scene root, so unrelated sibling parts remain fully visible. Dim unrelated descendants instead of excluding any root that contains the target.</violation>

<violation number="4" location="skills/manim-video/assets/teaching.py:667">
P1: After a prior 3D camera movement, `focus_on()` saves stale camera values and `animate=False` cannot apply its zoom or restore the rendered orientation because it writes plain attributes instead of camera trackers. Read the current tracker values and restore them through the camera setters or `move_camera()`.</violation>
</file>

<file name="skills/manim-video/references/mobjects.md">

<violation number="1" location="skills/manim-video/references/mobjects.md:110">
P2: This example raises `NameError` because `label` is never defined in the surrounding snippet. Define the text object as `label` before grouping it, or fade out `labeled_shape` directly.</violation>
</file>

<file name="skills/manim-video/assets/concept_explainer.py">

<violation number="1" location="skills/manim-video/assets/concept_explainer.py:157">
P2: Negative `margin` or `caption_rail_fraction` values move the safe bounds outside the frame and let invalid layouts pass. Validate both parameters before computing the bounds.</violation>

<violation number="2" location="skills/manim-video/assets/concept_explainer.py:196">
P2: When a final-state object is wholly outside the frame, `normalized_bounds()` returns an invalid rectangle with a negative dimension. Clamp each edge into the frame while preserving a zero-size intersection for fully clipped objects.</violation>
</file>

<file name="skills/manim-video/SKILL.md">

<violation number="1" location="skills/manim-video/SKILL.md:3">
P2: This update changes broad SKILL.md procedure and creative-direction prose outside the allowed edit scope in `AGENTS.md`. Move that procedure into the referenced documentation files and limit SKILL.md to the permitted contract changes.</violation>

<violation number="2" location="skills/manim-video/SKILL.md:206">
P2: After the documented `-qh` render, these concat paths point at the low-quality draft outputs. Use the production output directory, or make the stitch example explicitly consume a preceding `-ql` render.</violation>
</file>

<file name="skills/manim-video/assets/domains/math.py">

<violation number="1" location="skills/manim-video/assets/domains/math.py:179">
P2: When `MatrixMap` receives a non-identity matrix, the initial output arrow still matches the input arrow. Compute its endpoint using the default input vector and `values` during construction.</violation>

<violation number="2" location="skills/manim-video/assets/domains/math.py:235">
P2: When `x_value` is outside `x_range`, the constructor creates an invalid marker and guide instead of rejecting it. Validate the initial value with the same bounds as `set_x()`.</violation>

<violation number="3" location="skills/manim-video/assets/domains/math.py:323">
P2: When `amount` is NaN, `ensure_amount()` accepts it and corrupts both probability entries. Reject non-finite amounts before mutating `self.probabilities`.</violation>
</file>

<file name="skills/manim-video/assets/domains/physics.py">

<violation number="1" location="skills/manim-video/assets/domains/physics.py:81">
P2: When `move_body` runs, only the body moves while `connections` stays at its construction-time endpoints. Animate the affected connection lines with the body so the system remains joined.</violation>

<violation number="2" location="skills/manim-video/assets/domains/physics.py:139">
P2: After `set_vector` changes the arrow, the force label remains at the previous endpoint. Animate `part("label")` to the new arrow tip as part of the same update.</violation>

<violation number="3" location="skills/manim-video/assets/domains/physics.py:265">
P2: After several `flow` calls, charges leave the top wire instead of circulating around the loop. Wrap each charge to the segment or animate it along the circuit path.</violation>
</file>

<file name="skills/manim-video/assets/domains/systems.py">

<violation number="1" location="skills/manim-video/assets/domains/systems.py:75">
P2: When a long stage row triggers frame fitting, advancing the request uses an unscaled `UP * 0.30`, so the dot jumps to a different height above the target than it had at construction. Preserve the post-fit offset or derive it proportionally from the scaled stage height; apply the same correction to `DataPipeline.propagate`.</violation>

<violation number="2" location="skills/manim-video/assets/domains/systems.py:215">
P2: When a queue is moved or is large enough to trigger frame fitting, `enqueue` transforms `slots` to a fresh origin-sized group, separating it from the markers and label and potentially overflowing the frame. Build each replacement at the current slots center and scale before transforming, and apply the same fix in `dequeue`.</violation>
</file>

<file name="skills/manim-video/assets/domains/biology.py">

<violation number="1" location="skills/manim-video/assets/domains/biology.py:138">
P2: When a long sequence triggers `frame_safe`, both replacement rows ignore the fitted scale, so transcription or translation expands outside the frame and stops aligning with DNA. Apply the original fitted scale to both replacement rows before transforming them.</violation>

<violation number="2" location="skills/manim-video/assets/domains/biology.py:206">
P2: When `PopulationFlow` was frame-fitted at construction, `transfer` replaces its scaled compartments with unscaled geometry, causing the boxes to grow outside the frame and detach from the links. Scale the replacement by the component’s fitted scale before `Transform`.</violation>
</file>

<file name="skills/manim-video/assets/domains/finance.py">

<violation number="1" location="skills/manim-video/assets/domains/finance.py:92">
P2: When a finance component is large enough for `frame_safe` to shrink it, a state-changing animation rebuilds its target at the original size and breaks the frame fit and alignment. Scale and reposition each replacement target to the current registered part before returning the transform.</violation>

<violation number="2" location="skills/manim-video/assets/domains/finance.py:183">
P2: When a caller supplies a non-finite financial value, these checks accept it and produce invalid geometry or poison later balances. Reject non-finite amounts, principals, rates, and counts before constructing the components and in transfer validation.</violation>

<violation number="3" location="skills/manim-video/assets/domains/finance.py:241">
P2: When `FeedbackLoop` has 10 or more factors, the radius cap collapses adjacent arrows and nodes. Let the radius continue growing with the factor count, or reject lists that cannot fit the ring without overlap.</violation>
</file>

<file name="skills/manim-video/references/teaching-api.md">

<violation number="1" location="skills/manim-video/references/teaching-api.md:3">
P2: When an author follows the copy instruction and then uses `measured_layout_frame`, the import fails because `concept_explainer.py` was omitted. Include that asset in the setup copy list.</violation>

<violation number="2" location="skills/manim-video/references/teaching-api.md:37">
P2: Several documented actions do not return animations: `apply_force()` returns a created `Arrow`, and `dequeue()` returns an item plus a `Transform`. Qualify this statement so authors do not pass those results directly to `self.play()`.</violation>
</file>

<file name="install.md">

<violation number="1" location="install.md:135">
P2: On a fresh install, this new verification command fails because `pytest` is not installed by either documented dependency path, and the test suite does not import every helper as claimed. Run pytest through `uv` with a temporary pytest dependency or install it in the pip fallback, and describe the check as the test suite rather than an all-helper import check.</violation>

<violation number="2" location="install.md:135">
P2: A clean installation reaches this new verification step without `pytest`, so `python -m pytest -q` fails before testing any helper. Add pytest to a declared test dependency or install it before running this command.</violation>
</file>

<file name="skills/manim-video/assets/domains/computing.py">

<violation number="1" location="skills/manim-video/assets/domains/computing.py:303">
P2: When `mix_context` is called before any links exist, `context_links` is empty but `links` contains arrows, so the first transform cannot render the attention links. Populate or replace the registered group before animating it, then fade the new links in.</violation>
</file>

<file name="skills/manim-video/scripts/preview_scene.py">

<violation number="1" location="skills/manim-video/scripts/preview_scene.py:339">
P2: When a project relies on a root-level `manim.cfg` or relative project assets, this preview runs from `edit/animations` and can diverge from production rendering. Run Manim from the project root so previews load the documented project configuration and paths.</violation>
</file>

<file name="README.md">

<violation number="1" location="README.md:72">
P2: Following this optional setup does not install or check LaTeX, a hard prerequisite for the documented Manim explainer workflow. Add the platform-specific LaTeX prerequisite or point users to the Manim setup instructions before claiming the feature is set up.</violation>
</file>

<file name="tests/test_manim_teaching.py">

<violation number="1" location="tests/test_manim_teaching.py:131">
P2: The linked-value test does not verify the documented callback order because the set comparison discards ordering. Assert the ordered list of callbacks directly so an updater-order regression is detected.</violation>
</file>

<file name="skills/manim-video/references/concept-explainer.md">

<violation number="1" location="skills/manim-video/references/concept-explainer.md:49">
P2: The hard 150–185 WPM range conflicts with the instruction to begin near 145 WPM when the fixed voice rate is unknown. Keep the fallback inside the stated range, such as 150 WPM.</violation>
</file>

<file name="tests/test_manim_concept_asset.py">

<violation number="1" location="tests/test_manim_concept_asset.py:46">
P2: The `None` object is masked by `width=inf`, which fails before `measured_layout_frame` visits the object. Split this into a valid-dimension `None` case and a separate infinite-width case using a valid mobject so both validation paths are covered.</violation>
</file>

<file name="SKILL.md">

<violation number="1" location="SKILL.md:210">
P2: This adds procedure prose to `SKILL.md`, outside the repository’s documentation boundary, and duplicates the existing explainer contract. Move the details to the reference file and keep only a one-line pointer here to prevent the two contracts from drifting.</violation>
</file>

<file name="tests/test_comment_style.py">

<violation number="1" location="tests/test_comment_style.py:15">
P2: Comments containing `!`, `?`, `%`, hyphens, and other punctuation pass this guard because the regex only lists nine characters. Broaden the check to cover the full punctuation set so newly added definitions cannot bypass the convention.</violation>
</file>

<file name="AGENTS.md">

<violation number="1" location="AGENTS.md:34">
P2: The provider-independent `helpers/` contract contradicts the existing ElevenLabs-specific transcription helpers. Either move provider integrations behind an explicit adapter boundary or narrow this statement so contributors do not place or assume provider-specific code is provider-independent.</violation>

<violation number="2" location="AGENTS.md:82">
P2: This repository-wide rule sends companion-skill documentation to the wrong directory. Keep root-skill procedures in `references/`, but place companion-skill procedures in that skill's own `references/` directory so its existing relative links remain valid.</violation>
</file>

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

# validate a non negative amount and optionally that it does not exceed what is available
def ensure_amount(amount: float, *, available: float | None = None) -> float:
value = float(amount)
if value < 0:

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When a transfer amount is NaN, ensure_amount accepts it and the domain transfer methods write NaN into both source and target balances. Reject non-finite amounts before the non-negative and availability checks.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/manim-video/assets/domains/_common.py, line 189:

<comment>When a transfer amount is NaN, `ensure_amount` accepts it and the domain transfer methods write NaN into both source and target balances. Reject non-finite amounts before the non-negative and availability checks.</comment>

<file context>
@@ -0,0 +1,241 @@
+# validate a non negative amount and optionally that it does not exceed what is available
+def ensure_amount(amount: float, *, available: float | None = None) -> float:
+    value = float(amount)
+    if value < 0:
+        raise ValueError("amount cannot be negative")
+    if available is not None and value > available + 1e-9:
</file context>
Suggested change
if value < 0:
if not np.isfinite(value) or value < 0:
Fix with cubic

# capture every camera setting that focus may change
def _camera_values(self) -> dict[str, Any]:
return {
"phi": float(self.camera.phi),

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: After a prior 3D camera movement, focus_on() saves stale camera values and animate=False cannot apply its zoom or restore the rendered orientation because it writes plain attributes instead of camera trackers. Read the current tracker values and restore them through the camera setters or move_camera().

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/manim-video/assets/teaching.py, line 667:

<comment>After a prior 3D camera movement, `focus_on()` saves stale camera values and `animate=False` cannot apply its zoom or restore the rendered orientation because it writes plain attributes instead of camera trackers. Read the current tracker values and restore them through the camera setters or `move_camera()`.</comment>

<file context>
@@ -0,0 +1,730 @@
+    # capture every camera setting that focus may change
+    def _camera_values(self) -> dict[str, Any]:
+        return {
+            "phi": float(self.camera.phi),
+            "theta": float(self.camera.theta),
+            "gamma": float(self.camera.gamma),
</file context>
Fix with cubic

# FadeOut everything on screen (may contain mixed types)
self.play(FadeOut(Group(*self.mobjects)))
# A deliberate mixed-type departure can use Group.
self.play(FadeOut(Group(label, circle)))

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This example raises NameError because label is never defined in the surrounding snippet. Define the text object as label before grouping it, or fade out labeled_shape directly.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/manim-video/references/mobjects.md, line 110:

<comment>This example raises `NameError` because `label` is never defined in the surrounding snippet. Define the text object as `label` before grouping it, or fade out `labeled_shape` directly.</comment>

<file context>
@@ -106,8 +106,8 @@ shapes.set_color(BLUE)
-# FadeOut everything on screen (may contain mixed types)
-self.play(FadeOut(Group(*self.mobjects)))
+# A deliberate mixed-type departure can use Group.
+self.play(FadeOut(Group(label, circle)))

</file context>


</details>

```suggestion
self.play(FadeOut(labeled_shape))
Fix with cubic

Comment on lines +196 to +199
left = max(-config.frame_width / 2, mobject.get_left()[0] - padding)
right = min(config.frame_width / 2, mobject.get_right()[0] + padding)
top = min(config.frame_height / 2, mobject.get_top()[1] + padding)
bottom = max(-config.frame_height / 2, mobject.get_bottom()[1] - padding)

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When a final-state object is wholly outside the frame, normalized_bounds() returns an invalid rectangle with a negative dimension. Clamp each edge into the frame while preserving a zero-size intersection for fully clipped objects.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/manim-video/assets/concept_explainer.py, line 196:

<comment>When a final-state object is wholly outside the frame, `normalized_bounds()` returns an invalid rectangle with a negative dimension. Clamp each edge into the frame while preserving a zero-size intersection for fully clipped objects.</comment>

<file context>
@@ -0,0 +1,288 @@
+    if padding < 0:
+        raise ValueError("padding must be non-negative")
+    # clamp the padded bounds to the frame then convert to top left normalized units
+    left = max(-config.frame_width / 2, mobject.get_left()[0] - padding)
+    right = min(config.frame_width / 2, mobject.get_right()[0] + padding)
+    top = min(config.frame_height / 2, mobject.get_top()[1] + padding)
</file context>
Suggested change
left = max(-config.frame_width / 2, mobject.get_left()[0] - padding)
right = min(config.frame_width / 2, mobject.get_right()[0] + padding)
top = min(config.frame_height / 2, mobject.get_top()[1] + padding)
bottom = max(-config.frame_height / 2, mobject.get_bottom()[1] - padding)
left = min(max(-config.frame_width / 2, mobject.get_left()[0] - padding), config.frame_width / 2)
right = max(min(config.frame_width / 2, mobject.get_right()[0] + padding), -config.frame_width / 2)
top = max(min(config.frame_height / 2, mobject.get_top()[1] + padding), -config.frame_height / 2)
bottom = min(max(-config.frame_height / 2, mobject.get_bottom()[1] - padding), config.frame_height / 2)
Fix with cubic

cat > concat.txt << 'EOF'
file 'media/videos/script/480p15/Scene1_Introduction.mp4'
file 'media/videos/script/480p15/Scene2_CoreConcept.mp4'
file 'media/videos/script/480p15/CompositionChapter.mp4'

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: After the documented -qh render, these concat paths point at the low-quality draft outputs. Use the production output directory, or make the stitch example explicitly consume a preceding -ql render.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/manim-video/SKILL.md, line 206:

<comment>After the documented `-qh` render, these concat paths point at the low-quality draft outputs. Use the production output directory, or make the stitch example explicitly consume a preceding `-ql` render.</comment>

<file context>
@@ -108,84 +123,104 @@ project-name/
 cat > concat.txt << 'EOF'
-file 'media/videos/script/480p15/Scene1_Introduction.mp4'
-file 'media/videos/script/480p15/Scene2_CoreConcept.mp4'
+file 'media/videos/script/480p15/CompositionChapter.mp4'
+file 'media/videos/script/480p15/SystemsChapter.mp4'
 EOF
</file context>
Suggested change
file 'media/videos/script/480p15/CompositionChapter.mp4'
file 'media/videos/script/1080p60/CompositionChapter.mp4'
Fix with cubic


ROOT = Path(__file__).resolve().parents[1]
SKIP_DIRS = {".venv", "venv", "node_modules", "__pycache__", ".git", "media", "edit"}
PUNCTUATION = re.compile(r"[.,:;()\"'`]")

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Comments containing !, ?, %, hyphens, and other punctuation pass this guard because the regex only lists nine characters. Broaden the check to cover the full punctuation set so newly added definitions cannot bypass the convention.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_comment_style.py, line 15:

<comment>Comments containing `!`, `?`, `%`, hyphens, and other punctuation pass this guard because the regex only lists nine characters. Broaden the check to cover the full punctuation set so newly added definitions cannot bypass the convention.</comment>

<file context>
@@ -0,0 +1,83 @@
+
+ROOT = Path(__file__).resolve().parents[1]
+SKIP_DIRS = {".venv", "venv", "node_modules", "__pycache__", ".git", "media", "edit"}
+PUNCTUATION = re.compile(r"[.,:;()\"'`]")
+DEFINITIONS = (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)
+
</file context>
Suggested change
PUNCTUATION = re.compile(r"[.,:;()\"'`]")
PUNCTUATION = re.compile(r"[^\w\s]")
Fix with cubic

Comment thread install.md
```bash
python ~/Developer/video-use/helpers/timeline_view.py --help >/dev/null && echo "helpers OK"
ffprobe -version | head -1
cd ~/Developer/video-use && python -m pytest -q # proves every helper imports; Manim tests skip when it is not installed

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A clean installation reaches this new verification step without pytest, so python -m pytest -q fails before testing any helper. Add pytest to a declared test dependency or install it before running this command.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At install.md, line 135:

<comment>A clean installation reaches this new verification step without `pytest`, so `python -m pytest -q` fails before testing any helper. Add pytest to a declared test dependency or install it before running this command.</comment>

<file context>
@@ -132,6 +132,7 @@ Run one real thing. Prefer the lightest verification that still proves the pipel
 ```bash
 python ~/Developer/video-use/helpers/timeline_view.py --help >/dev/null && echo "helpers OK"
 ffprobe -version | head -1
+cd ~/Developer/video-use && python -m pytest -q   # proves every helper imports; Manim tests skip when it is not installed

</file context>


</details>

<a href="https://www.cubic.dev/action/fix/violation/4225f562-55d7-4177-88b5-b43dcf0d715b" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true">
  <picture>
    <source media="(prefers-color-scheme: dark)" srcset="https://cubic.dev/buttons/fix-with-cubic-dark.svg">
    <source media="(prefers-color-scheme: light)" srcset="https://cubic.dev/buttons/fix-with-cubic-light.svg">
    <img alt="Fix with cubic" src="https://cubic.dev/buttons/fix-with-cubic-dark.svg">
  </picture>
</a>

if isinstance(name, Mobject):
dependent = name
callback = mobject
binding_name = f"dependent_{len(self._bindings)}"

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: After clearing a shorthand binding, the next shorthand registration can reuse an existing auto-name and fail with ValueError. Generate the first unused dependent_N name instead.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/manim-video/assets/teaching.py, line 319:

<comment>After clearing a shorthand binding, the next shorthand registration can reuse an existing auto-name and fail with `ValueError`. Generate the first unused `dependent_N` name instead.</comment>

<file context>
@@ -0,0 +1,730 @@
+        if isinstance(name, Mobject):
+            dependent = name
+            callback = mobject
+            binding_name = f"dependent_{len(self._bindings)}"
+        else:
+            binding_name = name
</file context>
Fix with cubic

Comment thread AGENTS.md
Comment on lines +82 to +85
- Procedure prose belongs in `references/<feature>.md` at the repository root;
create that folder with the first reference file. Edits to `SKILL.md`
are limited to rules, helper-index bullets, directory-tree lines, the EDL
example, and one-line pointers to the reference files.

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This repository-wide rule sends companion-skill documentation to the wrong directory. Keep root-skill procedures in references/, but place companion-skill procedures in that skill's own references/ directory so its existing relative links remain valid.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At AGENTS.md, line 82:

<comment>This repository-wide rule sends companion-skill documentation to the wrong directory. Keep root-skill procedures in `references/`, but place companion-skill procedures in that skill's own `references/` directory so its existing relative links remain valid.</comment>

<file context>
@@ -0,0 +1,87 @@
+  history, so a later sync silently discards it.
+- Hard rules in `SKILL.md` are append-only. New rules get the next number.
+  Removing or renumbering a rule requires an explicit reason in the commit.
+- Procedure prose belongs in `references/<feature>.md` at the repository root;
+  create that folder with the first reference file. Edits to `SKILL.md`
+  are limited to rules, helper-index bullets, directory-tree lines, the EDL
</file context>
Suggested change
- Procedure prose belongs in `references/<feature>.md` at the repository root;
create that folder with the first reference file. Edits to `SKILL.md`
are limited to rules, helper-index bullets, directory-tree lines, the EDL
example, and one-line pointers to the reference files.
- Procedure prose for the root skill belongs in `references/<feature>.md`; companion-skill prose belongs in that skill's `references/` directory.
Create the relevant directory with the first reference file. Edits to `SKILL.md` are limited to rules, helper-index bullets, directory-tree lines, the EDL example, and one-line pointers to reference files.
Fix with cubic

Comment thread AGENTS.md
## Architecture boundaries

- `SKILL.md` defines the agent workflow and public editing contract.
- `helpers/` contains provider-independent production tools and validation.

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The provider-independent helpers/ contract contradicts the existing ElevenLabs-specific transcription helpers. Either move provider integrations behind an explicit adapter boundary or narrow this statement so contributors do not place or assume provider-specific code is provider-independent.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At AGENTS.md, line 34:

<comment>The provider-independent `helpers/` contract contradicts the existing ElevenLabs-specific transcription helpers. Either move provider integrations behind an explicit adapter boundary or narrow this statement so contributors do not place or assume provider-specific code is provider-independent.</comment>

<file context>
@@ -0,0 +1,87 @@
+## Architecture boundaries
+
+- `SKILL.md` defines the agent workflow and public editing contract.
+- `helpers/` contains provider-independent production tools and validation.
+- `skills/` contains focused companion skills and reusable production assets.
+- `tests/` protects public behavior. Optional clients or remote runners may
</file context>
Suggested change
- `helpers/` contains provider-independent production tools and validation.
- `helpers/` contains reusable production tools and narrow provider adapters.
Fix with cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant