Skip to content

feat(setbuilder): replace_slot agent tool — atomic track swap#472

Merged
thewrz merged 2 commits into
mainfrom
feat/issue-467
Jun 18, 2026
Merged

feat(setbuilder): replace_slot agent tool — atomic track swap#472
thewrz merged 2 commits into
mainfrom
feat/issue-467

Conversation

@thewrz

@thewrz thewrz commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Why

Part of #442 (Family 2 — safe mutations). Swapping the track in a slot today means remove + insert: two ops with a transient invalid state and shifting positions. replace_slot does it atomically — same position, new track.

What

Adds a single mutation tool replace_slot to the WrzDJSet pass-2 agent (server/app/services/setbuilder/pass2_agent.py):

  • _tool_replace_slot(db, set_obj, payload) resolves the slot via _slot_or_error, refuses locked slots (AgentToolError, mirroring _tool_remove_slot), resolves the replacement via _pool_track_or_error, then writes only the slot's track_id — keeping its position. db.flush(); returns ({"slot_id", "pool_track_id"}, {slot.position}).
  • Wired into the 4 existing additive spots, mirroring the sibling mutation tools: MUTATION_TOOLS, the handlers allowlist in apply_tool_call, the _agent_tools() schema (_tool("replace_slot", {"slot_id": "integer", "pool_track_id": "integer"}) — auto-adds rationale), and a one-sentence replace_slot clause in _tool_display_summary.

Slot ↔ pool-track id derivation (design decision)

The new track_id is derived with _pass1_track_meta(track).slot_track_id, the exact helper the insert tools route through. It resolves to track.track_id or f"pool:{track.id}" (pass1_deterministic.py), identical to what _insert_track_at builds inline — so the namespacing rule stays in one place rather than being re-derived.

Security invariants (from #442)

  • Owner-scoped: both slot and pool track resolved against set_obj.id.
  • Respects locked: a locked slot cannot be replaced.
  • Requires a rationale (enforced by MUTATION_TOOLS).
  • Dispatched only through apply_tool_call's closed allowlist.
  • Writes only the slot's track_id; never the requests table (regression test asserts unchanged).
  • No new REST endpoints, no DB migrations, no openapi/type regen.

Testing

  • Unit tests in server/tests/test_setbuilder_pass2.py: replace happy path (position preserved, other slot untouched), locked rejection, foreign pool track, foreign slot, missing-rationale, requests-untouched regression, display-summary.
  • Backend ruff check / ruff format --check / bandit: clean
  • Backend pytest: 2939 passed, coverage 87.92% (gate 85%)
  • CI green

🤖 Co-authored by Claude Opus 4.8. Closes #467.

Summary by CodeRabbit

  • New Features
    • Added track replacement operation to swap tracks in set slots while preserving their positions
    • AI agent now integrates track replacement support with built-in validation and error handling for various edge cases

Add a single Family-2 mutation tool `replace_slot` to the WrzDJSet pass-2
agent: swap the track in a slot in place, keeping its position, instead of
remove+insert (which passes through a transient invalid state and shifts
positions).

`_tool_replace_slot` resolves the slot via `_slot_or_error`, refuses locked
slots (mirroring `_tool_remove_slot`), resolves the replacement via
`_pool_track_or_error`, and writes ONLY the slot's `track_id` using the same
namespaced id derivation the insert tools use
(`_pass1_track_meta(track).slot_track_id`). It never touches the `requests`
table. Owner-scoped (both slot and pool track resolved against `set_obj.id`),
rationale-required via `MUTATION_TOOLS`, and dispatched only through
`apply_tool_call`'s closed allowlist.

Closes #467

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@thewrz, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 1 hour, 44 minutes, and 26 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e851ed53-8314-46b0-91dd-9436c2f64756

📥 Commits

Reviewing files that changed from the base of the PR and between 5f4a37f and 5e06dd7.

📒 Files selected for processing (2)
  • server/app/services/setbuilder/pass2_agent.py
  • server/tests/test_setbuilder_pass2.py
📝 Walkthrough

Walkthrough

Adds replace_slot as a new mutation tool in the pass-2 setbuilder agent. The implementation validates the target slot and pool track, enforces the locked-slot constraint, updates the slot's track_id in place, and returns the affected position. The display summary, tool routing, and agent schema registration are all extended accordingly. Comprehensive tests cover the happy path, four error conditions, and a Request table regression guard.

Changes

replace_slot Mutation Tool

Layer / File(s) Summary
Implementation and wiring
server/app/services/setbuilder/pass2_agent.py
Adds replace_slot to MUTATION_TOOLS, routes it in apply_tool_call, implements _tool_replace_slot (locked-slot guard, foreign-entity validation, in-place track_id update, DB flush), extends _tool_display_summary with a "Replaced …" branch, and registers the tool schema in _agent_tools.
Tests
server/tests/test_setbuilder_pass2.py
Adds _pool_track_by_track_id helper; tests display summary rendering, happy-path slot swap (result payload, affected positions, other slots unchanged), locked-slot rejection, foreign pool track rejection, foreign slot rejection, missing-rationale rejection via chat_with_agent, and a regression test asserting Request rows are untouched after the tool runs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

  • #467 — This PR directly implements the replace_slot agent tool described in that issue, satisfying all stated acceptance criteria.

Possibly related PRs

  • wrzonance/WrzDJ#460: Adds another new agent tool to pass2_agent.py using the same wiring pattern — apply_tool_call handler registration, _agent_tools schema entry, and _tool_display_summary extension.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(setbuilder): replace_slot agent tool — atomic track swap' directly and clearly describes the main change: adding a new replace_slot agent tool for atomic track swapping.
Linked Issues check ✅ Passed The PR fully implements all backend requirements from issue #467: _tool_replace_slot function with slot/pool track resolution, locked slot rejection, track id derivation, position preservation, and wiring into MUTATION_TOOLS, apply_tool_call handlers, _agent_tools schema, and display summary.
Out of Scope Changes check ✅ Passed All changes are strictly within scope: only implementation of replace_slot mutation tool in pass2_agent.py and comprehensive test coverage in test_setbuilder_pass2.py with no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-467

Comment @coderabbitai help to get the list of available commands and usage tips.

# Conflicts:
#	server/app/services/setbuilder/pass2_agent.py
#	server/tests/test_setbuilder_pass2.py
@thewrz thewrz merged commit 5ef9cf9 into main Jun 18, 2026
1 check was pending
@thewrz thewrz deleted the feat/issue-467 branch June 18, 2026 16:54
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.

feat(setbuilder): replace_slot agent tool — atomic track swap

1 participant