fix(quests): resolve Mission star rollups per course, not across joined courses#7772
Merged
Conversation
…ed courses (Closes #7771)
📝 WalkthroughWalkthroughQuest progression now stores mission rollups per course, scopes star summaries and course-panel progress to the active course, and preserves cross-quest gradient accumulation. Course identity is added to outlines, with expanded resolver, loader, UI, documentation, and test coverage. ChangesPer-course quest progression
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CourseOutline
participant ProgressionResolver
participant ObjectivesLoader
participant CourseObjectivesView
CourseOutline->>ProgressionResolver: provide courseId and mission outline
ProgressionResolver->>ProgressionResolver: build per-course mission rollup
ObjectivesLoader->>ProgressionResolver: request course-scoped stars and mission progress
ProgressionResolver-->>ObjectivesLoader: return resolved course data
ObjectivesLoader-->>CourseObjectivesView: expose progress for the active course
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 @.github/instructions/quests.instructions.md:
- Line 17: Update the sentence describing consumer behavior in the quest anchor
documentation to replace “consumers preference” with the grammatically correct
wording “consumers prefer still-unsatisfied Missions,” without changing the
surrounding meaning.
🪄 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 Plus
Run ID: 946cdc0e-b9b2-418b-8c0c-dc82cf62c49c
📒 Files selected for processing (13)
.github/instructions/quests.instructions.mdlib/features/quests/lo_progression.dartlib/features/quests/quest_objectives_loader.dartlib/features/quests/quest_progression_resolver.dartlib/features/quests/repo/quest_repo.dartlib/routes/courses/course_objectives/course_objectives_view.dartlib/routes/world/joined_objective_cache.darttest/pangea/joined_objective_cache_test.darttest/pangea/quest_progression_resolver_test.darttest/pangea/quest_star_summary_test.darttest/pangea/quests/per_course_mission_rollup_test.darttest/pangea/quests/quest_outline_restriction_test.darttest/pangea/world_map_ranking_test.dart
| - **The per-Mission star rollup** — a **star** is one orchestrator-awarded activity goal, read from awarded-goal state on the learner's own session rooms. Per activity, the learner's stars are their **best single session** (the most goals awarded to them in any one session of it — repeat sessions do not accumulate); the Mission total sums those per-activity bests across the Mission's distinct activities. Both rules are the org doc's satisfaction model. No server-side progression endpoint is needed: every session that earned a star is a room the client can read. (Same collectible pattern as vocab/grammar — see [analytics-system.instructions.md](analytics-system.instructions.md).) | ||
|
|
||
| From those, the resolver finds each quest's **anchor (next) Mission**: the **first Mission in quest order whose star total is below the satisfaction threshold**; once every Mission is satisfied, the anchor falls back to the **lowest-star Mission**, so a completed quest keeps pointing at the learner's weakest area instead of going silent. When several quests are in scope it yields an anchor **per quest** plus the global per-Mission star totals; consumers preference still-unsatisfied Missions and **accumulate** across quests (so an activity advancing several quests' unfinished Missions ranks higher) — the resolver just supplies the anchors and totals, the weighting lives in the consumer (see the [world map](world-map.instructions.md) Priority matrix). | ||
| From those, the resolver finds each quest's **anchor (next) Mission**: the **first Mission in quest order whose star total is below the satisfaction threshold**; once every Mission is satisfied, the anchor falls back to the **lowest-star Mission**, so a completed quest keeps pointing at the learner's weakest area instead of going silent. When several quests are in scope it yields, **per quest**, an anchor and that quest's own per-Mission star totals; consumers preference still-unsatisfied Missions and **accumulate** across quests (so an activity advancing several quests' unfinished Missions ranks higher) — the resolver just supplies the anchors and totals, the weighting lives in the consumer (see the [world map](world-map.instructions.md) Priority matrix). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the grammatical error in “consumers preference.”
Use “consumers prefer still-unsatisfied Missions” to make the sentence grammatically correct.
🧰 Tools
🪛 LanguageTool
[style] ~17-~17: To elevate your writing, try using more formal phrasing here.
Context: ...st-star Mission**, so a completed quest keeps pointing at the learner's weakest area instead o...
(CONTINUE_TO_VB)
🤖 Prompt for 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.
In @.github/instructions/quests.instructions.md at line 17, Update the sentence
describing consumer behavior in the quest anchor documentation to replace
“consumers preference” with the grammatically correct wording “consumers prefer
still-unsatisfied Missions,” without changing the surrounding meaning.
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Resolve the per-Mission star rollup per course outline instead of unioning every joined course's outline into one global rollup.
Why
Closes #7771
A Mission pinned to a single 4-star activity was displaying
0/8on the course panel. Missions are a shared catalog reused across quests, so a second joined course carrying the same Mission with different activities was being counted into this course's clamp and star total — and unpinned activities re-entered through the union, defeating the pin from #7748.Pre-existing; pinning made it visible. Design and rationale: quests.instructions.md ("One shared resolver", "Star display on the course panel") — updated in this PR to record the per-course scoping rule agreed with Will.
What changed
CourseLoOutlinecarries acourseId;QuestProgresscarries its ownrollup. No cross-quest rollup exists anymore.ProgressionResolution.forCourse(id)is how a per-course surface reads its own numbers;questStarstakes acourseId.missionGradientjudges "satisfied" against each quest's own rollup, so a Mission finished in one course no longer silences it in another. Cross-quest accumulation for the map's band is unchanged.earnableStarsacross outlines is gone — each course now clamps against its own outline's data. Outlines carry the same plan, so they shouldn't disagree.missionGradient's doc comment offquestStars, where it had been misattached.Riding along: unbreak
flutter analyzeon mainSecond commit, unrelated to the quests change, folded in at Will's direction.
code_testswas already failing onmain:flutter analyzeexits non-zero on any issue including infos, andtest/pangea/courses/course_catalog_parsing_test.dart:76trippeduse_null_aware_elements. Confirmed by running analyze on a cleanorigin/mainworktree at the pinned SDK — same single issue, exit 1. It arrived with #7770, which merged withcode_testsalready failing, so every branch cut since then inherited a red gate.Fix is one line:
if (courseId != null) 'course_id': courseId→'course_id': ?courseId. Identical semantics — the entry is omitted when the value is null either way.Testing
Verified on the pinned SDK (Flutter 3.41.4 via fvm, matching CI):
fvm flutter test— full suite, 1304 passed.fvm dart format lib/ test/ --set-exit-if-changed— clean.fvm flutter analyze— No issues found, exit 0 (was exit 1 on main before the ride-along fix).pubspec.lockunchanged.course_catalog_parsing_test.dartrun on its own — 12 passed, so the null-aware rewrite preserved behavior.test/pangea/quests/per_course_mission_rollup_test.dartcovers the scoping rules: own-activity clamp, no cross-course star credit, shared-activity double-count, per-quest anchors, scopedquestStars, and that the map's cross-quest band accumulation still works.quest_progression_resolver_test,quest_star_summary_test,quest_outline_restriction_test,world_map_ranking_test,joined_objective_cache_testfor the scoped API.Deploy Notes
None.