Skip to content

fix(basket): industry-grade engine, substitutes, and review fixes - #57

Merged
Gyaan507 merged 2 commits into
mainfrom
hotfix/basket-apis-latency
Sep 6, 2026
Merged

fix(basket): industry-grade engine, substitutes, and review fixes#57
Gyaan507 merged 2 commits into
mainfrom
hotfix/basket-apis-latency

Conversation

@sahim99

@sahim99 sahim99 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Industry-grade basket backend: create claim→reserve→persist→complete (no orphan 409), MISSING buys with includeHeld=true, Unknown-sector soft gates, batched ledger on holdings, priceQuality/priceAsOf, drafts harden (DuplicateKey, Number overrides, payload cap).
  • Review ops: shared Kafka SASL props, config-driven idempotency TTL + draft max-limit, null stale Mongo timestamps, precompiled ISIN Patterns, relative e2e README paths.
  • CI: preprod aligns with trade pipeline (am-portfolio-preprod, org GHCR, direct-deploy.yml @ main).

Review map (fixed in ffca258)

Thread Fix
Kafka SASL under consumer only Moved to spring.kafka.properties
Idempotency 24h hardcoded basket.create.idempotency-ttl-seconds + index config
DRAFT_LIMIT=5 basket.drafts.max-limit
Stale timestamp = now Mongo fallback leaves timestamp=null
Physical weight vs inventory Planning-only comment; create ledger 409 before persist
ISIN String.matches Static Patterns in EtfApiClient
Absolute e2e README paths .\scripts\...

P0 / P1

  • P0: create integrity, MISSING sizing, Unknown sectorial, drafts, apply price fail-open, review ops
  • P1: holdings batch (no per-ISIN ledger N+1), price quality, ETF L1 TTL / skip poor enrich
  • P2 (light): unique operationIds, final-preview alias docs, optional catalog write token

API contract notes

  • Create: 409 before portfolio row; idempotent key claim IN_PROGRESSCOMPLETED
  • Calc: MISSING always gap-fills when price+weight; priceWarnings
  • Apply: Unknown sector does not hard-gate; prices fail-open
  • Drafts: GET /drafts/{basketId}; DuplicateKey → merge 200

Test checklist

  • BasketPortfolioCreateServiceTest (incl. over-alloc before persist)
  • BasketEngineExclusiveSubstituteTest / BasketEngineServiceTest (Unknown not sectorial; MISSING buys)
  • BasketDraftServiceTest + BasketControllerTest drafts route
  • MarketDataServiceStaleBackfillTest

Preprod deploy note

Helm release am-portfolio-preprod; Argo owns image tags via gitops after CI publish.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 62052d78-9ccd-41c4-b928-250f4d4864fa


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sahim99
sahim99 requested a deployment to preprod-am-portfolio September 2, 2026 21:06 — with GitHub Actions Waiting
@sahim99
sahim99 requested a deployment to preprod-am-portfolio September 3, 2026 14:46 — with GitHub Actions Waiting
@sahim99
sahim99 requested a deployment to preprod-am-portfolio September 3, 2026 14:46 — with GitHub Actions Waiting
@ssd2658 ssd2658 changed the title Fix Kafka topic configurations and update deploy script fix(basket): API latency + apply-substitutes swap Sep 4, 2026
@sahim99
sahim99 requested a deployment to preprod-am-portfolio September 4, 2026 11:51 — with GitHub Actions Waiting
@sahim99
sahim99 requested a deployment to preprod-am-portfolio September 4, 2026 11:56 — with GitHub Actions Waiting
@ssd2658 ssd2658 changed the title fix(basket): API latency + apply-substitutes swap fix(basket): API latency + apply-substitutes residual capacity Sep 6, 2026

@ssd2658 ssd2658 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Senior Architectural & Code Quality Review for PR #57

Thank you for this PR! The latency improvements (batching ETF lookups, decomposing BasketEngineService into focused components, and adding the fail-open fallback) are great additions.

I have conducted a file-by-file review with a focus on hardcoded credentials/endpoints/paths, configuration management, data integrity, and operational reliability.

Please review the inline comments and suggestions below.

Comment thread scripts/README-basket-prod-e2e.md Outdated
Comment thread portfolio-app/src/main/resources/application.yml Outdated
Comment thread portfolio-basket/src/main/java/com/portfolio/basket/client/EtfApiClient.java Outdated
sahim99 and others added 2 commits September 6, 2026 14:51
Claim→reserve→persist create, MISSING buy sizing, Unknown-sector soft gates,
batched holdings ledger, priceQuality, drafts harden, and review ops fixes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Use org GHCR image, direct-deploy.yml -> central-deploy@main, and
trade-style preprod gitops/helm flags on portfolio-publish.

Co-authored-by: Cursor <cursoragent@cursor.com>
@sahim99
sahim99 force-pushed the hotfix/basket-apis-latency branch from e937ced to 65b69ff Compare September 6, 2026 09:21
@ssd2658 ssd2658 changed the title fix(basket): API latency + apply-substitutes residual capacity fix(basket): industry-grade engine, substitutes, and review fixes Sep 6, 2026
@Gyaan507
Gyaan507 merged commit 0cfad06 into main Sep 6, 2026
13 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants