Skip to content

Collapse obsolete live-preview direct-delta API after PreviewTyper move #12

Description

@BumpyClock

Problem

The streaming feed loop in crates/core/src/controller/live_preview.rs was reworked so the dedicated PreviewTyper thread owns the on-screen state and computes its own common-prefix delta. As a result, the older "direct delta" plumbing that apply_streaming_transcript and PreviewTyper::finish still expose is now dead surface area.

1. The publish closure ignores old and never fails. run_streaming_loop (live_preview.rs:265) builds a closure whose only job is to forward the latest text to the typer (live_preview.rs:295-298):

let mut publish = |_old: &str, new: &str| -> Result<(), String> {
    typer.publish(new.to_string());
    Ok(())
};

It discards _old and always returns Ok(()). Real delta errors are now handled inside the typer thread (PreviewTyper::runclipboard::apply_preview_delta, live_preview.rs:516-522), so the Result here is a permanently-Ok path.

2. apply_streaming_transcript carries an old/new/Result contract for a dead failure path. Its signature still requires the old text and an error branch (live_preview.rs:390-401):

fn apply_streaming_transcript<F>(last_applied: &mut String, text: &str, apply_delta: &mut F)
where
    F: FnMut(&str, &str) -> Result<(), String>,
{
    let text = text.trim();
    if text != last_applied {
        match apply_delta(last_applied, text) {
            Ok(()) => *last_applied = text.to_string(),
            Err(err) => log::error!("Live preview streaming delta failed: {}", err),
        }
    }
}

The only production caller passes the always-Ok publish closure (live_preview.rs:300-306 and live_preview.rs:330-334), so the Err arm is unreachable in production.

3. PreviewTyper::finish takes a final_text handoff that production always calls as finish(None). Signature at live_preview.rs:472:

fn finish(mut self, final_text: Option<&str>) -> String {

Both production call sites pass None: the streaming path at live_preview.rs:347 and the batch path at live_preview.rs:232. The final text is already published via apply_streaming_transcript before finish (live_preview.rs:330-334), so the Some(text) branch inside finish (live_preview.rs:475-477) is never exercised by the app.

Why it matters

  • Maintainability: three coupled signatures (publish closure, apply_streaming_transcript, PreviewTyper::finish) advertise an old-text + Result + final-text-handoff contract that production no longer uses. A reader must trace all three to confirm the failure/handoff paths are dead, which obscures the real design (typer owns state + diff).
  • Risk surface, not hot-path cost: this runs once per utterance, not per audio chunk, so there's no perf angle. The cost is the dead Err/Some branches that invite incorrect assumptions during future edits to the finalization flow.

Proposed change

Collapse to a latest-text-only API, in a single focused PR:

  1. Change apply_streaming_transcript to take apply_delta: &mut F where F: FnMut(&str) (drop the old param and the Result); apply unconditionally on change and update last_applied.
  2. Simplify the publish closure in run_streaming_loop to |new: &str| typer.publish(new.to_string()).
  3. Make PreviewTyper::finish no-arg (fn finish(mut self) -> String) and drop the Some(text) pre-seed branch — unless review finds a real caller that needs the final_text handoff. Update the streaming (live_preview.rs:347) and batch (live_preview.rs:232) call sites.

Tests that must change (they currently encode the old contract and act as the guard):

  • streaming_transcript_helper_records_deltas (live_preview.rs:864) and streaming_transcript_helper_skips_duplicate_text (live_preview.rs:894) use an apply_delta of |old, new| { ...; Ok(()) } and assert on (old, new) tuples — rewrite for the FnMut(&str) signature while preserving the dedup and delta-recording assertions.
  • batch_append_accumulates_text_with_spaces (live_preview.rs:724) already uses FnMut(&str) for the batch path — leave as-is, it's a model for the new streaming-helper test shape.

Tests that must stay green unchanged (the correctness guard for the is_final-after-typer fix):

  • pending_cap_drop_forces_unfinalized_streaming_result (live_preview.rs:629)
  • confirm_streaming_finalization_returns_true_when_text_landed (live_preview.rs:919)
  • confirm_streaming_finalization_falls_back_when_text_not_applied (live_preview.rs:928)
  • streaming_preview_reports_unfinalized_when_reset_fails (live_preview.rs:847)

Recommendation / triage

DEFER / do carefully. This area was deliberately reworked for the recent is_final-after-typer correctness fix (commit 62a9d3f, "fix(live-preview): emit is_final only after typer confirms text landed"). Any collapse must keep the confirm_streaming_finalization behavior intact — the final text is published, then the typer must prove it landed on screen before streaming_finalized stays true. Do this as a focused PR with the live_preview streaming tests as the guard, not a drive-by. If review surfaces any real future need for the final_text handoff in finish, keep that param and only collapse the closure + apply_streaming_transcript.

Acceptance criteria

  • apply_streaming_transcript takes FnMut(&str); no old param, no Result branch.
  • run_streaming_loop publish closure forwards only the latest text (no _old, no Ok(())).
  • PreviewTyper::finish is no-arg, OR the final_text param is retained with a documented real caller (decision recorded in the PR).
  • Behavior unchanged: confirm_streaming_finalization still gates streaming_finalized on the typer's confirmed on-screen text; pending-cap drop still forces batch fallback.
  • Guard tests pass unchanged: pending_cap_drop_forces_unfinalized_streaming_result, both confirm_streaming_finalization_*, streaming_preview_reports_unfinalized_when_reset_fails.
  • Rewritten helper tests (streaming_transcript_helper_records_deltas, streaming_transcript_helper_skips_duplicate_text) assert dedup + delta application against the new FnMut(&str) signature.
  • No change to clipboard::apply_preview_delta or PreviewTyper::run diff logic.

Traceability

  • clawpatch finding ID: fnd_sig-feat-service-39545238a6-ce5f_91fb601faf
  • Revalidate: clawpatch revalidate --finding fnd_sig-feat-service-39545238a6-ce5f_91fb601faf --reasoning-effort high
  • Coupled context: builds on the is_final-after-typer correctness fix (commit 62a9d3f); this is the maintainability follow-up to retire the now-dead direct-delta API left behind by that move.

Metadata

Metadata

Assignees

No one assigned

    Labels

    deslopifyFound by clawpatch deslopify reviewtech-debtCode quality / maintainability cleanup

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions