Skip to content

CORE-3868: Replace PopularCombination schema with session-ready combinations - #10

Merged
Rodnyy merged 1 commit into
mainfrom
feat/CORE-3868
Jul 28, 2026
Merged

Rodnyy merged 1 commit into
mainfrom
feat/CORE-3868

Conversation

@Rodnyy

@Rodnyy Rodnyy commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Link to JIRA ticket

Why

The PopularCombination endpoint never shipped (fujin has always returned Unimplemented), and its response shape — a list of markets — would force clients to re-assemble selections themselves. The quick-picks product needs the offer to be one-click creatable: the client must be able to echo the offer straight into SessionCreateRequest without any assembly of its own.

What

PopularCombinationResponse is replaced without a migration path (no producer ever emitted the old shape):

  • combinations — each entry carries selection_ids in the exact SessionCreateRequest.selection_ids format, total odds (×10 000, same convention as the session protos) and raw_probability (pre-margin, 0.0–1.0).
  • The old repeated PopularCombinationMarket markets = 1 field is deleted; field number and name are reserved — kollector v0.7.0 has compiled (dead) references to the old shape, and reserving keeps a stale decoder failing soft instead of hitting a wire-type mismatch if the number were reused.
  • Generated bindings regenerated for all languages (go/java/js/python).

After merge

Tag v0.9.0 — fujin's PopularCombination implementation (CORE-3868) depends on it, and kollector's next obbschema bump must drop its dead PopularCombinationMarket scaffolding.

…nations

The endpoint never shipped (fujin returns Unimplemented), so the old
markets-only shape is replaced without a migration path; field 1 is
reserved. Each combination now carries selection IDs in the exact
SessionCreateRequest.selection_ids format plus total odds and raw
probability, so a client can render a clickable offer and create a
session from it without assembling selections itself.
@Rodnyy
Rodnyy requested review from a team and Copilot July 15, 2026 11:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the PopularCombinationResponse protobuf schema to return session-ready selection combinations (directly usable in SessionCreateRequest), replacing the previously unused “markets list” shape while reserving the old field name/number to avoid accidental reuse.

Changes:

  • Replaces PopularCombinationResponse.markets (field 1) with PopularCombinationResponse.combinations (field 2) and reserves field number/name 1/"markets".
  • Introduces PopularCombination message containing selection_ids, odds (×10,000), and raw_probability.
  • Regenerates language bindings for Go/Java/JS/Python to match the updated schema.

Reviewed changes

Copilot reviewed 2 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
proto/obb/popular.proto Updates the protobuf schema: reserves legacy markets field and adds combinations + PopularCombination message.
go/obb/popular.pb.go Regenerates Go bindings to expose Combinations and the new PopularCombination message fields.
java/src/main/java/com/oddin/obb/Popular.java Regenerates Java bindings replacing markets accessors/types with combinations and new message structure.
js/obb/popular_pb.js Regenerates JS bindings to replace PopularCombinationMarket with PopularCombination and wire field 2 for combinations.
js/obb/popular_pb.d.ts Updates TS declarations for the new PopularCombinationResponse and PopularCombination APIs.
python/obb/popular_pb2.py Regenerates Python bindings to match the updated descriptor (new message + reserved legacy field).
Files not reviewed (3)
  • go/obb/popular.pb.go: Generated file
  • java/src/main/java/com/oddin/obb/Popular.java: Generated file
  • python/obb/popular_pb2.py: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@stetinatomas-oddin stetinatomas-oddin 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.

LGTM. Clean replacement. Reserving both the old field number (1) and the name (markets) is the right call given kollector still carries compiled references to the old shape — a stale decoder then fails soft instead of risking a wire-type collision if 1 were ever reused. The new combinations shape is session-ready (selection IDs in the exact SessionCreateRequest format, odds x10000 matching the session protos, raw probability pre-margin), which is exactly what the one-click quick-picks flow needs, and bindings are regenerated across all languages. No migration concern since the endpoint never shipped.

Only follow-up is the coordination already in the description: tag v0.9.0 after merge so the fujin side can pin it.

Co-Authored-By: Claude Opus 4.8 (1M context)

@Rodnyy
Rodnyy merged commit fc739cf into main Jul 28, 2026
1 check passed
@Rodnyy
Rodnyy deleted the feat/CORE-3868 branch July 28, 2026 07:34
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.

3 participants