fix(property-model): forced-output cells can never be planner sources#34
Merged
Conversation
Documents the fixpoint algorithm for identifying cells that can never be sources under the currently active relationships, fixing the bug where single-method relationships could have their forced direction overridden by unrelated cell strength. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Task-by-task plan implementing the fixpoint design from 2026-07-09-planner-forced-outputs-design.md: the planner fixpoint itself, then the public is_forced/forced_cells API. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A relationship's method structure can guarantee a cell is always produced by a method (e.g. the sole output of a single-method relationship), independent of cell strength. The planner previously let strength alone decide source candidacy, so such a cell could be wrongly promoted to a source, orphaning its real input or reporting a spurious Conflict. forced_output_cells computes this set as a fixpoint over active relationships (eliminating a method whose output collides with another relationship's forced cell can force further cells) and excludes it from the strength-ordered source loop. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pure_outputs scans inputs once per output cell, matching the O(K^2) complexity already documented on plan() for the same kind of linear-scan-heavy operation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
UI code binding a form to a Sheet needs to know which fields can never accept user input (they are always overwritten by an active relationship, regardless of priority) so it can disable them. is_forced/forced_cells expose the forced-output set computed by the planner, cached from the last full propagate() the same way is_source/last_plan already are. Removes the temporary #[allow(dead_code)] on Plan::forced_outputs now that it is read outside #[cfg(test)] via Sheet::propagate(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ew term pure_outputs is O(K^2) (inputs.contains scans linearly per output) but had no Complexity bullet. plan()'s aggregate complexity omitted the forced_output_cells term added by the forced-outputs fixpoint. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes property-model planning so structurally forced output cells are never selected as planner “sources” due to strength ordering, and exposes that forced-ness through Sheet for UI consumers.
Changes:
- Add a fixpoint pre-pass in
planner::plan()to compute forced output cells and exclude them from source selection. - Cache forced cells from the last successful
propagate()and expose them viaSheet::is_forced/Sheet::forced_cells. - Add regression + API tests, plus design/implementation docs; update a VS Code task to pass
--pathtotokensave.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| property-model/src/planner.rs | Adds forced_output_cells fixpoint analysis, threads forced_outputs through Plan, and skips forced cells as source candidates. |
| property-model/src/sheet.rs | Caches forced cells from the last propagate() and exposes is_forced / forced_cells APIs (with staleness note for propagate_without_replan). |
| property-model/tests/integration.rs | Adds integration coverage for forced-direction correctness, cascades, conditional activation, and the new Sheet API. |
| docs/superpowers/specs/2026-07-09-planner-forced-outputs-design.md | Documents the forced-output problem, fixpoint approach, and intended semantics. |
| docs/superpowers/plans/2026-07-09-planner-forced-outputs.md | Provides a detailed implementation plan and verification checklist. |
| .vscode/tasks.json | Updates worktree task to pass --path to tokensave branch add. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ed_output_cells is_forced's doc claimed to reflect "currently active relationships" but actually serves the cached result from the last propagate(); reworded to match forced_cells's phrasing. forced_output_cells allocated a HashSet on every fixpoint iteration for every relationship purely for membership checks; replaced with direct lookups against global_forced/own forced set. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CLAUDE.md's documented clippy commands never passed --all-targets, so #[cfg(test)] unit tests, integration tests, and doctests were never linted by the official gate. That's how an unused nested unsafe block and several bool_assert_comparison violations slipped into raw_stack.rs and property-model's PR. Added --all-targets to the clippy commands and fixed everything it surfaces workspace-wide: redundant unsafe blocks, assert_eq!(x, true/false) -> assert!, an unannotated transmute, a redundant import, a needless borrow, and swapped 3.14 test literals for 42.14 to avoid clippy reading them as an approximation of PI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8e76120. Configure here.
forced_output_cells identifies methods that must never run because they'd double-write a cell a different relationship structurally owns, but plan() only used the resulting forced-cell set to exclude sources -- the flood-fill's method-eligibility check never consulted method-level aliveness. A dead method's own pure output can still be undetermined at the moment it's considered, so a higher-strength cell reachable only through that dead method could get flood-filled first, permanently claiming a cell the real relationship was meant to produce and turning an otherwise solvable sheet into a spurious Error::Conflict. forced_output_cells now returns the per-method alive map alongside the forced-cell set, and plan()'s eligibility and pre-claim-feasibility checks both consult it so dead methods are never selected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Error::Conflict.forced_output_cellsinproperty-model/src/planner.rs) that computes, perplan()call, the set of cells no active relationship can ever treat as a source — including cells only transitively forced through a cascade across relationships — and excludes them from the strength-ordered source-selection loop. The existing flood-fill / eligibility logic is unchanged.Sheet::is_forced/Sheet::forced_cellspublic API, so UI code binding a form to aSheetcan disable fields that can never accept user input.docs/superpowers/specs/2026-07-09-planner-forced-outputs-design.md. Implementation plan:docs/superpowers/plans/2026-07-09-planner-forced-outputs.md.Test plan
cargo test --workspace— all greencargo test --doc --workspace— all greencargo clippy --workspace --exclude begin -- -D warnings— cleancargo clippy -p begin --no-default-features -- -D warnings— cleancargo fmt --all— no diffsNote
Medium Risk
Core constraint propagation logic in
property-modelchanges, but behavior is heavily tested; remaining risk is subtle planner edge cases on large graphs.Overview
Fixes a property-model planner bug where high cell strength could treat structurally forced outputs as sources (e.g. single-method
a → b), breaking propagation or yielding spurious conflicts.Adds a
forced_output_cellsfixpoint before strength-ordered source selection: cells that every viable method of some active relationship must write (pure outputs, with cross-relationship method elimination) are skipped as sources.Plancarriesforced_outputs;Sheet::propagate()caches them aslast_forcedand exposesis_forced/forced_cellsfor UI (e.g. disabling read-only fields). Integration and planner unit tests cover single-method, cascade, multi-method choice, and conditional branches.Also documents
--all-targetsfor workspace clippy inCLAUDE.md, passes--pathtotokensave branch addin the worktree VS Code task, adds design/plan docs underdocs/superpowers/, and applies clippy-driven test cleanups (boolean asserts, explicittransmutetypes,eprintln!in a debug proc-macro) without changing runtime behavior.Reviewed by Cursor Bugbot for commit 8e76120. Bugbot is set up for automated code reviews on this repo. Configure here.