[codex] harden atomic writes and checks

.github/workflows/release.yml

@@ -1,8 +1,8 @@
 name: release
 
 # Build wheels and an sdist for `pyvedit`.
-# - On push to main / pull requests: build all targets, run Rust tests,
-#   but do not publish.
+# - On push to main / pull requests: build all targets, run Rust/Python
+#   tests plus Rust formatting and clippy checks, but do not publish.
 # - On a `v*` tag (e.g. `v0.0.1`): build all targets, then publish to
 #   PyPI using the PYPI_API_TOKEN secret.
 #
@@ -19,16 +19,42 @@ permissions:
   contents: read
 
 jobs:
-  rust-tests:
-    name: Rust tests
+  rust-checks:
+    name: Rust checks
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
+        with:
+          components: rustfmt, clippy
       - uses: Swatinem/rust-cache@v2
+      - name: cargo fmt
+        run: cargo fmt --check
+      - name: cargo clippy
+        run: cargo clippy --workspace --exclude vedit-py -- -D warnings
       - name: cargo test
         run: cargo test --workspace --exclude vedit-py
 
+  python-tests:
+    name: Python tests
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+      - uses: Swatinem/rust-cache@v2
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.12"
+      - name: Create test venv
+        run: python -m venv .venv
+      - name: Install test tooling
+        run: .venv/bin/python -m pip install --upgrade pip maturin pytest
+      - name: Build extension in place
+        working-directory: crates/vedit-py
+        run: ../../.venv/bin/maturin develop --release
+      - name: pytest
+        run: .venv/bin/python -m pytest crates/vedit-py/tests/
+
   build-linux:
     name: Wheels - Linux ${{ matrix.target }}
     runs-on: ubuntu-latest
@@ -113,7 +139,15 @@ jobs:
 
   publish-to-pypi:
     name: Publish to PyPI
-    needs: [rust-tests, build-linux, build-macos, build-windows, build-sdist]
+    needs:
+      [
+        rust-checks,
+        python-tests,
+        build-linux,
+        build-macos,
+        build-windows,
+        build-sdist,
+      ]
     if: startsWith(github.ref, 'refs/tags/v')
     runs-on: ubuntu-latest
     environment:

crates/vedit-cli/src/cmd/checkout.rs

