Bhavya patch#8
Conversation
ConfigPanel concatenated service defaults and node.config overrides without deduplication. If an override used the same key as a default (e.g. "Runtime"), the same key appeared twice in the list, causing duplicate React keys and potential incorrect row updates. Now using a Map so node-level overrides win and each key appears exactly once.
DiagramEdge called nodes.find() twice per edge on every render, making edge resolution O(N*E). The main component already builds a nodeMap; pass it into DiagramEdge so each endpoint lookup is O(1).
The coarse ForgeArchNode.type caused mis-renders: cache mapped to DynamoDB instead of ElastiCache, storage mapped to S3 instead of RDS, auth mapped to Cognito instead of Secrets Manager. mapForgeTypeToService is replaced with mapForgeNodeToService which checks terraformResource first and falls back to the type field only when no tf resource match exists.
tooltipTimerRef was cleared on hover/leave but not on component unmount. If the component unmounted while a 250ms tooltip delay was pending, the scheduled setTooltip would fire on an unmounted component causing a React warning. Added a useEffect cleanup to cancel the timer.
The wrapper div had an aria-label describing the diagram but no ARIA role, so assistive technology would not treat it as a landmark or announce the label. Adding role="img" makes the relationship explicit and ensures screen readers announce the diagram's textual description.
feat(arch): interactive diagram with tooltips, config panels, and build file navigation
…ndRequest, ReviewRequest
…n and node events
…th real SSE calls, remove stub deploy route
…r and add missing ownership checks to arch SSE respond/review endpoints
… all decrypt calls with HTTP 400
… disconnect detection, sanitize error messages
…s, prd, arch, builds, deployments
…ted serializers from build and deploy routers
…name, description, and cloud fields
…register, 10/min login
…oller for SSE streams, deprecate mock createProject
preview ui changes
There was a problem hiding this comment.
Pull request overview
This PR updates the backend’s local Ollama LLM configuration and hardens agent1’s clarification/planning flow against inconsistent model JSON shapes, while improving option-selection behavior in the standalone smoke test.
Changes:
- Rename the Ollama model setting from the previous Qwen-specific key to a generic
LLM_MODEL/llm_modelconfiguration. - Normalize/repair LLM JSON outputs (clarifier + planner) and improve deduping/filtering of already-answered clarification questions.
- Refine standalone option text matching to reduce ambiguous prefix collisions.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/app/config.py | Renames/standardizes the model setting to llm_model. |
| backend/app/agents/agent1/llm.py | Switches ChatOllama factory to use settings.llm_model. |
| backend/app/agents/agent1/state.py | Changes answered_qa_pairs structure to dict-based entries. |
| backend/app/agents/agent1/nodes.py | Adds payload coercion, fallback clarification questions, answer ingestion, and filtering logic for already-answered questions. |
| backend/app/agents/agent1/prompts.py | Tightens clarifier prompt schema rules for follow_up_questions. |
| backend/app/agents/agent1/standalone_smoke_test.py | Improves option matching with exact-first, then “contains” best-match selection. |
| backend/README.md | Updates documented env var name to LLM_MODEL. |
| backend/.env.example | Updates example env var name to LLM_MODEL. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -94,7 +94,7 @@ class AgentState(BaseModel): | |||
| # User-selected answers mapped by question index or question text. | |||
| selected_option_answers: dict[int, str] = Field(default_factory=dict) | |||
| # Structured Q&A pairs of (question, answer) to track clarifications with context. | |||
There was a problem hiding this comment.
The field comment still describes answered_qa_pairs as a list of tuples, but the type was changed to list[dict[str, str]]. Update the comment to reflect the new {question, answer} shape to avoid confusion for future maintainers.
| # Structured Q&A pairs of (question, answer) to track clarifications with context. | |
| # Structured Q&A pairs as {"question": ..., "answer": ...} dicts to track clarifications with context. |
| # Second pass: contains match, prefer the longest label to avoid prefix collisions | ||
| # like "S3 Standard" matching "S3 Standard-IA". | ||
| contains_matches: list[tuple[int, QuestionOption]] = [] | ||
| for option in question.options: | ||
| option_label = _normalize(option.label) | ||
| if normalized_input in option_label or option_label in normalized_input: | ||
| contains_matches.append((len(option_label), option)) | ||
|
|
||
| if contains_matches: | ||
| _, best_option = max(contains_matches, key=lambda item: item[0]) | ||
| state.selected_option_answers[question_idx] = best_option.value | ||
| logger.info("User matched option text for question %s: %s", question_idx + 1, best_option.label) | ||
| return |
There was a problem hiding this comment.
The new “contains match” pass can incorrectly match the built-in Custom option when a user types a free-form answer that includes the word "custom" (e.g., "Custom: HIPAA + SOC2"), causing the input to be reduced to the option value (often just "custom") and losing the actual answer text. Consider treating matches to is_custom options as free-form unless the user input exactly equals the option label/value (or explicitly selects it).
| - `LLM_MODEL` (default: `gemma3:latest`) | ||
| - `ENABLE_WEB_SEARCH` (`true`/`false`) |
There was a problem hiding this comment.
The README now documents LLM_MODEL as the default model, but later in the same README the run instructions still tell users to ollama pull qwen3.5:latest (and the intro mentions a Qwen model). This becomes inconsistent/confusing after the env var rename—update those references to match the new default/model naming.
devgunnu
force-pushed
the
main
branch
2 times, most recently
from
6b25aac to
53e4058
Compare
No description provided.