Skip to content

fix: make solveUntilPhase always terminate on terminal pipeline states - #270

Open
Furox-Art wants to merge 1 commit into
tscircuit:mainfrom
Furox-Art:fix/solve-until-phase-terminal
Open

Furox-Art wants to merge 1 commit into
tscircuit:mainfrom
Furox-Art:fix/solve-until-phase-terminal

Conversation

@Furox-Art

Copy link
Copy Markdown

/claim #267

Problem

solveUntilPhase(phase) looped with a single stopping condition — "current phase name == target". Two terminal states break that condition and hang the event loop on no-op steps (BaseSolver.step() early-returns once solved/failed):

  1. Mid-pipeline failure (the issue's actual scenario): a sub-solver marks failed, the phase never advances → infinite loop
  2. Exhausted pipeline: getCurrentPhase() returns "none", which matches no target → an early-phase request after completion also hung, and an unknown phase name fell into the same hang

Fix

const targetStepIndex = this.pipelineDef.findIndex((stepDef) => stepDef.solverName === phase)
if (targetStepIndex === -1) throw new Error(`solveUntilPhase: unknown phase "..." Known phases: ...`)
while (!this.solved && !this.failed && this.currentPipelineStepIndex < targetStepIndex) this.step()
  • Same terminal-state guard the issue asks for, plus direct pipeline indexing so an exhausted pipeline (currentPipelineStepIndex past the end) terminates for any target
  • Fail-fast on unknown phase names with the list of valid phases — a typo used to silently hang; now it points at the fix. (Alternative considered: silently running the full pipeline, which hides the bug.)

Tests (4 new, snapshot-free)

Scenario main this PR
Mid-pipeline failure, unreachable target hangs forever exits after the failing step (failed=true, 2 step calls)
Solved pipeline, early phase requested hangs forever 0 step calls
Unknown phase name hangs forever throws with known-phase list
Normal use: stop just before target, then solve() works works (unchanged semantics — verified against xiao-stm32-usb and both LayoutPipelineSolver callers)

Relation to #269

PR #269 (opened same day) adds the terminal-state guard and two already-terminal-state tests. This PR covers additionally the mid-pipeline failure scenario (the issue's actual case), the exhausted-pipeline case, and fail-fast on typos — the two PRs do not conflict textually but this one is a superset of the fix. Happy to consolidate if maintainers prefer.

solveUntilPhase looped forever when the pipeline failed or completed before
reaching the target phase, and hung on unknown phase names. It now exits on
solved/failed, indexes the pipeline directly so an exhausted pipeline is
handled, and throws with the list of known phases for a bad name.

Closes tscircuit#267
@vercel

vercel Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
matchpack Ready Ready Preview Sep 18, 2026 1:52pm UTC

Request Review

@Furox-Art

Copy link
Copy Markdown
Author

@mohan-bee gentle ping when you get a chance 🙏

This one's small and time-sensitive-ish: #267 has three competing fix PRs now (#269, #270, #272), and ours carries the regression test for both terminal-state failure modes plus the /claim #267 registration. Happy to rebase on whatever shape you prefer if another PR gets picked instead.

@Furox-Art

Copy link
Copy Markdown
Author

Fresh local verification of the regression suite (2026-09-20, bun 1.4.2, this branch):

bun test tests/LayoutPipelineSolver/solveUntilPhaseTermination.test.ts

 4 pass
 0 fail
 12 expect() calls
Ran 4 tests across 1 file. [2.88s]

All four scenarios pass: normal stop-just-before-phase, mid-pipeline failure, exhausted pipeline, and the typo fail-fast guard.

This branch was successfully deployed

1 active deployment
Preview — b8a63401 Deployed Sep 18, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant