Skip to content

Add backtest signal alpha design - #1409

Merged
stranske merged 2 commits into
mainfrom
codex/issue-1401-backtest-signal-alpha-design
Jul 9, 2026
Merged

stranske merged 2 commits into
mainfrom
codex/issue-1401-backtest-signal-alpha-design

Conversation

@stranske

@stranske stranske commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Closes #1401

Summary

Validation

  • git diff --check
  • README/docs line-length scan (only pre-existing README note line exceeds 100 chars)

Post-push review: recorded for the next automation tick; this run does not wait for CI.

Summary by CodeRabbit

  • Documentation
    • Added a new API documentation entry for “Backtest Signal Alpha Design.”
    • Introduced detailed guidance for point-in-time strategy validation, return attribution, pricing inputs, metrics, and reproducible run records.
    • Expanded notes on supported workflows, expected data formats, and validation scenarios for backtest results.

Copilot AI review requested due to automatic review settings July 9, 2026 19:07
@stranske stranske added agent:codex Assign to Codex agent autofix Let bots format/lint automatically agents:keepalive Enable keepalive monitoring on PR labels Jul 9, 2026
@stranske
stranske temporarily deployed to agent-standard July 9, 2026 19:07 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 41 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b670a6a1-e479-49ed-be3d-ff2dd046e4a1

📥 Commits

Reviewing files that changed from the base of the PR and between f9c50b4 and 0bb7b23.

📒 Files selected for processing (2)
  • README.md
  • docs/backtest_signal_alpha_design.md
📝 Walkthrough

Walkthrough

This PR adds a new design document, docs/backtest_signal_alpha_design.md, describing a backtest/signal-alpha validation harness including strategy contract, price provider protocol, metrics, data model, API rollout, and acceptance tests. README.md is updated with a corresponding API Documentation link.

Changes

Backtest Design Documentation

Layer / File(s) Summary
Backtest harness design document
docs/backtest_signal_alpha_design.md
New design doc defines the strategy contract centered on high_conviction_new_buys, a PriceReturnProvider protocol, first-slice metrics, additive data model tables (backtest_runs, backtest_positions, backtest_metrics), CLI/service rollout plan (no operator UI initially), acceptance tests, and open risks.
README API documentation link
README.md
Adds a new API Documentation bullet linking to the new design doc with a short description.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The docs cover the strategy spec, point-in-time holdings contract, disclosure lag, price-source plan, metrics, and UI scope decisions required by #1401.
Out of Scope Changes check ✅ Passed The changes stay within documentation and a README link update, with no unrelated implementation or feature work.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding the backtest signal alpha design document and README link.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-1401-backtest-signal-alpha-design

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

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/backtest_signal_alpha_design.md`:
- Around line 75-87: Make benchmark support required in the provider contract:
the current PriceReturnProvider.returns signature allows benchmark to be
omitted, but the harness always expects benchmark-relative metrics and stores
benchmark on each metric row. Update the adapter contract so benchmark is
mandatory in returns, and then propagate that requirement through any harness or
caller code that constructs or invokes PriceReturnProvider so runs cannot
proceed without benchmark data.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1c07d396-75e0-441f-9e2d-b7f83bf69f8d

📥 Commits

Reviewing files that changed from the base of the PR and between 4cd3a14 and f9c50b4.

📒 Files selected for processing (2)
  • README.md
  • docs/backtest_signal_alpha_design.md

Comment thread docs/backtest_signal_alpha_design.md

Copilot AI 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.

Pull request overview

Adds a new design document describing a backtest / “signal alpha” validation harness that depends on point-in-time (bitemporal) holdings, and links it from the README so it’s discoverable alongside other API/design docs.

Changes:

  • Add a new design doc defining strategy replay semantics, adapter boundaries for price/returns data, proposed metrics, and a draft data model for backtest runs.
  • Document a staged rollout plan (CLI/service first, UI later) and propose fixture-backed pricing for the initial implementation.
  • Link the new design doc from the README “API Documentation” section.

Reviewed changes

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

File Description
README.md Adds a documentation link to the new backtest/signal-alpha design doc.
docs/backtest_signal_alpha_design.md Introduces the backtest harness design: strategy contract, price adapter boundary, metrics, data model sketch, and phased surface plan.

Comment thread docs/backtest_signal_alpha_design.md Outdated
Comment thread docs/backtest_signal_alpha_design.md
Comment thread docs/backtest_signal_alpha_design.md Outdated
@stranske

stranske commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

Closer review-fix update pushed in 2d0275e.

Addressed the CodeRabbit/Copilot design-contract threads:

  • aligned the document H1 with the README link text
  • clarified entry timing uses filed_date + disclosure_lag while knowledge_time only gates as-of visibility
  • made benchmark mandatory in PriceReturnProvider.returns
  • defined ReturnFrame row shape and benchmark-data failure semantics

Validation:

  • git diff --check -> passed
  • README/docs line-length scan -> only pre-existing README line 40 exceeds 100 chars

Post-push review: recorded for the next automation tick; fresh checks/reviews may still be settling.

@stranske
stranske force-pushed the codex/issue-1401-backtest-signal-alpha-design branch from 2d0275e to 0bb7b23 Compare July 9, 2026 19:26
@stranske

stranske commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

Closer rebase update pushed in 0bb7b23.

Rebased after #1407 merged and resolved the README API Documentation conflict by preserving both links:

  • Manager Similarity
  • Backtest Signal Alpha Design

Validation:

  • git diff --check origin/main..HEAD -> passed
  • README/docs line-length scan -> only pre-existing README line 41 exceeds 100 chars

Post-push review: recorded for the next automation tick; fresh checks/reviews may still be settling.

@stranske stranske added the agent:retry Add to trigger agent retry after rate limit or pause label Jul 9, 2026
@stranske
stranske temporarily deployed to agent-standard July 9, 2026 20:04 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Runner dispatch state for codex on PR #1409. Do not edit.

@stranske-keepalive

stranske-keepalive Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🤖 Keepalive Loop Status

PR #1409 | Agent: Codex | Iteration 0/12

Current State

Metric Value
Iteration progress [----------] 0/12
Action run (agent-run-failed)
Agent status ❌ AGENT FAILED
Gate success
Tasks 0/5 complete
Timeout 45 min (default)
Timeout usage 2m elapsed (6%, 43m remaining)
Keepalive ✅ enabled
Autofix ❌ disabled

Last Codex Run

Result Value
Status ❌ AGENT FAILED
Reason agent-run-failed
Exit code unknown
Failures 1/3 before pause

To retry immediately:

  • Add the agent:retry label to this PR

Or wait for the next successful Gate run to automatically retry.

🔍 Failure Classification

| Error type | infrastructure |
| Error category | transient |
| Suggested recovery | Capture logs and context; retry once and escalate if the issue persists. |

⚠️ Failure Tracking

| Consecutive failures | 1/3 |
| Reason | agent-run-failed |

@agents-workflows-bot

Copy link
Copy Markdown
Contributor
Keepalive Work Log (click to expand)
# Time (UTC) Agent Action Result Files Tasks Progress Commit Gate
0 2026-07-09 20:06:50 Codex run (agent-run-failed) failure 30 file(s) 0 0/5 success

@stranske
stranske merged commit 820ed24 into main Jul 9, 2026
65 checks passed
@stranske
stranske deleted the codex/issue-1401-backtest-signal-alpha-design branch July 9, 2026 20:24
@stranske stranske added the verify:compare Runs verifier comparison mode after merge label Jul 9, 2026
@stranske
stranske temporarily deployed to agent-standard July 9, 2026 20:24 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Provider Comparison Report

Provider Summary

Provider Model Verdict Confidence Summary
openai gpt-5.4 PASS 95% This merged PR satisfies the documented scope: it adds a backtest/signal-alpha design document, defines strategy replay semantics, price-provider adapter boundaries, metrics, data model and rollout...
anthropic claude-sonnet-4-6 PASS 88% This PR delivers exactly what the acceptance criteria require: (1) a new design document covering strategy replay semantics, price-provider adapter boundaries, metrics, data model sketches, and sta...
📋 Full Provider Details (click to expand)

openai

  • Model: gpt-5.4
  • Verdict: PASS
  • Confidence: 95%
  • Scores:
    • Correctness: 9.0/10
    • Completeness: 10.0/10
    • Quality: 9.0/10
    • Testing: 6.0/10
    • Risks: 9.0/10
  • Summary: This merged PR satisfies the documented scope: it adds a backtest/signal-alpha design document, defines strategy replay semantics, price-provider adapter boundaries, metrics, data model and rollout guidance, and links the design from the README API/docs section. The design is aligned with the point-in-time/bitemporal holdings prerequisite from [Design] Point-in-time / bitemporal holdings (foundation for backtest + attribution) #1400 and explicitly confirms sequencing and the internal-harness-first scope requested in issue [Design] Backtest / signal-alpha harness (validate conviction & activism signals) #1401. Quality is strong for a docs/design change, with clear structure and maintainable contracts. No functional code or tests were added, but for a design-only PR that does not block acceptance. Based on the diff shown, the implementation meets the acceptance criteria.
  • Concerns:
    • No dedicated tests were added for the new documentation link or document content, though this is low risk for a docs-only design PR.
    • The verifier prompt truncates the tail of the design doc, so line-length acceptance is inferred from the provided content and PR intent rather than exhaustively rechecked here.

anthropic

  • Model: claude-sonnet-4-6
  • Verdict: PASS
  • Confidence: 88%
  • Scores:
    • Correctness: 9.0/10
    • Completeness: 9.0/10
    • Quality: 9.0/10
    • Testing: 5.0/10
    • Risks: 9.0/10
  • Summary: This PR delivers exactly what the acceptance criteria require: (1) a new design document covering strategy replay semantics, price-provider adapter boundaries, metrics, data model sketches, and staged API/UI rollout, all grounded in the point-in-time holdings contract; and (2) a two-line README link addition in the API documentation section. The git diff check criterion is satisfied with no whitespace errors visible. The line-length criterion appears satisfied for the new README lines. The design document is well-structured, clearly scoped to an internal harness first, defers the paid-provider decision appropriately, and defines a narrow but extensible strategy and metrics contract. The only notable gap is the absence of any accompanying test or fixture file, but the acceptance criteria for this PR are documentation-focused and do not mandate tests. Overall the changes are correct, complete relative to the stated criteria, and of good quality.
  • Concerns:
    • No automated tests were added alongside the design document. The acceptance criteria do not explicitly require tests for a design doc PR, but the absence of any test fixture or validation script means the design's testability claims are unverified in code.
    • The CI workflow 'ci.yml' was still in_progress at evaluation time (per the prompt), but per instructions this is ignored post-merge. Noted only for completeness.
    • The design document references a PriceReturnProvider Protocol and ReturnFrame type, but no stub or skeleton implementation is included. This is acceptable for a design PR but means the adapter boundary is purely aspirational until a follow-up PR lands.
    • Line-length acceptance criterion states 'only pre-existing README note line exceeds 100 chars.' The new README lines added are short (well under 100 chars) and the design doc lines appear within limits based on the visible diff, so this criterion appears satisfied but the full doc was truncated in the diff, making a complete line-length audit impossible from the provided context.

Agreement

  • Verdict: PASS (all providers)
  • Correctness: scores within 1 point (avg 9.0/10, range 9.0-9.0)
  • Completeness: scores within 1 point (avg 9.5/10, range 9.0-10.0)
  • Quality: scores within 1 point (avg 9.0/10, range 9.0-9.0)
  • Testing: scores within 1 point (avg 5.5/10, range 5.0-6.0)
  • Risks: scores within 1 point (avg 9.0/10, range 9.0-9.0)

Disagreement

No major disagreements detected.

Unique Insights

  • openai: No dedicated tests were added for the new documentation link or document content, though this is low risk for a docs-only design PR.; The verifier prompt truncates the tail of the design doc, so line-length acceptance is inferred from the provided content and PR intent rather than exhaustively rechecked here.
  • anthropic: No automated tests were added alongside the design document. The acceptance criteria do not explicitly require tests for a design doc PR, but the absence of any test fixture or validation script means the design's testability claims are unverified in code.; The CI workflow 'ci.yml' was still in_progress at evaluation time (per the prompt), but per instructions this is ignored post-merge. Noted only for completeness.; The design document references a PriceReturnProvider Protocol and ReturnFrame type, but no stub or skeleton implementation is included. This is acceptable for a design PR but means the adapter boundary is purely aspirational until a follow-up PR lands.; Line-length acceptance criterion states 'only pre-existing README note line exceeds 100 chars.' The new README lines added are short (well under 100 chars) and the design doc lines appear within limits based on the visible diff, so this criterion appears satisfied but the full doc was truncated in the diff, making a complete line-length audit impossible from the provided context.

🔍 LangSmith Traces

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Workflow state fingerprint for Agents Verifier. Do not edit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:codex Assign to Codex agent agent:retry Add to trigger agent retry after rate limit or pause agents:keepalive Enable keepalive monitoring on PR autofix Let bots format/lint automatically verify:compare Runs verifier comparison mode after merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Design] Backtest / signal-alpha harness (validate conviction & activism signals)

2 participants