feat(setbuilder): autobuild + fill_to_duration agent tools (#491)#523
Conversation
…tools (#491) Family 3 destructive structural tools. Records the key finding that #493/#494's global undo already closes #491's undo gate, so the original Task 0 (per-tool 'Undo this rebuild' button) is replaced by: rely on global undo + a discoverability hint + a snapshot round-trip identity test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ty (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…target (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nch (#491) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
… cards (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… test (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lots (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds two destructive setbuilder agent tools, Changesautobuild + fill_to_duration agent tools
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/app/services/setbuilder/agent_tools_structural.py (1)
38-40: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winLog autobuild outcome metrics for destructive-turn observability.
The dispatcher logs invocation, but this tool doesn’t log result state (
slot_count,iterations). Add one lazy-formatted outcome log here for easier postmortems/debugging.As per coding guidelines, agent implementation files should log agent actions/state changes, and based on learnings server logs should use lazy `%s` formatting.Proposed refactor
def _tool_autobuild( db: Session, set_obj: Set, payload: dict[str, Any] ) -> tuple[dict[str, Any], set[int]]: @@ result = build_set(db, set_obj, commit=False) affected = {slot.position for slot in result.slots} + logger.info( + "setbuilder autobuild: set %s rebuilt (%s slots, %s refinement passes)", + set_obj.id, + result.slot_count, + result.iterations, + ) return {"slot_count": result.slot_count, "iterations": result.iterations}, affected🤖 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 `@server/app/services/setbuilder/agent_tools_structural.py` around lines 38 - 40, Add outcome logging after the build_set function call to capture the autobuild metrics for observability. After the result assignment (the line calling build_set with commit=False), add a logger.info statement that logs the slot_count and iterations values from the result object using lazy %s string formatting. This should provide visibility into the tool's execution state for debugging and postmortem analysis.Sources: Coding guidelines, Learnings
🤖 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 `@server/app/services/setbuilder/agent_tools_structural.py`:
- Around line 57-59: The presence check for target_duration_sec in the condition
uses `if not target:` which treats the value 0 as missing and raises an
AgentToolError, but 0 might be a valid target duration value. Replace the falsy
check `if not target:` with an explicit null check `if target is None:` so that
a zero value is properly handled as a valid assigned duration rather than
triggering the error.
---
Nitpick comments:
In `@server/app/services/setbuilder/agent_tools_structural.py`:
- Around line 38-40: Add outcome logging after the build_set function call to
capture the autobuild metrics for observability. After the result assignment
(the line calling build_set with commit=False), add a logger.info statement that
logs the slot_count and iterations values from the result object using lazy %s
string formatting. This should provide visibility into the tool's execution
state for debugging and postmortem analysis.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2be846e2-c72e-497a-b52c-79ba749fa2a2
📒 Files selected for processing (13)
dashboard/app/(dj)/setbuilder/components/ChatPanelBody.tsxdashboard/app/(dj)/setbuilder/components/__tests__/ChatPanelBody.test.tsxdashboard/app/(dj)/setbuilder/setbuilder.module.cssdocs/superpowers/plans/2026-06-22-setbuilder-autobuild-fill.mddocs/superpowers/specs/2026-06-21-setbuilder-autobuild-fill-design.mdserver/app/services/setbuilder/agent_common.pyserver/app/services/setbuilder/agent_display.pyserver/app/services/setbuilder/agent_tool_specs.pyserver/app/services/setbuilder/agent_tools_structural.pyserver/app/services/setbuilder/pass1_deterministic.pyserver/app/services/setbuilder/pass2_agent.pyserver/tests/test_setbuilder_pass1.pyserver/tests/test_setbuilder_structural.py
…tion (#491) CodeRabbit: 'if not target' rejected a legitimate 0 target as missing. Use an explicit None check so a 0 target is a no-op (loop sees it as already met) rather than an error. Pinned with a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CodeRabbit nitpick: mirror fill_to_duration by logging autobuild's result (slot_count + iterations) on this destructive turn. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
CodeRabbit body nitpick addressed (autobuild observability): |
Why
The WrzDJSet chat agent could sense the set and make granular edits, but had no way to (re)build it wholesale. This closes Family 3 of epic #442 with the two destructive structural tools. The family was gated on undo UX — now resolved by the global-undo stack from #493/#494.
What
autobuild— regenerate the entire set order from the pool + energy curve via the deterministic pass-1 builder (already honors locked slots and saved pairings). Replaces the hand-arranged order wholesale.fill_to_duration— append unused pool tracks (deterministic pool order) until the set reaches itstarget_duration_sec; bounded by a per-turn hard cap (MAX_FILL_INSERTS) and never moves locked slots.build_setgainedcommit: bool = True; the agent path calls it withcommit=Falseso a multi-tool chat turn stays atomic (the turn owns commit/rollback). The one REST caller keeps the default → unchanged.MUTATION_TOOLS, so the existing global-undo stack (feat(setbuilder): bridge agent mutations into the document undo stack (gates #491 + Family 4) #493/feat(setbuilder): make agent chat mutations undoable #494) snapshots the whole document before the turn and makes them revertible for free (⌘Z / Undo). The frontend adds only a discoverability hint on destructive tool cards.Design + plan:
docs/superpowers/specs/2026-06-21-setbuilder-autobuild-fill-design.md,docs/superpowers/plans/2026-06-22-setbuilder-autobuild-fill.md.Testing
tsc --noEmitcleanrequeststable left untouched🤖 Co-authored by Claude Opus 4.8 (1M context). Closes #491.
Summary by CodeRabbit