Skip to content

fix: deduplicate monthly Russell execution - #350

Merged
Pigbibi merged 2 commits into
mainfrom
agent/fix-russell-monthly-dedup
Aug 6, 2026
Merged

fix: deduplicate monthly Russell execution#350
Pigbibi merged 2 commits into
mainfrom
agent/fix-russell-monthly-dedup

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • use the monthly feature snapshot date as the stable execution signal identity when no explicit signal date exists
  • persist the cycle execution identity into runtime report summaries
  • add regression coverage for Russell snapshot mapping and report persistence
  • align the QPK dependency pin required by the repository's current CI contract

Root cause

The Russell monthly strategy exposed snapshot_as_of but not signal_date. The LongBridge duplicate marker builder therefore received an empty execution identity and skipped marker reads/writes, allowing every day in the three-day execution window to submit another rebalance.

Behavior

After the first successful order submission records the monthly snapshot identity, later scheduler invocations for the same snapshot are skipped. A cycle that submits no order still does not record a marker and remains eligible for a later retry.

Dependency note

CI resolves QuantPlatformKit main and requires downstream pins to match its canonical QPK_PIN. The original PR run failed because this repository still pinned 92458590a463; the PR now uses the required 9618b4bd8e17. Full LongBridge tests were rerun against that exact pin.

Validation

  • regression test verified RED before the fix and GREEN afterward
  • uv run --frozen pytest -q tests/test_strategy_runtime.py tests/test_decision_mapper.py tests/test_rebalance_service.py tests/test_execution_state.py tests/test_request_handling.py (100 passed)
  • uv run --no-sync pytest -q tests (315 passed, 15 subtests passed) against QPK 9618b4bd8e17
  • uv run --no-sync ruff check --exclude external .
  • uv run --no-sync python scripts/check_qpk_pin_consistency.py
  • uv lock --check
  • git diff --check

Pigbibi and others added 2 commits August 6, 2026 18:41
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi
Pigbibi merged commit a16516a into main Aug 6, 2026
1 check passed
@Pigbibi
Pigbibi deleted the agent/fix-russell-monthly-dedup branch August 6, 2026 10:46

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eafa5d0ead

ℹ️ 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".

Comment thread main.py
Comment on lines +305 to +306
if execution.get(field) not in (None, ""):
summary[field] = execution[field]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep no-order monthly cycles eligible for retry

Only persist this execution identity when the cycle actually placed or previewed an order. As written, a Russell cycle with action_done=False and no skip logs—such as the zero-equity path in _normalize_to_value_target_decision—is saved as a successful report with the snapshot-derived signal_date; _report_matches_execution explicitly treats such clean no-action reports as completed, so if the account is funded later in the same three-day window, has_prior_execution_report suppresses the rebalance for that snapshot even though _should_record_execution_marker deliberately did not record a marker.

Useful? React with 👍 / 👎.

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.

1 participant