-
Notifications
You must be signed in to change notification settings - Fork 5
feat: Discovery Answer UI Integration (Feature 012) #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
09e50d4
feat: Complete planning for 012-discovery-answer-ui (Sprint 9.5 Featu…
frankbria b265252
feat: Add discovery answer UI foundation and input field (Phase 1-3)
frankbria 860a435
feat: Implement discovery answer submission UI (Phases 4-7, US2-US7)
frankbria b1e1e79
feat: Complete discovery answer UI with tests and accessibility (Phas…
frankbria dd77c2f
fix: Add critical discovery state validation to answer endpoint
frankbria bf6b3a7
fix: Resolve three runtime issues in discovery answer endpoint
frankbria 5c96bcf
fix: Prevent divide-by-zero and out-of-range percentage calculations
frankbria 637940e
fix: Add submission guard to prevent duplicate concurrent requests
frankbria File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix critical data integrity bug: capture answered question ID before processing.
The
broadcast_discovery_answer_submittedcall at line 682 passes the wrongquestion_id. The current implementation:agent.process_discovery_answer()which updates the agent's_current_question_idto the next questioncurrent_questionreflects the next question, not the answered one)current_question_idfrom this updated statusAccording to
broadcast_discovery_answer_submitteddocumentation, thequestion_idparameter should be "Unique identifier of the answered question", but we're passing the next question's ID instead. This breaks WebSocket event tracking and could cause UI state inconsistencies.Apply this diff to capture the answered question ID before processing:
# CRITICAL: Validate discovery is active before processing answer status = agent.get_discovery_status() if status.get('state') != 'discovering': raise HTTPException( status_code=400, detail=f"Discovery is not active. Current state: {status.get('state')}. " f"Please start discovery first by calling POST /api/projects/{project_id}/discovery/start" ) + + # Capture the question ID being answered (before processing updates it to next question) + answered_question_id = status.get("current_question", {}).get("id", "") # Process the answer (trimmed by Pydantic validator) agent.process_discovery_answer(answer_data.answer) # Get updated discovery status after processing status = agent.get_discovery_status() # ... (rest of code) try: # Broadcast answer submitted event await broadcast_discovery_answer_submitted( manager=manager, project_id=project_id, - question_id=current_question_id, + question_id=answered_question_id, answer_preview=answer_data.answer[:100], # First 100 chars current_index=current_question_index, total_questions=total_questions, )📝 Committable suggestion
🧰 Tools
🪛 Ruff (0.14.5)
648-652: Abstract
raiseto an inner function(TRY301)
663-663: Do not catch blind exception:
Exception(BLE001)
664-664: Use
logging.exceptioninstead oflogging.errorReplace with
exception(TRY400)
665-665: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
665-665: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents