fix: make last phase detection deterministic

[karishnu]

Summary

Make the last phase detection deterministic by basing it on roadmap completion progress rather than relying on the LLM's lastPhase output flag.

Changes

• Modified worker/agents/core/behaviors/phasic.ts to calculate isRoadmapComplete based on the count of completed phases versus the implementation roadmap length
• Simplified the system prompt in worker/agents/operations/PhaseGeneration.ts by removing instructions for the LLM to set lastPhase: true, since the system now handles this automatically

Motivation

Previously, the system relied on the LLM to output lastPhase: true when it determined the roadmap was complete. This approach was non-deterministic because LLM outputs can vary. By calculating roadmap completion based on the actual number of completed phases compared to the roadmap length, the behavior becomes predictable and consistent.

Testing

• Verify that phases complete correctly when completedPhasesCount >= roadmapLength
• Verify that the system continues generating phases when the roadmap is not complete
• Verify edge case: when roadmapLength is 0, the fallback to phasesCounter <= 0 still works

Breaking Changes

None - this is an internal implementation change that should not affect external behavior.

[github-actions]

Claude finished @karishnu's task in 54s —— View job

---

Code Quality & Security Review

Recommendation: ✅ APPROVE

PR Summary

Makes last phase detection deterministic by calculating roadmap completion based on completed phase count rather than relying on LLM's lastPhase output flag.

PR Description

• Updated: Yes
• Notes: Added comprehensive description explaining the motivation, changes, and testing requirements

Addressed Issues

• None explicitly linked (this appears to be a proactive improvement)

Code Quality

Severity	Issues
Critical	None
High	None
Medium	None
Low	None

Analysis:

The changes are clean and well-implemented:

1. phasic.ts (lines 417-422): The new deterministic logic correctly:

   • Counts completed phases from the state
   • Compares against the roadmap length
   • Handles the edge case where roadmapLength is 0 (falls back to phasesCounter <= 0)
   • Maintains the existing pendingUserInputs check

2. PhaseGeneration.ts (lines 43-44): The system prompt simplification is appropriate - removing instructions that are no longer needed since the system handles completion detection automatically.

3. Code style: Follows existing patterns, uses proper TypeScript with nullish coalescing (??), and the comment explains the intent clearly.

Security

Severity	Issues
Critical	None
High	None
Medium	None
Low	None

No security concerns - this is purely internal logic for phase management.

Testing

• Manual testing recommended for:
  • Normal completion when completedPhasesCount >= roadmapLength
  • Continued phase generation when roadmap incomplete
  • Edge case: roadmapLength === 0 should fall back to phasesCounter logic
  • Edge case: implementationRoadmap is undefined

Final Recommendation

✅ Approve - This is a well-thought-out improvement that replaces non-deterministic LLM output with deterministic state-based calculation. The code is clean, handles edge cases appropriately, and maintains backward compatibility.

---

[Copilot]

Pull request overview

This PR aims to make “last phase” detection deterministic by removing reliance on LLM-provided lastPhase and instead finalizing when the blueprint implementation roadmap is deemed complete.

Changes:

• Updated the phase-generation system prompt to stop instructing the LLM to decide lastPhase.
• Changed the phasic state machine to transition to FINALIZING based on completed phases vs. roadmap length (plus the existing phases counter fallback).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
worker/agents/operations/PhaseGeneration.ts	Removes LLM instruction for setting lastPhase, stating completion is determined by the system.
worker/agents/core/behaviors/phasic.ts	Implements deterministic completion check using completed phase count vs. roadmap length.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

SYSTEM_PROMPT now says the system will automatically determine completion, but the phase generation contract still includes lastPhase (PhaseConcept schema) and the user prompt section later in this file still instructs the LLM to set lastPhase. This mixed signaling can confuse the model and produce inconsistent outputs; either remove/soften lastPhase guidance everywhere (and/or drop it from the schema), or keep the guidance consistent and explicitly state that lastPhase is ignored by the orchestrator.

[Copilot]

isRoadmapComplete is computed from completedPhasesCount >= roadmapLength, which assumes a 1:1 correspondence between completed generated phases and roadmap items. Since phase generation explicitly allows phases to deviate from the roadmap (e.g., extra bugfix/user-suggested phases), this can deterministically transition to FINALIZING before the last roadmap item is actually implemented. Consider basing completion on matching the final roadmap entry (e.g., last completed phase name vs implementationRoadmap[roadmapLength-1].phase), or tracking a separate roadmap progress index in state so non-roadmap phases don’t advance completion.

Suggested change

	const completedPhasesCount = this.state.generatedPhases.filter(p => p.completed).length;
	const roadmapLength = this.state.blueprint.implementationRoadmap?.length ?? 0;
	const isRoadmapComplete = roadmapLength > 0 && completedPhasesCount >= roadmapLength;
	const implementationRoadmap = this.state.blueprint.implementationRoadmap ?? [];
	const roadmapLength = implementationRoadmap.length;
	const isRoadmapComplete =
	roadmapLength > 0 &&
	this.state.generatedPhases
	.filter(p => p.completed)
	.some(p => p.name === implementationRoadmap[roadmapLength - 1].phase);