@@ -31,9 +31,7 @@ pub fn run(refstr: &str, output: Option<&Path>) -> Result<()> {
 
     // Branch-switch mode. The ref must be a branch.
     if repo.branch_target(refstr)?.is_none() {
-        anyhow::bail!(
-            "{refstr} is not a branch. Use `-o <path>` to write a timeline by hash."
-        );
+        anyhow::bail!("{refstr} is not a branch. Use `-o <path>` to write a timeline by hash.");
     }
     repo.switch_branch(refstr)?;
     println!("Switched to branch {refstr}");

crates/vedit-cli/src/cmd/log.rs

@@ -22,9 +22,10 @@ pub fn run(refstr: &str) -> Result<()> {
         let mut tags: Vec<String> = Vec::new();
         if let Some((head_label, branch_name)) = &head_target
             && let Ok(Some(target)) = repo.branch_target(branch_name)
-                && &target == hash {
-                    tags.push(format!("{head_label} -> {branch_name}"));
-                }
+            && &target == hash
+        {
+            tags.push(format!("{head_label} -> {branch_name}"));
+        }
         let tag_str = if tags.is_empty() {
             String::new()
         } else {

crates/vedit-cli/src/cmd/merge.rs

@@ -10,8 +10,8 @@
 //!     and exit non-zero — v0.6 has no in-place resolution UX.
 
 use crate::author;
-use anyhow::{anyhow, bail, Result};
-use vedit_core::merge::{merge as run_merge, MergeOutcome};
+use anyhow::{Result, anyhow, bail};
+use vedit_core::merge::{MergeOutcome, merge as run_merge};
 use vedit_core::object;
 use vedit_core::otio;
 use vedit_core::repo::{HeadState, Repo};
@@ -57,10 +57,7 @@ pub fn run(target: &str, options: MergeOptions) -> Result<()> {
             );
             return Ok(());
         }
-        std::fs::write(
-            repo.root.join("refs/heads").join(&current_branch),
-            format!("{target_hash}\n"),
-        )?;
+        repo.set_branch_target(&current_branch, &target_hash)?;
         println!(
             "Fast-forwarded {current_branch} from {} to {}",
             short(&head_hash),
@@ -118,9 +115,9 @@ pub fn run(target: &str, options: MergeOptions) -> Result<()> {
             let merged_value = synthesize_otio(&repo, &head_hash, &target_hash, &merged_timeline)?;
             let timeline_hash = repo.write_timeline(&merged_value)?;
 
-            let message = options.message.unwrap_or_else(|| {
-                format!("Merge branch '{target}' into {current_branch}")
-            });
+            let message = options
+                .message
+                .unwrap_or_else(|| format!("Merge branch '{target}' into {current_branch}"));
 
             let commit_hash = repo.commit_with_parents(
                 &timeline_hash,
@@ -221,16 +218,18 @@ fn synthesize_otio(
         // Prefer ours's JSON if structurally equal.
         if let Some(t) = ours_index.get(&key)
             && *t == merged_track
-                && let Some(json) = ours_tracks_json.remove(&key) {
-                    new_track_children.push(json);
-                    continue;
-                }
+            && let Some(json) = ours_tracks_json.remove(&key)
+        {
+            new_track_children.push(json);
+            continue;
+        }
         if let Some(t) = theirs_index.get(&key)
             && *t == merged_track
-                && let Some(json) = theirs_tracks_json.remove(&key) {
-                    new_track_children.push(json);
-                    continue;
-                }
+            && let Some(json) = theirs_tracks_json.remove(&key)
+        {
+            new_track_children.push(json);
+            continue;
+        }
         // Fallback: emit a minimal-but-valid Track JSON from our model.
         new_track_children.push(synth_track_json(merged_track));
     }
@@ -300,12 +299,12 @@ fn describe_conflict(kind: &vedit_core::merge::ConflictKind) -> String {
     match kind {
         BothModified => "both branches modified this track".to_string(),
         BothAdded => "both branches added this track with different content".to_string(),
-        DeleteVsModify { deleter: Side::Ours } => {
-            "ours deleted this track but theirs modified it".to_string()
-        }
-        DeleteVsModify { deleter: Side::Theirs } => {
-            "theirs deleted this track but ours modified it".to_string()
-        }
+        DeleteVsModify {
+            deleter: Side::Ours,
+        } => "ours deleted this track but theirs modified it".to_string(),
+        DeleteVsModify {
+            deleter: Side::Theirs,
+        } => "theirs deleted this track but ours modified it".to_string(),
     }
 }
 

crates/vedit-cli/src/diff/render.rs

@@ -3,7 +3,10 @@ use vedit_core::model::{RationalTime, TimeRange};
 
 pub fn render(changes: &[Change]) -> Vec<String> {
     let collapsed = collapse_synced_pairs(changes);
-    collapsed.iter().map(|(c, synced)| render_one(c, *synced)).collect()
+    collapsed
+        .iter()
+        .map(|(c, synced)| render_one(c, *synced))
+        .collect()
 }
 
 /// Detect video/audio mirror pairs and collapse them. Two changes are
@@ -44,28 +47,70 @@ fn mirrors(a: &Change, b: &Change) -> bool {
     use Change::*;
     match (a, b) {
         (
-            Trimmed { clip: ca, before: ba, after: aa, track: ta },
-            Trimmed { clip: cb, before: bb, after: ab, track: tb },
+            Trimmed {
+                clip: ca,
+                before: ba,
+                after: aa,
+                track: ta,
+            },
+            Trimmed {
+                clip: cb,
+                before: bb,
+                after: ab,
+                track: tb,
+            },
         ) => ta != tb && ca.name == cb.name && ba == bb && aa == ab,
         (
-            Moved { clip: ca, after_neighbor: na, before_neighbor: pa, .. },
-            Moved { clip: cb, after_neighbor: nb, before_neighbor: pb, .. },
-        ) => {
-            ca.name == cb.name
-                && neighbor_names_match(na, nb)
-                && neighbor_names_match(pa, pb)
-        }
+            Moved {
+                clip: ca,
+                after_neighbor: na,
+                before_neighbor: pa,
+                ..
+            },
+            Moved {
+                clip: cb,
+                after_neighbor: nb,
+                before_neighbor: pb,
+                ..
+            },
+        ) => ca.name == cb.name && neighbor_names_match(na, nb) && neighbor_names_match(pa, pb),
         (
-            Added { clip: ca, track: ta, .. },
-            Added { clip: cb, track: tb, .. },
+            Added {
+                clip: ca,
+                track: ta,
+                ..
+            },
+            Added {
+                clip: cb,
+                track: tb,
+                ..
+            },
         ) => ta != tb && ca.name == cb.name,
         (
-            Removed { clip: ca, track: ta, .. },
-            Removed { clip: cb, track: tb, .. },
+            Removed {
+                clip: ca,
+                track: ta,
+                ..
+            },
+            Removed {
+                clip: cb,
+                track: tb,
+                ..
+            },
         ) => ta != tb && ca.name == cb.name,
         (
-            EffectsChanged { clip: ca, before: ba, after: aa, track: ta },
-            EffectsChanged { clip: cb, before: bb, after: ab, track: tb },
+            EffectsChanged {
+                clip: ca,
+                before: ba,
+                after: aa,
+                track: ta,
+            },
+            EffectsChanged {
+                clip: cb,
+                before: bb,
+                after: ab,
+                track: tb,
+            },
         ) => ta != tb && ca.name == cb.name && ba == bb && aa == ab,
         (
             TransitionAdded {
@@ -83,10 +128,7 @@ fn mirrors(a: &Change, b: &Change) -> bool {
                 ..
             },
         ) => {
-            ta != tb
-                && neighbor_names_match(ba1, bb1)
-                && neighbor_names_match(ba2, bb2)
-                && da == db
+            ta != tb && neighbor_names_match(ba1, bb1) && neighbor_names_match(ba2, bb2) && da == db
         }
         (
             TransitionRemoved {
@@ -101,11 +143,7 @@ fn mirrors(a: &Change, b: &Change) -> bool {
                 track: tb,
                 ..
             },
-        ) => {
-            ta != tb
-                && neighbor_names_match(ba1, bb1)
-                && neighbor_names_match(ba2, bb2)
-        }
+        ) => ta != tb && neighbor_names_match(ba1, bb1) && neighbor_names_match(ba2, bb2),
         _ => false,
     }
 }
@@ -127,7 +165,12 @@ fn render_one(change: &Change, synced: bool) -> String {
         Change::TrackRemoved { name, kind } => {
             format!("  Removed {} track \"{}\"", track_kind_word(kind), name)
         }
-        Change::Trimmed { clip, before, after, .. } => {
+        Change::Trimmed {
+            clip,
+            before,
+            after,
+            ..
+        } => {
             format!("{}{}", render_trim(clip, before, after), suffix)
         }
         Change::Moved {
@@ -139,7 +182,13 @@ fn render_one(change: &Change, synced: bool) -> String {
             ..
         } => format!(
             "{}{}",
-            render_move(clip, *from_index, *to_index, after_neighbor, before_neighbor),
+            render_move(
+                clip,
+                *from_index,
+                *to_index,
+                after_neighbor,
+                before_neighbor
+            ),
             suffix
         ),
         Change::Added { clip, track, index } => format!(
@@ -156,14 +205,24 @@ fn render_one(change: &Change, synced: bool) -> String {
             index,
             suffix
         ),
-        Change::EffectsChanged { clip, before, after, .. } => format!(
+        Change::EffectsChanged {
+            clip,
+            before,
+            after,
+            ..
+        } => format!(
             "  Effects on \"{}\" changed ({} → {}){}",
             clip_label(clip),
             before,
             after,
             suffix
         ),
-        Change::Replaced { clip, before_media, after_media, .. } => format!(
+        Change::Replaced {
+            clip,
+            before_media,
+            after_media,
+            ..
+        } => format!(
             "{}{}",
             render_replaced(clip, before_media, after_media),
             suffix