Show CP at resolution time on resolved questions

[SylvainChevalier]

Summary

Fixes #5053. The default aggregate forecast ("latest" CP) preview on resolved questions showed the last CP value rather than the CP at the point of resolution. These differ when a question is resolved as of a past date (it kept accumulating forecasts until the resolution was actually entered). The preview now reflects the forecast value at the question's effective close/resolution time.

Key Changes

• questions/services/forecasts.py: get_last_aggregated_forecasts_for_questions() now caps the "last" aggregate at actual_close_time. Open questions (null actual_close_time) still use the most recent forecast; closed/resolved questions only consider forecasts that started at or before actual_close_time.
• questions/serializers/aggregate_forecasts.py: Added _get_latest_aggregate_forecast(), which selects the forecast live at actual_close_time for the "latest" CP preview instead of blindly taking the most recent one. Full history is still serialized; only the "latest" preview is capped.
• tests/unit/test_questions/test_services/test_forecasts.py: New TestCPAtResolutionTime suite covering retroactively-resolved questions (preview uses CP at resolution time, not the later ones) and a regression guard that open questions still show the most recent CP.

Notes

• actual_close_time = min(actual_resolve_time, scheduled_close_time), matching the horizon that scoring already uses.
• Scores are unaffected: scoring (scoring/score_math.py) already clamps every forecast window to actual_close_time, so it computes on the correct range independently of this display value — the bug was display-only.
• Supersedes Cap aggregate forecast previews at question close time #5054 (recreated on a branch without "github" in the name, which Fly.io's abuse filter blocked for the preview deploy).

🤖 Generated with Claude Code

https://claude.ai/code/session_01XgUzpLZNHiVXK3t7UGoSsU

---

Generated by Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Fixed “latest” forecast selection for resolved questions to use the last available aggregation at (or before) the question’s actual close time.
  • Updated resolved-question previews so the close-time result is shown as active, including clearing its effective end time for preview purposes.
  • Prevented previews from displaying when no eligible forecasts exist at or before the actual close time (returns no preview).
  • Preserved full forecast history when requested, while feed views show only the relevant aggregation; open questions still use the most recent forecast.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d7044d4-23ea-4fc7-a418-dad0ab70a23b

📥 Commits

Reviewing files that changed from the base of the PR and between a52728b and 2dcf1f0.

📒 Files selected for processing (1)

• questions/serializers/aggregate_forecasts.py

---

📝 Walkthrough

Walkthrough

Resolved questions now select aggregate forecasts at their actual close time, while open questions retain the most recent forecast. Serialization marks resolved-question previews as active, and tests cover capped selection, history, no-preview behavior, and end-time handling.

Changes

Aggregate forecast selection

Layer / File(s)	Summary
Close-time query selection questions/services/forecasts.py	get_last_aggregated_forecasts_for_questions caps resolved questions at actual_close_time and preserves current-time selection for open questions.
Latest forecast serialization questions/serializers/aggregate_forecasts.py	Serialization selects the latest cutoff-eligible forecast, returns latest=None when none qualify, and clears end_time for resolved-question previews.
Resolution-time behavior tests tests/unit/test_questions/test_services/test_forecasts.py	Tests cover resolved-question capping, serialized history, cleared end times, no-preview behavior, and open-question selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• Metaculus/metaculus#4827: Also changes latest aggregate selection in the forecast service.

Suggested reviewers: hlbmtc

Poem

A rabbit checks the closing date,
Picks the forecast at the gate.
Old CPs rest in history,
The latest view stays clear and free.
Open questions hop ahead—
Resolved ones show what resolution said.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: showing CP at resolution time for resolved questions.
Linked Issues check	✅ Passed	The changes cap aggregate forecasts at actual_close_time and add tests, matching the issue's CP-at-resolution and score-safety goals.
Out of Scope Changes check	✅ Passed	The PR stays focused on forecast preview logic and regression tests, with no obvious unrelated code changes.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/issue-5053-sb5kii

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

🚀 Preview Environment

Your preview environment is ready!

Resource	Details
🌐 Preview URL	https://metaculus-pr-5055-claude-issue-5053-sb5kii-preview.mtcl.cc
📦 Docker Image	ghcr.io/metaculus/metaculus:claude-issue-5053-sb5kii-2dcf1f0
🗄️ PostgreSQL	NeonDB branch preview/pr-5055-claude-issue-5053-sb5kii
⚡ Redis	Fly Redis mtc-redis-pr-5055-claude-issue-5053-sb5kii

Details

• Commit: 2dcf1f005524ecd757b35321144d5e5e4db82e46
• Branch: claude/issue-5053-sb5kii
• Fly App: metaculus-pr-5055-claude-issue-5053-sb5kii

---

ℹ️ Preview Environment Info

Isolation:

• PostgreSQL and Redis are fully isolated from production
• Each PR gets its own database branch and Redis instance
• Changes pushed to this PR will trigger a new deployment

Limitations:

• Background workers and cron jobs are not deployed in preview environments
• If you need to test background jobs, use Heroku staging environments

Cleanup:

• This preview will be automatically destroyed when the PR is closed

[SylvainChevalier]

lgtm.

[coderabbitai]

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 `@questions/serializers/aggregate_forecasts.py`:
- Around line 79-82: Update the forecast selection logic around the eligible
forecast computation to return None when no forecast starts at or before the
cutoff, instead of falling back to forecasts[0]. Add a regression test covering
the case where every forecast begins after actual_close_time, ensuring the
history path does not expose a post-close forecast and remains consistent with
get_last_aggregated_forecasts_for_questions.

🪄 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: CHILL

Plan: Pro

Run ID: 58966cd9-aaf4-4994-8d59-6c016a38655c

📥 Commits

Reviewing files that changed from the base of the PR and between e346eb8 and b23f2d1.

📒 Files selected for processing (3)

• questions/serializers/aggregate_forecasts.py
• questions/services/forecasts.py
• tests/unit/test_questions/test_services/test_forecasts.py

[hlbmtc]

@claude Review.

Also, why don’t we filter out AggregateForecast records for closed or resolved questions directly in get_aggregated_forecasts_for_questions instead of the current approach where we filter them in get_last_aggregated_forecasts_for_questions and then apply the same filter again in _get_latest_aggregate_forecast?

Don’t implement this yet. First, investigate how get_aggregated_forecasts_for_questions is used by other functions and make sure this change wouldn’t break anything or negatively affect database query performance.

[hlbmtc]

Is this covered by indexes, won't we get performance drains? Double-check resulted query and psql plan

[SylvainChevalier]

Checked the generated SQL and EXPLAIN plan against the read replica (real data). The new predicate adds an INNER JOIN questions_question (for actual_close_time); here's the before/after for questions 45074–45079:

Before (no join):

Unique
  -> Incremental Sort (Key: question_id, method, start_time DESC; Presorted: question_id)
    -> Index Scan using questions_aggregateforecast_question_id_... 
         (Cond: question_id = ANY(...); Filter: start_time <= now())

After (with join):

Unique
  -> Incremental Sort (Presorted: question_id)
    -> Nested Loop
        -> Index Scan using questions_aggregateforecast_question_id_...
             (Cond: question_id = ANY(...); Filter: start_time <= now())
        -> Index Scan using questions_question_pkey
             (Cond: id = af.question_id; Filter: actual_close_time IS NULL OR af.start_time <= actual_close_time)

Both plans are fully index-driven — no sequential scans. The driving scan still uses the FK index on aggregateforecast.question_id, and the added join is a per-row PK lookup on questions_question_pkey (rows=1). The start_time <= actual_close_time comparison is applied as a filter on those already-narrowed rows, not a scan. Every caller passes a bounded question__in set (a post's questions, a batch), so the join stays small. So it's index-covered and no new index is needed.

---

Generated by Claude Code

[claude]

Claude finished @hlbmtc's task in 2m 26s —— View job

---

Review

The PR is well-scoped, well-tested, and the intent is clearly documented in the module-level comments. Behaviour looks correct against actual_close_time = min(actual_resolve_time, scheduled_close_time), and it lines up with how scoring/score_math.py already clamps windows.

A few observations (nothing blocking):

• questions/serializers/aggregate_forecasts.py:79 — the eligibility scan is a linear filter over a list that's already sorted by start_time. Fine at current forecast counts, so not worth changing, but noting it in case a question ever grows a very long aggregation history.
• questions/serializers/aggregate_forecasts.py:176-177 — nulling end_time on the capped preview is a pragmatic workaround for the frontend's liveness gating, and the inline comment explains it well. One subtle side effect: if the last eligible aggregate has an end_time before actual_close_time (the CP went stale before close), we now present it as live-at-close rather than exposing that staleness. This matches pre-PR behaviour for closed questions and is fine, but worth being aware of.
• questions/services/forecasts.py:419-428 — the Q(actual_close_time__isnull=True) | start_time <= actual_close_time filter mirrors the shape already used in posts/models.py:60-63 for the manager-level filter, which is nice for consistency. No new index needed — the plan reuses the existing FK join.
• Tests — good coverage: retroactive resolution + preserved history, feed path, the "all forecasts after close → no preview" boundary, and an open-question regression guard.

---

Answer: why not filter directly in get_aggregated_forecasts_for_questions?

Short answer: feasible and would simplify the code, but it's a semantic change — not a pure refactor — so it should be a deliberate follow-up, not folded into this fix.

Callers of get_aggregated_forecasts_for_questions:

Caller	include_cp_history	What it does with history
posts/serializers.py:532 (feed + question detail)	driven by request param, default true on FE detail pages	drives the CP history chart on the question page
notifications/services.py:322 ("similar posts" notif)	False	latest CP only, binary questions
questions/services/movement.py:82 (calculate_period_movement_for_questions)	True	walks history to pick first_agg (at period start) and last_agg (most recent in past)

What actually happens today:

get_last_aggregated_forecasts_for_questions is the single source of truth for the feed path — it already applies the actual_close_time filter at the DB layer. The filter in _get_latest_aggregate_forecast isn't a duplicate of the feed path; it exists purely for the history path: when include_cp_history=True, the outer function unions in the full history without the cap, so the serializer has to pick the "latest" from a list that can contain post-close rows. That per-list filter is what keeps the serializer safe against being handed unfiltered history.

If we pushed the cap into the outer get_aggregated_forecasts_for_questions history branch too:

• ✅ The _get_latest_aggregate_forecast in-memory filter could go away (it'd just take forecasts[-1]).
• ✅ Perf is neutral to slightly better — same filter shape, same join to question, potentially fewer rows on resolved questions with many post-close aggregations.
• ⚠️ CP history chart change: on retroactively-resolved questions, post-close aggregations would disappear from the history array. Arguably desirable (post-resolution CP is meaningless), but this PR's docstring explicitly states "Full history is still serialized; only the 'latest' preview is capped." That's a product/UX call, not a refactor.
• ⚠️ Movement calc: calculate_period_movement_for_questions only runs on questions closed within the last 7 days. Its last_agg = most-recent-in-past. For a retroactively-resolved question, capping history changes last_agg from the post-close aggregation to the at-close one — probably more correct, but a behaviour change to a cached calc.
• ➖ Notifications ("similar posts"): unaffected (already include_cp_history=False, so it goes through the already-filtered get_last_aggregated_forecasts_for_questions).

Recommendation: ship this PR as-is. If y'all decide post-resolution CP points shouldn't render in the history chart either, a small follow-up can:

1. Move the actual_close_time cap into get_aggregated_forecasts_for_questions's history branch (lines 458–465 in forecasts.py).
2. Delete the in-memory filter from _get_latest_aggregate_forecast (it becomes forecasts[-1]).
3. Update the retroactive-resolution test to assert 1 history entry instead of 3.
4. Re-verify the movement path in movement.py:82 — should still be correct, but worth an explicit test.

• Review changed files in the PR
• Investigate how get_aggregated_forecasts_for_questions is used
• Assess feasibility and DB performance implications
• Post review + answer to the question
  | Branch: claude/issue-5053-sb5kii