8d33fc5a

Avoid double DAG walk after auto_detect_revision

a73x   2026-03-21 19:08

Change auto_detect_revision to return Option<Revision> instead of u32,
so callers can push the new revision onto their in-memory PatchState
rather than re-materializing the entire DAG via from_ref_uncached.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---

diff --git a/src/patch.rs b/src/patch.rs
index dba1b83..21d7e8b 100644
--- a/src/patch.rs
+++ b/src/patch.rs
@@ -6,47 +6,55 @@ use crate::error::Error;
use crate::event::{Action, Event, ReviewVerdict};
use crate::identity::get_author;
use crate::signing;
use crate::state::{self, PatchState, PatchStatus};
use crate::state::{self, PatchState, PatchStatus, Revision};

/// Auto-detect whether the branch tip has changed since the last recorded revision.
/// If it has, append a PatchRevision event. Returns the current revision number.
/// If it has, append a PatchRevision event and return the new `Revision` so callers
/// can update their in-memory `PatchState` without re-walking the DAG.
fn auto_detect_revision(
    repo: &Repository,
    ref_name: &str,
    patch: &PatchState,
    sk: &ed25519_dalek::SigningKey,
) -> Result<u32, Error> {
) -> Result<Option<Revision>, Error> {
    let current_rev = patch.revisions.last().map(|r| r.number).unwrap_or(0);

    // Try to resolve the branch tip
    let tip_oid = match patch.resolve_head(repo) {
        Ok(oid) => oid,
        Err(_) => return Ok(current_rev), // Branch deleted or unavailable
        Err(_) => return Ok(None), // Branch deleted or unavailable
    };

    let last_commit = patch.revisions.last().map(|r| r.commit.as_str()).unwrap_or("");
    let tip_hex = tip_oid.to_string();

    if tip_hex == last_commit {
        return Ok(current_rev);
        return Ok(None);
    }

    // Branch tip changed — insert a PatchRevision event
    let commit = repo.find_commit(tip_oid)?;
    let tree_oid = commit.tree()?.id();
    let author = get_author(repo)?;
    let timestamp = chrono::Utc::now().to_rfc3339();
    let event = Event {
        timestamp: chrono::Utc::now().to_rfc3339(),
        timestamp: timestamp.clone(),
        author,
        action: Action::PatchRevision {
            commit: tip_hex,
            commit: tip_hex.clone(),
            tree: tree_oid.to_string(),
            body: None,
        },
        clock: 0,
    };
    dag::append_event(repo, ref_name, &event, sk)?;
    Ok(current_rev + 1)
    Ok(Some(Revision {
        number: current_rev + 1,
        commit: tip_hex,
        tree: tree_oid.to_string(),
        body: None,
        timestamp,
    }))
}

pub fn create(
@@ -209,14 +217,10 @@ pub fn comment(
    let patch = PatchState::from_ref(repo, &ref_name, &id)?;

    // Auto-detect revision (runs for all comment types to record branch changes)
    let current_rev = auto_detect_revision(repo, &ref_name, &patch, &sk)?;

    // Re-read state after potential revision insertion
    let patch = if current_rev != patch.revisions.last().map(|r| r.number).unwrap_or(0) {
        PatchState::from_ref_uncached(repo, &ref_name, &id)?
    } else {
        patch
    };
    let mut patch = patch;
    if let Some(rev) = auto_detect_revision(repo, &ref_name, &patch, &sk)? {
        patch.revisions.push(rev);
    }

    let author = get_author(repo)?;

@@ -280,14 +284,10 @@ pub fn review(
    let patch = PatchState::from_ref(repo, &ref_name, &id)?;

    // Auto-detect revision
    let current_rev = auto_detect_revision(repo, &ref_name, &patch, &sk)?;

    // Re-read state after potential revision insertion
    let patch = if current_rev != patch.revisions.last().map(|r| r.number).unwrap_or(0) {
        PatchState::from_ref_uncached(repo, &ref_name, &id)?
    } else {
        patch
    };
    let mut patch = patch;
    if let Some(rev) = auto_detect_revision(repo, &ref_name, &patch, &sk)? {
        patch.revisions.push(rev);
    }

    let rev = if let Some(target) = target_revision {
        if !patch.revisions.iter().any(|r| r.number == target) {
@@ -366,13 +366,14 @@ pub fn merge(repo: &Repository, id_prefix: &str) -> Result<PatchState, crate::er
    }

    // Auto-detect revision before merge
    auto_detect_revision(repo, &ref_name, &p, &sk)?;
    let mut p = p;
    if let Some(rev) = auto_detect_revision(repo, &ref_name, &p, &sk)? {
        p.revisions.push(rev);
    }

    // Check merge policy
    let config = read_collab_config(repo);
    if config.require_approval_on_latest {
        // Re-read state after potential revision insertion
        let p = PatchState::from_ref_uncached(repo, &ref_name, &id)?;
        let latest_rev = p.revisions.last().map(|r| r.number).unwrap_or(0);
        let has_approval = p.reviews.iter().any(|r| {
            r.verdict == ReviewVerdict::Approve && r.revision == Some(latest_rev)