From 9b91940d9a78c6dfaa93c44b3e8a09682ec2d7c8 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Tue, 22 Sep 2026 16:30:58 +0200 Subject: [PATCH] Stop the play table's own tests from fooling themselves (PA.f77, PA.f79, PA.f81) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PA.f77: table_view_prints_only_a_subjects_own_granted_affordances found the holder's row with an unscoped view.find("{label} [{id}]"), which can match the same marker in the Places occupant list first and then read whichever subject row sorts first out of that mismatch — flaky about 1 run in 4. Route it through the existing subject_row helper, which searches only from the Subjects section onward. Swept the rest of the test module for other unscoped find/contains that could cross into the Places occupant list or a fact's known-by list; every other one was already either an intentional "appears anywhere" check or already scoped through subject_row/ view_section. Stability: 8 sequential runs of the table:: suite, 41/41 passing every time. PA.f79: two of the table's own refusal tests could pass under a mutated implementation. describe_refusal_names_unresolved_drafts_handle_and_expected_kind used a handle literally named "missing_place", so a mutation that replaced the rendered expected-kind with a fixed "thing" still passed the "place" containment check via the handle text alone. Renamed the handle to "unresolved_target" and pinned the exact rendered sentence. describe_refusal_names_both_calls_for_a_duplicate_handle only ever gave describe_duplicate_handle_sites two declarations to filter, both colliding, so a filter that matched every declaration in the batch regardless of handle produced the same "call #0 and call #1" text. Added a third, unrelated declaration and asserted it is not named. PA.f81: nothing pinned describe_refusal_to_actor's exact wording — every existing test used `contains`, so appending a `{:?}` dump of the failed precondition (leaking internal Mismatch shape the actor's own call never handed back) would have survived all of them. Added assert_eq! tests for one role-bearing precondition failure (CanBroadcast/NoAudience, a `horn` channel role) and one roleless one (HasStanding/NoStanding), pinning the full string each renders. Every fix above was run against its own revert/loosening mutation and confirmed to fail before being restored: the raw is_alphanumeric-neighbour scoping wasn't touched here (that's PA.f78, committed separately), the fixed-"thing" mutation, the match-every-declaration filter, and the {:?}-append mutation on both PA.f81 branches all failed as required. Lib suite: 620 passed, 0 failed, 1 ignored (618 baseline + 2 new PA.f81 tests; the PA.f77 flake no longer reproduces). Co-Authored-By: Claude Sonnet 5 --- crates/ghostlight/src/table.rs | 68 +++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 14 deletions(-) diff --git a/crates/ghostlight/src/table.rs b/crates/ghostlight/src/table.rs index a704f02..ac28b08 100644 --- a/crates/ghostlight/src/table.rs +++ b/crates/ghostlight/src/table.rs @@ -2255,12 +2255,16 @@ mod tests { /// Rule (PA.f73): `UnresolvedDraft` names the handle that never resolved /// and the kind it was expected to resolve to, not just its site. + /// PA.f79: the handle used to be `missing_place`, which contains the + /// expected-kind word itself — a mutation that replaced the rendered + /// kind with a fixed `"thing"` still passed `text.contains("place")`. + /// The handle here carries no kind word, and the kind is pinned exactly. #[test] fn describe_refusal_names_unresolved_drafts_handle_and_expected_kind() { let fixture = play_fixture(); let mismatch = Mismatch::UnresolvedDraft { site: patch::Site::Operation(3), - referent: patch::DraftHandle::new("missing_place"), + referent: patch::DraftHandle::new("unresolved_target"), expected: RefKind::Entity(crate::EntityKind::Place), }; let text = describe_refusal( @@ -2270,21 +2274,35 @@ mod tests { None, &KernelError::PatchRejected(vec![mismatch]), ); - assert!(text.contains("missing_place"), "{text}"); - assert!(text.contains("place"), "{text}"); + assert!(text.contains("unresolved_target"), "{text}"); + assert!( + text.contains("draft handle `unresolved_target` was never resolved to a place"), + "{text}" + ); } /// Rule (PA.f73): a handle declared twice in one run is named by both /// calls that declared it, not by one ambiguous "the call that declared /// `handle`" — the old generic wording could only ever pick one. + /// PA.f79: with only the two colliding declarations in the batch, a + /// filter that matched every declaration regardless of its handle would + /// still produce "call #0 and call #1" and pass. A third, unrelated + /// declaration pins the filter: it must be named by neither call. #[test] fn describe_refusal_names_both_calls_for_a_duplicate_handle() { let first = serde_json::json!({"handle": "yard", "label": "The Cavity Yard", "container": null}) .to_string(); let second = serde_json::json!({"handle": "yard", "label": "The Second Yard", "container": null}) .to_string(); - let batch = decode_authoring_calls(&[("declare_place", &first), ("declare_place", &second)]) - .expect("both calls decode; duplicate handles are a commit-time, not decode-time, refusal"); + let unrelated = + serde_json::json!({"handle": "gate", "label": "The Rain Gate", "container": null}) + .to_string(); + let batch = decode_authoring_calls(&[ + ("declare_place", &first), + ("declare_place", &second), + ("declare_place", &unrelated), + ]) + .expect("all three calls decode; duplicate handles are a commit-time, not decode-time, refusal"); let fixture = play_fixture(); let text = describe_refusal( &fixture.snapshot, @@ -2297,6 +2315,7 @@ mod tests { ); assert!(text.contains("call #0"), "{text}"); assert!(text.contains("call #1"), "{text}"); + assert!(!text.contains("call #2"), "{text}"); } /// Rule (PA.f73): an `EmptyEvidence` mismatch names the exact call that @@ -2544,6 +2563,33 @@ mod tests { assert!(text.contains("`horn`"), "{text}"); } + /// PA.f81: nothing pinned `describe_refusal_to_actor`'s exact wording for + /// a role-bearing precondition — every prior test used `contains`, so + /// appending a `{:?}` dump of the failed precondition (which would leak + /// internal shape the actor never handed back) survived every one of + /// them. Pin the full string for one role-bearing failure. + #[test] + fn describe_refusal_to_actor_exact_wording_for_a_role_bearing_precondition() { + let entry = affordance_snapshot(vec![patch::Precondition::CanBroadcast { + via: patch::AudienceSpec::Channel(patch::Role("horn".into())), + }]); + let error = KernelError::ActionRejected(vec![ActionMismatch::NoAudience { precondition: 0 }]); + let text = describe_refusal_to_actor(&entry, &error); + assert_eq!(text, "a precondition over role `horn`, which you bound yourself, failed"); + } + + /// PA.f81: the same pin for a roleless precondition failure + /// (`HasStanding`/`NoStanding`). + #[test] + fn describe_refusal_to_actor_exact_wording_for_a_roleless_precondition() { + let entry = affordance_snapshot(vec![patch::Precondition::HasStanding { + grievance: patch::GrievanceKindName("noise".into()), + }]); + let error = KernelError::ActionRejected(vec![ActionMismatch::NoStanding { precondition: 0 }]); + let text = describe_refusal_to_actor(&entry, &error); + assert_eq!(text, "a precondition you bound no role for failed"); + } + /// Rule (PA.f57): every `KernelError` other than `ActionRejected` — /// `MissingApprovals`, a store or journal error — reveals nothing to the /// actor. @@ -2882,16 +2928,10 @@ mod tests { .iter() .find(|affordance| !holder.affordances.contains(&affordance.id)) .expect("the fixture world declares an affordance the holder was not granted"); - let subject_line_start = format!("{} [{}]", holder.label, id_text(holder.id)); - let start = view - .find(&subject_line_start) - .expect("the holder's row is in the view"); - let row = &view[start..]; - let end = row.find(");").map_or(row.len(), |index| index + 1); + let row = subject_row(&view, &holder.label, &id_text(holder.id)); assert!( - !row[..end].contains(&ungranted.entry.kind.0), - "table_view granted the holder an affordance it does not hold: {}", - &row[..end] + !row.contains(&ungranted.entry.kind.0), + "table_view granted the holder an affordance it does not hold: {row}" ); }