Skip to content

Commit cee8317

Browse files
committed
fix(cockpit): bind curation action to its chunk + rollback on persist fail (PR #100 review)
CodeRabbit review of the Slice 6 inspector: - bind each pending CurationAction to the chunk id it was rendered for, so a same-frame row click can't redirect the edit to the newly-selected chunk (apply_curation takes an explicit id now, not self.selected). - roll the in-memory edit back when persist_chunk fails (the native fs::write path), so the dock never shows a change that didn't reach disk. - mark undecided chunks (reviewer None) with "?" like an explicit NeedsReview. Skipped the hash-in-filename suggestion: the corpus is the readable, CLI-byte- compatible chunk.json tree (ADR-0027 section 3), so hex-encoding the id would break that convention; ids are unique slugs in the normal flow and the chunk's internal id stays authoritative. Per the maintainer, the optimistic OPFS "saved" (matching capture's fire-and-forget) and the single-commit layout stand (as #99). cockpit 23 tests pass; native + wasm clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
1 parent 91aa38e commit cee8317

1 file changed

Lines changed: 33 additions & 15 deletions

File tree

‎cockpit/src/lib.rs‎

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,8 @@ fn chunk_row(ui: &mut egui::Ui, chunk: &ChunkMeta, selected: bool) -> bool {
231231
let mark = match chunk.reviewer {
232232
Some(ReviewerDecision::Accepted) => "✓ ",
233233
Some(ReviewerDecision::Rejected) => "✗ ",
234-
Some(ReviewerDecision::NeedsReview) => "? ",
235-
None => "",
234+
// No decision yet reads as "needs a look", like an explicit NeedsReview.
235+
Some(ReviewerDecision::NeedsReview) | None => "? ",
236236
};
237237
let dup = if chunk.duplicate.is_some() { "≈ " } else { "" };
238238
let clicked = ui
@@ -660,18 +660,30 @@ impl CockpitApp {
660660
Ok(edited)
661661
}
662662

663-
/// Curates the selected chunk and persists it back to the corpus, recording
664-
/// the outcome in the dock's status line.
665-
fn apply_curation(&mut self, action: &CurationAction) {
666-
let Some(id) = self.selected.clone() else {
667-
return;
668-
};
669-
self.dock_status = Some(match self.curate(&id, action) {
663+
/// Curates the chunk `id` and persists it back to the corpus, recording the
664+
/// outcome in the dock's status line. On a (synchronous, native) persist
665+
/// failure the in-memory edit is rolled back so the dock matches disk.
666+
fn apply_curation(&mut self, id: &str, action: &CurationAction) {
667+
let snapshot = self
668+
.corpus
669+
.iter()
670+
.position(|c| c.id.0 == id)
671+
.and_then(|idx| self.corpus.get(idx).cloned().map(|chunk| (idx, chunk)));
672+
self.dock_status = Some(match self.curate(id, action) {
670673
Ok(json) => {
671-
let filename = chunk_filename(&id);
674+
let filename = chunk_filename(id);
672675
match persist_chunk(&filename, &json) {
673676
Ok(()) => format!("saved {filename}"),
674-
Err(err) => format!("save failed: {err}"),
677+
Err(err) => {
678+
// The persist failed — undo the in-memory edit so the dock
679+
// never shows a change that didn't reach disk.
680+
if let Some((idx, before)) = snapshot {
681+
if let Some(slot) = self.corpus.get_mut(idx) {
682+
*slot = before;
683+
}
684+
}
685+
format!("save failed: {err}")
686+
}
675687
}
676688
}
677689
Err(err) => format!("curate failed: {err}"),
@@ -683,7 +695,7 @@ impl CockpitApp {
683695
/// loaded `corpus`. On web the 📚 toolbar button fills it from OPFS; `c` toggles.
684696
fn corpus_dock(&mut self, ctx: &egui::Context) {
685697
let mut clicked: Option<String> = None;
686-
let mut action: Option<CurationAction> = None;
698+
let mut action: Option<(String, CurationAction)> = None;
687699
egui::Window::new("corpus").default_width(360.0).show(ctx, |ui| {
688700
let stats = CorpusStats::aggregate(&self.corpus);
689701
if stats.total == 0 {
@@ -732,7 +744,13 @@ impl CockpitApp {
732744
if let Some(id) = self.selected.clone() {
733745
if let Some(chunk) = self.corpus.iter().find(|c| c.id.0 == id) {
734746
ui.separator();
735-
inspector(ui, chunk, &mut self.rename_buf, &mut action);
747+
// Bind any edit to *this* chunk's id, so a same-frame row click
748+
// (which changes the selection below) can't redirect it.
749+
let mut pending = None;
750+
inspector(ui, chunk, &mut self.rename_buf, &mut pending);
751+
if let Some(pending) = pending {
752+
action = Some((chunk.id.0.clone(), pending));
753+
}
736754
}
737755
}
738756
if let Some(status) = &self.dock_status {
@@ -749,8 +767,8 @@ impl CockpitApp {
749767
}
750768
self.selected = Some(id);
751769
}
752-
if let Some(action) = action {
753-
self.apply_curation(&action);
770+
if let Some((id, action)) = action {
771+
self.apply_curation(&id, &action);
754772
}
755773
}
756774

0 commit comments

Comments
 (0)