[SOLVE]: constraint freedom visualisation #9573
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Merging this PR will degrade performance by 14.19%Summary
Performance Changes
Comparing 
Footnotes
|
Irev-Dev
changed the title
There was a problem hiding this comment.
I added these tests failing before I started working on a fix, now they pass.
| (ObjectId(1), Freedom::Fixed), // line1.start | ||
| (ObjectId(2), Freedom::Fixed), // line1.end | ||
| (ObjectId(4), Freedom::Fixed), // line2.start | ||
| (ObjectId(5), Freedom::Free), // line2.end (currently bug shows Conflict) | ||
| (ObjectId(7), Freedom::Conflict), // line3.start (currently bug shows Free) | ||
| (ObjectId(8), Freedom::Conflict), // line3.end (currently bug shows Free) |
There was a problem hiding this comment.
The kcl for this is
line1 with both ends fixed
line2 with one end fixed
line3 with two conflicting distance constraint
But before the fix, point_freedoms would come back as fixed, fixed, fixed, conflict, free, free.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 3
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Updated should_force_freedom_analysis to only return true if: The cache is empty (analysis hasn't run yet) AND all points are Free (likely default values) If the cache has values (even if all Free), it means analysis has already run, so we don't force another run. This prevents the repeated retries that led to incorrect results. What This Fixes Prevents the infinite loop of retries after each edit Stops forcing analysis when it has already run Reduces the chance of hitting the bug where freedom analysis returns incorrect results The underlying issue (freedom analysis sometimes returning 2 instead of 18 underconstrained) may still occur, but it will happen less often since we're not repeatedly retrying. Test this and see if segments still turn white. If the issue persists, we may need to investigate why kcl-ezpz freedom analysis returns inconsistent results with the same constraint counts.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| expect(freeColor).toBe(UNCONSTRAINED_COLOR) | ||
| expect(fixedColor).toBe(TEXT_COLOR) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Duplicate test files with nearly identical content
Low Severity
Both segments.spec.ts and segments.test.ts are new files with nearly identical test content for deriveSegmentFreedom and getSegmentColor. The .test.ts file has one additional test case (should return null when all points are missing) and slightly different test naming. If the test runner picks up both *.spec.ts and *.test.ts patterns, the same tests will run twice. One of these files should likely be removed.
Additional Locations (1)
| const darkerRgb = `${Math.round(r * 0.7)}, ${Math.round(g * 0.7)}, ${Math.round(b * 0.7)}` | ||
| innerCircle.style.backgroundColor = `rgb(${darkerRgb})` | ||
| innerCircle.style.border = `1px solid rgba(${darkerRgb}, 0.5)` | ||
| return // Hover styles take precedence |
There was a problem hiding this comment.
Point hover color is doubly darkened causing visual inconsistency
Medium Severity
In updatePointColors, when isHovered is true, getSegmentColor already returns the hover color at 70% brightness. Then the code at lines 135-141 darkens it again by 70%, resulting in 49% brightness (0.7 × 0.7 = 0.49). In contrast, updateLineColors and updateArcColors use the color from getSegmentColor directly without additional darkening. This causes hovered points to appear significantly darker than hovered lines and arcs, creating a visual inconsistency. The hover branch either shouldn't darken the color again, or isHovered shouldn't be passed to getSegmentColor if the intent is to darken a base color.
| point_ids.push(arc.end); | ||
| point_ids.push(arc.center); | ||
| } | ||
| } |
There was a problem hiding this comment.
Circle segments not handled in freedom analysis check
Low Severity
The should_force_freedom_analysis function handles Point, Line, and Arc segment types when collecting point IDs, but does not handle Circle segments. The Circle struct has a start field (the center point's ObjectId) that should be included. For sketches containing only circles, point_ids would be empty, causing the function to return false at line 2161 and skip forcing freedom analysis. This could result in circle center points showing incorrect freedom colors until analysis runs for another reason. The TypeScript equivalent in segmentsUtils.ts correctly handles circles.
Ostensibly trying to show colours in sketch mode,
Resolves #9281
but did a fair bit in rust:
On the TS side I revamp the colors a little to give clear prirotiy of what status wins out for colors, it's the following order now:
draft colour
hover colour
select colour
conflict colour
free colour
fixed colour
Also I added the following logic for deriving the freedom of segment bodies
Note
Improves solver output and sketch freedom visualization while making
Freedomnon-null and cached.variables_in_conflictsfrom unsatisfied constraints by scanning all constraints; use it to mark conflicts (replacingunsatisfiedchecks), and pass combined constraints toSolved::from_ezpz_outcome; add robust helpers for extracting IDs and fallback parsing; default missing freedom analysis toFreefor points.point_freedom_cacheandupdate_state_after_exec(outcome, freedom_analysis_ran)to preserve/merge point freedoms when analysis isn’t run; optionally force analysis when cache empty and all points appearFree; makePoint.freedomnon-optional in API and wire freedom flags through execution paths; addfreedom_analysis_tests(feature-gated).segmentsUtilswithderiveSegmentFreedom(segment = Conflict if any point conflicts, else Free if any free, else Fixed) andgetSegmentColorwith precedence: draft > hover > select > conflict > free > fixed; refactor segment rendering to use these and propagatefreedomthrough groups/hover.Written by Cursor Bugbot for commit 39b6b00. This will update automatically on new commits. Configure here.