feat: add polish mode to the loop engine - #7
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds optional polish mode to loop startup, introduces polish-specific worker and critic prompts, runs phase-aware polish cycles, preserves passed outcomes during failures and recovery, and displays polish phases and verdicts in the dashboard. ChangesPolish loop workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Dashboard
participant Daemon
participant Worker
participant Critic
Client->>Dashboard: Select Polish after pass
Dashboard->>Daemon: Submit loop with polish=true
Daemon->>Worker: Run polish worker prompt
Worker->>Critic: Provide worker output
Critic->>Daemon: Return SHIP or IMPROVE
Daemon->>Daemon: Schedule next polish cycle or complete loop
Possibly related PRs
Comment |
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/prompts.js`:
- Around line 32-45: Update the prompts generated by polishWorkerPrompt and the
corresponding critic prompt to require reading GUIDELINES.md before making or
evaluating changes. Explicitly require polish passes to preserve and revalidate
the documented requirements, preventing the critic from returning SHIP without
checking them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ddaf168-f59e-47ff-9e28-56f2d5330e6c
📒 Files selected for processing (4)
bridge.jspublic/index.htmlsrc/daemon.jssrc/prompts.js
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
src/daemon.js (1)
src/prompts.js (3)
criticPrompt(49-64)loopWorkerPrompt(17-31)parseCriticVerdict(156-172)
🪛 ast-grep (0.44.1)
src/daemon.js
[error] 790-803: React's useState should not be directly called
Context: setImmediate(() => {
const next = readRunningLoop(loop.id);
if (!next) {
return;
}
if (next.cancelRequested) {
completeLoop(next, 'cancelled', 'Cancelled.', 'cancelled');
return;
}
startLoopCycle(next);
})
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🪛 OpenGrep (1.25.0)
src/prompts.js
[ERROR] 186-186: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (4)
src/prompts.js (1)
173-200: LGTM!bridge.js (1)
298-298: LGTM!Also applies to: 360-360
public/index.html (1)
122-127: LGTM!Also applies to: 363-364, 784-787, 1275-1293, 1298-1300, 1777-1777
src/daemon.js (1)
11-16: LGTM!Also applies to: 425-435, 657-657, 668-673, 790-805, 815-863, 879-895, 914-972, 991-1080, 1095-1100, 1127-1127, 1406-1409, 1779-1797, 2005-2046
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88f2fc5b49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe0005081e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/daemon.js (1)
1032-1055: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the incomplete-polish summary for every critic failure.
invalidPolishCriticonly matchescritic_invalid_verdict. Timeout, non-zero-exit, or other critic failures after an earlier passing cycle still return the generic “received a valid verdict” summary instead of warning that polish ended incompletely. Preserve the passed status if intended, but report the actual incomplete polish outcome.Proposed fix
- const invalidPolishCritic = polishing && reason === 'critic_invalid_verdict'; + const incompletePolishCritic = polishing && Boolean(reason); ... - passed && invalidPolishCritic + passed && incompletePolishCritic🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/daemon.js` around lines 1032 - 1055, Update the critic-failure handling around invalidPolishCritic and completeLoop so any critic failure during polishing after a passed cycle uses incompletePolishSummary(), not only failures with reason critic_invalid_verdict. Preserve the existing passed/failed status behavior and generic summaries for non-polish or not-yet-passed cycles.src/prompts.js (1)
33-46: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winTreat critic-supplied improvements as untrusted prompt data.
parsePolishVerdict()forwardsVERDICT: IMPROVEtext intoprevious.fixes, andpolishWorkerPrompt()appends it verbatim to the next worker prompt. Wrap it in explicit delimiters and tell the worker to ignore any embedded instructions.src/prompts.js:32-46🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/prompts.js` around lines 33 - 46, Update polishWorkerPrompt() so the critic-supplied improvement from previous.fixes is enclosed in explicit delimiters and accompanied by an instruction to treat the delimited content as untrusted data, ignoring any embedded instructions. Preserve the existing feedback inclusion behavior while preventing the improvement text from being interpreted as prompt instructions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/daemon.js`:
- Around line 1032-1055: Update the critic-failure handling around
invalidPolishCritic and completeLoop so any critic failure during polishing
after a passed cycle uses incompletePolishSummary(), not only failures with
reason critic_invalid_verdict. Preserve the existing passed/failed status
behavior and generic summaries for non-polish or not-yet-passed cycles.
In `@src/prompts.js`:
- Around line 33-46: Update polishWorkerPrompt() so the critic-supplied
improvement from previous.fixes is enclosed in explicit delimiters and
accompanied by an instruction to treat the delimited content as untrusted data,
ignoring any embedded instructions. Preserve the existing feedback inclusion
behavior while preventing the improvement text from being interpreted as prompt
instructions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8cdb2cd3-32fd-43a5-b3f2-b8c3dc660d18
📒 Files selected for processing (2)
src/daemon.jssrc/prompts.js
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
src/prompts.js (1)
src/daemon.js (2)
polishWorkerPrompt(858-858)polishCriticPrompt(968-968)
🔇 Additional comments (1)
src/daemon.js (1)
1076-1095: 🗄️ Data Integrity & IntegrationVerify the terminal
IMPROVEresult contract.When the polish budget is exhausted, an
IMPROVEverdict is completed with top-level statuspassedand summaryPassed after ..., although onlySHIPrepresents a successful polish verdict. Ifpassedintentionally means the baseline loop passed, make the result explicitly state that polish remains unfinished; otherwise, downstream consumers may interpret an unresolved improvement as shipped.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a2aa22e18
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds a “polish mode” that continues running additional post-pass cycles to apply one high-impact quality improvement at a time, integrating the behavior across the loop engine, dashboard UI, and the MCP bridge.
Changes:
- Introduces dedicated worker/critic prompts and verdict parsing for polish cycles (IMPROVE/SHIP).
- Updates the daemon loop state machine to optionally continue after PASS and to handle partial/failed polish cycles without losing the earlier PASS.
- Extends the dashboard and MCP bridge to allow enabling polish mode when starting a loop.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/prompts.js | Adds polish worker/critic prompts and a new verdict parser for SHIP/IMPROVE. |
| src/daemon.js | Implements polish-cycle control flow, event rendering updates, and API support for a polish flag. |
| public/index.html | Adds a “Polish after pass” checkbox and displays polish cycle tags in the running loop UI. |
| bridge.js | Extends the MCP start_loop tool schema and request payload with the polish flag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/daemon.js (1)
2028-2046: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the unapplied-polish outcome during restart recovery.
A polish critic persists
status: 'improve'beforestartNextLoopCycle()runs. If the daemon restarts in that gap, this predicate misses it and recovery reports “Passed before the daemon restarted,” even though the requested improvement was never applied. Detect a final polishimprovecycle and use a distinct unapplied-improvement summary.Proposed fix
- const interruptedPolishCycle = isLoop && (Array.isArray(task.cycles) ? task.cycles : []).some((cycle) => ( + const interruptedPolishCycle = isLoop && (Array.isArray(task.cycles) ? task.cycles : []).some((cycle) => ( cycle && cycle.status === 'running' && cycle.phase === 'polish' )); + const finalPolishImprovement = isLoop + && task.cycles?.at(-1)?.phase === 'polish' + && task.cycles.at(-1).status === 'improve'; ... ? interruptedPolishCycle ? incompletePolishSummary() + : finalPolishImprovement + ? 'The final polish improvement was not applied before the daemon restarted; the working tree may not match the last validated state.' : 'Passed before the daemon restarted.'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/daemon.js` around lines 2028 - 2046, Update the restart-recovery logic around passed, interruptedPolishCycle, and summary to detect a final polish cycle whose status is “improve” in addition to an interrupted running polish cycle. Preserve the existing incomplete-polish summary for interrupted polish work, but use the distinct unapplied-improvement summary for a final improve cycle so it is not reported as passed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/daemon.js`:
- Around line 2028-2046: Update the restart-recovery logic around passed,
interruptedPolishCycle, and summary to detect a final polish cycle whose status
is “improve” in addition to an interrupted running polish cycle. Preserve the
existing incomplete-polish summary for interrupted polish work, but use the
distinct unapplied-improvement summary for a final improve cycle so it is not
reported as passed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0752288c-1097-44b6-9d72-d9741dce3642
📒 Files selected for processing (2)
src/daemon.jssrc/prompts.js
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
src/prompts.js (1)
src/daemon.js (1)
polishWorkerPrompt(858-858)
🔇 Additional comments (2)
src/prompts.js (1)
32-48: LGTM!Also applies to: 66-84, 177-204
src/daemon.js (1)
11-16: LGTM!Also applies to: 425-440, 661-677, 794-810, 819-835, 856-867, 883-887, 899-914, 924-952, 968-986, 1005-1009, 1021-1150, 1429-1432, 1802-1820
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 834b9c6429
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Adds polish mode across the loop engine, dashboard, and MCP bridge.