T-208/T-211: random_constrained_tree() and impose_constraint() fixes#229
Merged
ms609 merged 6 commits intocpp-searchfrom Mar 25, 2026
Merged
T-208/T-211: random_constrained_tree() and impose_constraint() fixes#229ms609 merged 6 commits intocpp-searchfrom
ms609 merged 6 commits intocpp-searchfrom
Conversation
…ilder Build random binary tree that satisfies topological constraints by: 1. Ordering constraint splits smallest-to-largest 2. Assigning each tip to its tightest enclosing split 3. Bottom-up: randomly resolve each split's children into binary subtree 4. Wire root-level items (unconstrained tips + top-level splits) Replaces Wagner fallback (T-214 workaround) for RANDOM_TREE strategy, restoring uniform random topology sampling diversity under constraints. Tests: 916 constraint + 24 random-constrained + 373 simplify/driven pass.
Three issues in impose_one_pass / impose_constraint: 1. Bail-out threshold n_tip/4 was too aggressive. For n_tip=5 the threshold was 1, so any split requiring 2+ moves caused an immediate bail-out before making any repairs. Raised to n_tip. 2. impose_one_pass returned 0 both for 'no violations' and 'bailed out', so impose_constraint couldn't distinguish the two. Now returns -1 on bail-out, allowing the caller to know the repair may be incomplete. 3. Documented the root-child limitation: spr_clip() doesn't fully handle root children, so impose_one_pass skips them. The fuse path's post-repair verification guard (T-214) catches these cases. Tests: 940 constraint + 308 simplify/driven/sector pass.
spr_clip() can't detach root children (root is its own parent, so the bypass logic fails). All search callers skip root children, but impose_constraint needs them for constraint repair. New topology_spr() helper in ts_constraint.cpp handles the root-child case by absorbing the sibling into root and repurposing it as the insertion node. No changes to spr_clip or any search caller. Also removes the build_postorder call that was missing between individual moves within impose_one_pass — each topology_spr is now followed by a postorder rebuild so edge enumeration stays valid. Tests: 942 constraint (90 impose-constraint, incl 2 new root-child tests) + 308 simplify/driven/sector pass.
d523c99 to
8cd3ac7
Compare
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
Implements
random_constrained_tree()and fixes three bugs inimpose_constraint().New:
random_constrained_tree()(T-208)Constraint-aware random topology builder. Orders constraint splits smallest→largest,
assigns tips to tightest enclosing split, randomly resolves each split's children
into a binary subtree, then wires root level. Replaces the Wagner tree fallback
that T-214 introduced as a workaround.
Fix:
impose_constraint()bugs (T-211)Three interconnected bugs prevented constraint repair from working correctly:
n_tip/4too aggressiven_tipspr_clip()can't detach root childrentopology_spr()helper (65 lines)Testing
Agent A (Claude Sonnet 4).