fix(portfolio): return explicit 204 Response on position delete#2
Conversation
Use Response(status_code=204) instead of decorator status_code with None return to avoid empty-body issues. Add router tests and make apiRequest handle 204/no-content responses without JSON parsing. Co-authored-by: Cursor <cursoragent@cursor.com>
Use Response(status_code=204) instead of decorator status_code with None return to avoid empty-body issues. Add router tests and make apiRequest handle 204/no-content responses without JSON parsing. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ironment handling - Added a new npm script for evaluating golden intents in the testing suite. - Updated GitHub Actions workflow to include environment variable handling for production and staging. - Improved error handling for missing deployment secrets and streamlined pre-deployment checks. - Introduced caching for npm and pip installations to optimize build times. Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 960a181b80
ℹ️ 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".
| runs-on: ubuntu-latest | ||
| needs: [deploy-backend-worker, pages-deploy-mode, smoke-check] | ||
| if: ${{ always() && (needs.deploy-backend-worker.result == 'failure' || needs.pages-deploy-mode.result == 'failure' || needs.smoke-check.result == 'failure') }} | ||
| if: ${{ always() && (needs.deploy-backend-worker.result == 'failure' || needs.smoke-check.result == 'failure') }} |
There was a problem hiding this comment.
Prevent rollback from running on failed deploy attempts
This job triggers rollback when deploy-backend-worker fails, not just when a newly deployed version fails smoke tests. Because the rollback command is called without a VERSION_ID, Wrangler defaults to the version before the latest and immediately makes it active, so failures like auth/npm/setup errors can unintentionally downgrade a previously healthy production worker. Limit rollback to post-deploy validation failures (or target an explicit deployment/version) to avoid accidental rollbacks.
Useful? React with 👍 / 👎.
|
|
||
|
|
||
| @router.delete("/{portfolio_id}/positions/{position_id}", status_code=status.HTTP_204_NO_CONTENT) | ||
| @router.delete("/{portfolio_id}/positions/{position_id}") |
There was a problem hiding this comment.
Document DELETE endpoint as 204 in OpenAPI
Removing status_code=HTTP_204_NO_CONTENT from the route decorator makes FastAPI document this operation with its default success code (200), even though the handler now returns Response(status_code=204). That mismatch can break generated SDK/client behavior and API contracts that rely on the OpenAPI spec, especially around no-body handling for successful deletes.
Useful? React with 👍 / 👎.
* Add safe bash script to consolidate branches into main Provides backup branch, rebase/merge loop, force-push with lease, commit validation, dry-run mode, rollback, and --exclude-branch. Co-authored-by: Unni T A <unnita1235-code@users.noreply.github.com> * Apply --exclude-branch when processing remote tracking branches Co-authored-by: Unni T A <unnita1235-code@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Unni T A <unnita1235-code@users.noreply.github.com>
…ssing Co-authored-by: Unni T A <unnita1235-code@users.noreply.github.com>
…ata, SEC Search, LangSmith
…main-only-9617 Merges Cloudflare deployment configuration and fixes. All 13 status checks passing.
…nt-langgraph Merges temporal agent LangGraph orchestration features. All 2 status checks passing.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b6cee02ee
ℹ️ 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".
| deepeval-required: | ||
| runs-on: ubuntu-latest | ||
| continue-on-error: false | ||
| needs: [deepeval-optional] |
There was a problem hiding this comment.
Decouple keyed DeepEval job from skipped dependency
deepeval-required depends on deepeval-optional, but deepeval-optional only runs when OPENAI_API_KEY is missing. In repos where the secret is set, the optional job is skipped, and GitHub Actions skips downstream needs jobs in that dependency chain, so the required DeepEval path never executes even though HAS_KEY is true.
Useful? React with 👍 / 👎.
| if config is None: | ||
| raise NotImplementedError( | ||
| "PortfolioAgent requires PortfolioAgentConfig. " | ||
| "Update app/graph/portfolio.py." | ||
| ) |
There was a problem hiding this comment.
Keep build_portfolio_agent callable without config
This change makes build_portfolio_agent() raise when no config is passed, but the server still resolves the portfolio graph via build_portfolio_agent() with no arguments (apps/server/app/graph/portfolio.py). As a result, any runtime path that resolves the portfolio graph now fails with NotImplementedError instead of returning a graph.
Useful? React with 👍 / 👎.
| language sql stable as $$ | ||
| select d.* | ||
| from public.document_embeddings d | ||
| where d.content @@ plainto_tsquery('english', fts_query) |
There was a problem hiding this comment.
Use tsvector in FTS match predicate
The migration defines where d.content @@ plainto_tsquery(...), but document_embeddings.content is a TEXT column, and @@ expects a tsvector on the left side. This makes the function definition invalid at migration time, so upgrade() can fail and block deployment of this revision.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cec28ab866
ℹ️ 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".
| # Optional: OpenAI quota / billing failures should not block the pipeline | ||
| continue-on-error: true | ||
| needs: [python-lint] | ||
| if: ${{ !secrets.OPENAI_API_KEY }} |
There was a problem hiding this comment.
Replace invalid secret reference in job-level if
This if uses secrets directly at the job level, which GitHub Actions does not support for conditionals; workflow validation fails before execution, so the DeepEval jobs never run in CI. Move the secret into an allowed context (for example via vars/needs outputs or a preceding job output) and gate on that value instead.
Useful? React with 👍 / 👎.
| analysis_llm = ChatOpenAI( | ||
| model=settings.synthesis_model, | ||
| temperature=0.0 |
There was a problem hiding this comment.
Use provider-compatible chat model for portfolio graph
This constructs a ChatOpenAI client with settings.synthesis_model, but the project’s default synthesis model is Anthropic (claude-3-5-sonnet-20241022), so resolving/invoking the portfolio graph will fail at runtime with a provider/model mismatch unless every environment overrides that setting to an OpenAI model. Use the Anthropic client here (or provider-based routing) to keep the graph callable with default configuration.
Useful? React with 👍 / 👎.
d933e02
into
feature/temporal-agent-langgraph
Fixed empty-body issues by using Response(status_code=204) instead of the decorator + None return
.Added router tests.
Updated apiRequest to handle 204/no-content responses safely without trying to parse JSON.