Skip to content

Commit 58cd56b

Browse files
sean-parentclaude
andcommitted
fix: address final review findings — doc gaps and plan-switch test
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 94a4481 commit 58cd56b

2 files changed

Lines changed: 47 additions & 3 deletions

File tree

begin/src/bridge.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ pub struct GraphData {
133133
pub changed: Vec<String>,
134134
/// Always empty; reserved for future `when`/`otherwise` conditional relationships.
135135
pub groups: Vec<GroupData>,
136-
/// `true` when a plan has been cached and links are directed; `false` for undirected.
136+
/// `true` when at least one relationship has a cached plan and links are directed where
137+
/// plans exist; `false` when no plan has been computed.
137138
pub arrows: bool,
138139
}
139140

property-model/src/sheet.rs

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,8 @@ impl Sheet {
269269
/// If `execute_plan` fails, `last_plan` is not updated; `selected_method` will continue
270270
/// to reflect the last *successful* propagation.
271271
///
272+
/// On success, `selected_method` reflects the newly computed plan.
273+
///
272274
/// # Errors
273275
///
274276
/// - `Error::Conflict` — no valid method assignment exists.
@@ -293,7 +295,8 @@ impl Sheet {
293295
/// - `Error::TypeMismatch` — a method output's runtime type does not match the cell's
294296
/// registered type.
295297
///
296-
/// - Complexity: O(R·K) where R is the number of entries and K is the max cells per method.
298+
/// - Complexity: O(R·K) where R is the number of entries and K is the max cells per method,
299+
/// plus per-method execution cost.
297300
fn execute_plan(&mut self, execution_order: &[(RelationshipId, usize)]) -> Result<(), Error> {
298301
for &(rel_id, method_idx) in execution_order {
299302
// Gather outputs in a scoped block so the shared borrows on
@@ -340,7 +343,8 @@ impl Sheet {
340343

341344
/// Returns the index of the method selected for `rel` in the last propagation.
342345
///
343-
/// Returns `None` if no propagation has run yet or `rel` is not in the cached plan.
346+
/// Returns `None` if no propagation has run yet, `rel` is not in the cached plan,
347+
/// or `rel` was added after the last `propagate()` call.
344348
pub fn selected_method(&self, rel: RelationshipId) -> Option<usize> {
345349
self.last_plan
346350
.as_ref()?
@@ -398,6 +402,8 @@ impl Sheet {
398402
///
399403
/// - `Error::Conflict` — `propagate()` has not yet been called; no plan is cached.
400404
/// - `Error::MethodFailed` — a method's function returned an error.
405+
/// - `Error::TypeMismatch` — a method output's runtime type does not match the cell's
406+
/// registered type.
401407
///
402408
/// - Complexity: O(R·K) where R is the number of relationships in the cached plan and K is the maximum cells per method, plus per-method execution cost.
403409
pub fn propagate_without_replan(&mut self) -> Result<(), Error> {
@@ -792,4 +798,41 @@ mod tests {
792798
sheet.propagate_without_replan().unwrap();
793799
assert_eq!(*sheet.read::<i32>(b).unwrap(), 10);
794800
}
801+
802+
#[test]
803+
fn selected_method_returns_none_for_invalid_id() {
804+
let sheet = Sheet::new();
805+
assert!(sheet.selected_method(RelationshipId::default()).is_none());
806+
}
807+
808+
#[test]
809+
fn propagate_without_replan_correct_after_plan_switch() {
810+
// Setup: two cells, b added last (higher strength), so b→a method is selected.
811+
// Sheet has two methods: b→a and a→b.
812+
let mut sheet = Sheet::new();
813+
let a = sheet.add_cell(0_i32);
814+
let b = sheet.add_cell(0_i32);
815+
sheet
816+
.add_relationship(vec![
817+
Method::from_fn_1_1(b, a, |x: &i32| Ok(*x * 2)),
818+
Method::from_fn_1_1(a, b, |x: &i32| Ok(*x * 3)),
819+
])
820+
.unwrap();
821+
// First propagate: b is source (added last, higher strength). b→a selected.
822+
sheet.write(b, 5_i32).unwrap();
823+
sheet.propagate().unwrap();
824+
assert_eq!(*sheet.read::<i32>(a).unwrap(), 10); // a = b * 2 = 10
825+
assert!(!sheet.is_source(a)); // a is output
826+
827+
// Write to a: raises a's strength above b, plan switches to a→b.
828+
sheet.write(a, 4_i32).unwrap();
829+
sheet.propagate().unwrap(); // plan now: a→b selected (a*3)
830+
assert_eq!(*sheet.read::<i32>(b).unwrap(), 12); // b = a * 3 = 12
831+
assert!(sheet.is_source(a)); // a is now a source
832+
833+
// Second write to a: is_source(a) is true → propagate_without_replan is safe.
834+
sheet.write(a, 7_i32).unwrap();
835+
sheet.propagate_without_replan().unwrap();
836+
assert_eq!(*sheet.read::<i32>(b).unwrap(), 21); // b = a * 3 = 21
837+
}
795838
}

0 commit comments

Comments
 (0)