Ci [SCRUM-180] us 705 multi stage docker image builds for production#76
Conversation
…le support, improved error handling, and context usage tracking
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8affb923b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| _logger.LogError(ex, "Google login failed for email {Email}.", payload.Email); | ||
| return StatusCode(500, new { message = "Google sign-in failed while linking your account." }); | ||
| return StatusCode(500, new { message = $"Google sign-in failed: {ex.Message}" }); |
There was a problem hiding this comment.
Remove exception details from Google sign-in error responses
This response now returns ex.Message to the client on Google login failures, which can leak internal provider/database/runtime details whenever auth errors occur. Those details should stay server-side in logs; exposing them in a public 500 response increases information disclosure risk without helping clients recover.
Useful? React with 👍 / 👎.
| var systemPrompt = BuildSystemPrompt(node, snapshot, includeFormattingInstruction); | ||
| var provider = node.PreferredProvider ?? InferProviderFromModel(node.PreferredModel); | ||
| return BuildUsageEstimate(systemPrompt, history, provider, node.PreferredModel); |
There was a problem hiding this comment.
Exclude compacted history when estimating context usage
When a snapshot exists, older messages before snapshot.BoundaryMessageId have already been compacted into the system prompt, but this method still estimates usage from the full history. That double-counts compacted content (snapshot + original messages), so persisted/displayed token usage is inflated and can incorrectly report nodes as near/over context limits.
Useful? React with 👍 / 👎.
| if (rowsUpdated == 0) | ||
| throw new InvalidOperationException("Chat node layout update did not persist."); |
There was a problem hiding this comment.
Avoid throwing 500 for missing nodes in layout updates
A missing/stale node id now throws InvalidOperationException, but SaveLayoutAsync does not catch/translate it, so normal client staleness (for example, a node deleted before a delayed drag-save arrives) becomes an unhandled 500. This should be surfaced as a controlled 4xx path instead of treating it as an internal server fault.
Useful? React with 👍 / 👎.
Summary
This PR introduces improvements to the CI/CD pipeline and Docker setup, focusing on multi-stage Docker image builds for production and enhanced validation processes.
Type of Change
Changes Made
How to Test
Related Commits
ci [SCRUM-180]: enhance CI/CD pipeline with Docker image validationfeat [SCRUM-28]: enhance Docker setup and API with environment variablesChecklist