WrzDJSet two-pass builder and agent#424
Merged
Merged
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a two-pass set-building system to WrzDJSet. Pass 1 ( ChangesTwo-pass Set Builder
Sequence Diagram(s)sequenceDiagram
actor User
participant Page as SetBuilder Page
participant API_Client as ApiClient
participant Server as FastAPI /setbuilder
participant P1 as pass1_deterministic
participant P2 as pass2_agent
participant LLM as Gateway
rect rgba(59, 130, 246, 0.5)
Note over User,P1: Recompute (Pass 1)
User->>Page: click Recompute, confirm
Page->>API_Client: buildSet(setId, confirmed=true)
API_Client->>Server: POST /sets/{set_id}/build
Server->>P1: build_set(db, set_obj)
P1->>P1: greedy fill + swap refinement
P1-->>Server: BuildResult
Server-->>API_Client: BuildSetResponse
API_Client-->>Page: slot_count, iterations
Page->>Page: setRefreshToken(n+1)
end
rect rgba(16, 185, 129, 0.5)
Note over User,LLM: Agent Chat (Pass 2)
User->>Page: open ChatSidebar
Page->>API_Client: critiqueSet(setId)
API_Client->>Server: POST /sets/{set_id}/critique
Server->>P2: critique_set(db, actor, set_obj)
P2->>LLM: dispatch(critique_tool)
LLM-->>P2: structured payload
P2-->>Server: SetCritique
Server-->>API_Client: SetCritiqueOut
API_Client-->>Page: renders CritiqueCard
User->>Page: sends chat message
Page->>API_Client: chatWithSetAgent(setId, payload)
API_Client->>Server: POST /sets/{set_id}/agent/chat
Server->>P2: chat_with_agent(db, actor, set_obj, message)
P2->>LLM: dispatch(message, agent_tools)
LLM-->>P2: tool_calls
P2->>P2: apply_tool_call mutations
P2->>P1: recompute_transition_scores
P1-->>P2: updated scores
P2-->>Server: AgentChatResult
Server-->>API_Client: AgentChatOut
API_Client-->>Page: renders tools + scores
Page->>Page: setRefreshToken(n+1)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
server/app/services/setbuilder/pass2_agent.py (1)
171-181: 💤 Low valueMinor: redundant
_ordered_slotsquery.
slotsis fetched at line 171, then fetched again at line 179. Since no mutations occur between these lines, the second query is redundant.Suggested fix
slots = _ordered_slots(db, set_obj.id) transition_scores: list[TransitionScore] = [] if affected: affected_with_neighbors = _with_neighbors(affected) transition_scores = recompute_transition_scores(db, set_obj, slots, affected_with_neighbors) return AgentChatResult( message=response.text, tool_calls=applied, - slots=_ordered_slots(db, set_obj.id), + slots=slots, affected_transition_scores=transition_scores, )🤖 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/pass2_agent.py` around lines 171 - 181, The function calls _ordered_slots(db, set_obj.id) twice unnecessarily: once at the beginning of the function to assign to the slots variable, and again when constructing the AgentChatResult object. Since no mutations occur between these two calls, replace the second _ordered_slots(db, set_obj.id) call in the AgentChatResult constructor with the slots variable that was already fetched, eliminating the redundant database query.
🤖 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 `@dashboard/app/`(dj)/setbuilder/[setId]/page.tsx:
- Line 172: The toast notification div at line 172 lacks ARIA attributes
required for screen reader accessibility. Add `role="status"` and
`aria-live="polite"` attributes to the div element that renders the toast (the
one with className styles.builderToast) to ensure screen reader users are
properly notified when the toast notification appears or updates.
- Around line 140-170: The confirmBackdrop element has an onClick handler that
always closes the modal by calling setConfirmBuild(false), but during a build
operation (when building is true) the Cancel button is disabled to prevent
dismissal. To fix the UX inconsistency, make the backdrop onClick handler
conditional so it only calls setConfirmBuild(false) when building is false,
preventing users from dismissing the dialog while the build operation is in
progress. This aligns the backdrop behavior with the disabled state of the
Cancel button.
In `@dashboard/app/`(dj)/setbuilder/components/ChatSidebar.tsx:
- Around line 133-138: The `history` variable in the useMemo hook does not cap
the number of entries before sending to the server, but the
`AgentChatIn.history` schema is limited to 30 items maximum. This causes a 422
error when posting to `/agent/chat` if the chat history exceeds 30 items. Fix
this by limiting the filtered entries to the first 30 items within the useMemo
hook, ensuring the mapped history array never exceeds the server's constraint
before it is sent in the request.
In `@server/app/services/setbuilder/pass1_deterministic.py`:
- Around line 114-116: The dictionary comprehension for tracks_by_id calls
_track_meta(t) twice per iteration—once to extract the slot_track_id key and
once for the value—resulting in unnecessary object allocation. Restructure the
dictionary comprehension to call _track_meta(t) only once per track by storing
its result in a variable, then use that single result to both extract the key
and populate the value in the dictionary.
In `@server/openapi.json`:
- Around line 1545-1555: The BuildSetRequest schema makes the confirmed field
optional with a default of false, which allows clients to submit requests
without explicit confirmation. Change the confirmed field to be required (remove
the default) and constrain it to only accept true by adding const: true to its
definition in BuildSetRequest. Additionally, add documentation of the 400
response on the /build endpoint to model the rejection path when confirmation is
not properly provided, ensuring the generated API contract fully enforces the
explicit confirmation requirement.
---
Nitpick comments:
In `@server/app/services/setbuilder/pass2_agent.py`:
- Around line 171-181: The function calls _ordered_slots(db, set_obj.id) twice
unnecessarily: once at the beginning of the function to assign to the slots
variable, and again when constructing the AgentChatResult object. Since no
mutations occur between these two calls, replace the second _ordered_slots(db,
set_obj.id) call in the AgentChatResult constructor with the slots variable that
was already fetched, eliminating the redundant database query.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 84cdfd53-bd17-4883-94ff-5e140462f07b
📒 Files selected for processing (20)
dashboard/app/(dj)/setbuilder/[setId]/page.tsxdashboard/app/(dj)/setbuilder/__tests__/BuilderWorkspace.test.tsxdashboard/app/(dj)/setbuilder/__tests__/curveMath.test.tsdashboard/app/(dj)/setbuilder/components/BuilderWorkspace.tsxdashboard/app/(dj)/setbuilder/components/ChatSidebar.tsxdashboard/app/(dj)/setbuilder/components/__tests__/ChatSidebar.test.tsxdashboard/app/(dj)/setbuilder/components/types.tsdashboard/app/(dj)/setbuilder/setbuilder.module.cssdashboard/lib/__tests__/api.test.tsdashboard/lib/api-types.generated.tsdashboard/lib/api-types.tsdashboard/lib/api.tsserver/app/api/setbuilder.pyserver/app/schemas/setbuilder.pyserver/app/services/setbuilder/pass1_deterministic.pyserver/app/services/setbuilder/pass2_agent.pyserver/openapi.jsonserver/tests/test_setbuilder_pass1.pyserver/tests/test_setbuilder_pass2.pyserver/tests/test_setbuilder_pass_api.py
Co-Authored-By: Claude <noreply@anthropic.com>
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)
server/app/services/setbuilder/pass2_agent.py (1)
112-116:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winJSON fallback lacks error handling.
If the LLM doesn't return tool calls (despite
force_tool) andresponse.textis not valid JSON,json.loadsraises an unhandledJSONDecodeError. Since_critique_from_payloadalready defaults missing fields gracefully, wrap the parsing in try/except to provide a sensible fallback critique instead of a 500 error.Proposed fix
payload: dict[str, Any] = {} if response.tool_calls: payload = response.tool_calls[0].input elif response.text: - payload = json.loads(response.text) + try: + payload = json.loads(response.text) + except json.JSONDecodeError: + payload = {} # _critique_from_payload handles missing fields return _critique_from_payload(payload)🤖 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/pass2_agent.py` around lines 112 - 116, The json.loads call in the elif branch (when response.tool_calls is empty and response.text exists) lacks error handling for invalid JSON. Wrap the json.loads(response.text) statement in a try/except block to catch JSONDecodeError, and provide a sensible fallback payload (such as an empty dict) that can be safely passed to _critique_from_payload, allowing the function to return a gracefully degraded critique instead of raising an unhandled exception.
🤖 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 `@server/app/services/setbuilder/pass2_agent.py`:
- Around line 112-116: The json.loads call in the elif branch (when
response.tool_calls is empty and response.text exists) lacks error handling for
invalid JSON. Wrap the json.loads(response.text) statement in a try/except block
to catch JSONDecodeError, and provide a sensible fallback payload (such as an
empty dict) that can be safely passed to _critique_from_payload, allowing the
function to return a gracefully degraded critique instead of raising an
unhandled exception.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c3a1345-9944-4b0a-b86d-ab78b46cdce3
📒 Files selected for processing (8)
dashboard/app/(dj)/setbuilder/[setId]/page.tsxdashboard/app/(dj)/setbuilder/components/ChatSidebar.tsxdashboard/lib/api-types.generated.tsserver/app/api/setbuilder.pyserver/app/schemas/setbuilder.pyserver/app/services/setbuilder/pass1_deterministic.pyserver/app/services/setbuilder/pass2_agent.pyserver/openapi.json
✅ Files skipped from review due to trivial changes (1)
- dashboard/lib/api-types.generated.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- dashboard/app/(dj)/setbuilder/[setId]/page.tsx
- server/app/schemas/setbuilder.py
- dashboard/app/(dj)/setbuilder/components/ChatSidebar.tsx
- server/app/api/setbuilder.py
- server/app/services/setbuilder/pass1_deterministic.py
- server/openapi.json
This was referenced Jun 14, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
WrzDJSet needs the core v1 two-pass flow: deterministic pool + curve ordering first, then an LLM-assisted critique/chat editor for vibe, culture, and narrative adjustments.
What
Testing
cd server && /home/adam/github/WrzDJ/server/.venv/bin/ruff check .cd server && /home/adam/github/WrzDJ/server/.venv/bin/ruff format --check .cd server && /home/adam/github/WrzDJ/server/.venv/bin/bandit -r app -c pyproject.toml -qcd server && /home/adam/github/WrzDJ/server/.venv/bin/pytest --tb=short -q(2811 passed, coverage 87.89%)cd dashboard && npx tsc --noEmitcd dashboard && npm run lintcd dashboard && npm test -- --run(83 files, 1143 tests)Design decisions
key_strictnessand defaults to a low-impact contribution for open-format sets, matching the executive summary guidance.services/llm/gateway.py; the current gateway exposes connector model hints rather than fast/strong tiers, so the code documents the conceptual fast/strong split without overriding connector-configured models.rationale; API responses include both tool args and rationale so the chat log can provide an audit trail.Closes #390
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests