feat(setbuilder): set_curve_point/remove_curve_point agent tools (#468)#470
Conversation
Add two Family-2 mutation tools to the WrzDJSet pass-2 agent for precise energy-curve editing: place or remove an individual energy point at a time offset. Both manage standalone (non-window) SetCurvePoint rows and never disturb add_slow_window's paired vibe-window rows. - curve.py: upsert_curve_point / remove_curve_point / get_standalone_point helpers, all filtered to non-window points (both window flags False). - pass2_agent.py: _tool_set_curve_point (upsert by position_sec, energy 0-10) and _tool_remove_curve_point (remove by position_sec; AgentToolError if absent). Wired into MUTATION_TOOLS, the apply_tool_call allowlist, the _agent_tools() schema list, and the display-summary chain. Owner-scoped, rationale-required, allowlist-dispatched. Writes only to set_curve_points; a regression test asserts the requests table is unchanged. Closes #468 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 1 hour, 34 minutes, and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds ChangesStandalone curve point tools
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 unit tests (beta)
Comment |
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 `@server/app/services/setbuilder/pass2_agent.py`:
- Around line 1044-1047: The `label` field in the `set_curve_point` tool schema
definition is currently marked as required by the `_tool()` function, but the
actual implementation in `_tool_set_curve_point` uses `payload.get("label")`
indicating it should be optional. Modify the schema definition for the
`set_curve_point` tool to mark the `label` field as optional so it matches the
actual implementation behavior and PR contract, rather than requiring it like
the `position_sec` and `energy` fields.
🪄 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: 92a6bce0-6a2c-4e43-8f7b-c01d3533bfc8
📒 Files selected for processing (3)
server/app/services/setbuilder/curve.pyserver/app/services/setbuilder/pass2_agent.pyserver/tests/test_setbuilder_pass2.py
CodeRabbit (PR #470): the _tool() helper marked every listed field required, so label was wrongly required in the LLM-facing set_curve_point schema even though _tool_set_curve_point reads it via payload.get("label") and the contract says it is optional. Extend _tool() with an optional_fields kwarg (in schema properties but not in required); set_curve_point now passes label there. rationale stays required for all mutation tools. Pinned with a schema test asserting label is optional while position_sec/energy/rationale remain required. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # server/app/services/setbuilder/pass2_agent.py # server/tests/test_setbuilder_pass2.py
Why
Part of #442 (Family 2 — safe mutations). Beyond
add_slow_window/set_peak_at, the WrzDJSet pass-2 agent needs precise curve editing: place or remove an individual energy point at a time offset. These two tools manage standaloneSetCurvePointrows.What
Two new mutation tools on the pass-2 agent:
set_curve_point— upsert a non-window energy point atposition_sec(energy validated 0–10, optionallabel).remove_curve_point— remove a non-window point atposition_sec.New service helpers in
curve.py(upsert_curve_point,remove_curve_point,get_standalone_point) keeppass2_agent.pythin and match how the curve domain already lives there. Both helpers filter to non-window points (both window flagsFalse), soadd_slow_window's paired vibe-window rows are never touched.Wiring mirrors the existing mutation tools: both registered in
MUTATION_TOOLS, theapply_tool_callallowlist, the_agent_tools()schema list (via_tool(...), which auto-adds the requiredrationale), and the display-summary chain. The frontendToolCardrenders the new tools through its existing genericdisplay_summarypath — no frontend change needed.Design decisions
position_sec(notpoint_id). The agent reasons about the curve in time offsets and never sees DB row ids, so keying both tools byposition_secis symmetric and matches the upsert match rule.is_slow_window_start == False AND is_slow_window_end == False) at the exact sameposition_sec: sameposition_sec→ updateenergy+label; otherwise insert. A standalone point may share aposition_secwith a window boundary without colliding — the standalone filter makes window rows invisible to the upsert/remove queries.AgentToolError.remove_curve_pointon a missing (or window-only)position_secraises rather than silently no-opping, giving the agent a clear signal. This is also what protects window rows: removing at a window boundary errors instead of deleting the paired row.Security invariants (#442)
SetCurvePointquery filtersset_id == set_obj.id.False).rationale(enforced byMUTATION_TOOLS).apply_tool_call's closed allowlist.set_curve_points; a regression test asserts therequeststable is unchanged.Testing
server/tests/test_setbuilder_pass2.py): insert, upsert-update, remove, slow-window-rows-untouched, remove-at-window-boundary-errors, energy validation, missing-rationale, missing-point, requests-untouched regression, display-summary readabilityruff check/ruff format --checkcleanbandit -r app -c pyproject.tomlcleanpytestgreen: 2943 passed, coverage 87.89% (gate 85%)🤖 Co-authored by Claude Opus 4.8. Closes #468.
Summary by CodeRabbit
New Features
Tests