-
Notifications
You must be signed in to change notification settings - Fork 22
docs!: typos, types #758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs!: typos, types #758
Conversation
📝 WalkthroughWalkthroughAdds exported Archimedea types and renames Changes
Sequence Diagram(s)(omitted — changes are model/type updates, parsing tweaks and CI edits; no new multi-component control flow to visualize) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/models/Kuva.ts (1)
51-63: Consider removing redundant property assignments.Lines 61-62 explicitly set
archwingandsharkwingwith defaults, but line 55 already spreads...mission.solnodedata, which includes these properties. While the explicit assignments work correctly (overriding the spread), they're redundant.🔎 Proposed simplification
If
solnodedataalways includesarchwingandsharkwing, you can rely on the spread and remove the explicit assignments:const p: ExternalMission = { id: '', activation: new Date(mission.start), expiry: new Date(mission.end), ...mission.solnodedata, node: node(mission.solnode, locale), nodeKey: node(mission.solnode, 'en'), type: nodeMissionType(mission.solnode, locale), typeKey: nodeMissionType(mission.solnode, 'en'), expired: false, - archwing: mission.solnodedata?.archwing ?? false, - sharkwing: mission.solnodedata?.sharkwing ?? false, };However, if
solnodedatamight not include these fields, keep the explicit assignments to ensure the interface contract is met.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignorelib/models/Archimedea.tslib/models/Kuva.tslib/supporting/ExternalMission.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-16T02:24:30.860Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-worldstate-parser PR: 702
File: lib/models/Archidemea.ts:74-90
Timestamp: 2025-10-16T02:24:30.860Z
Learning: In Warframe's Archimedea/Conquest data structure (lib/models/Archidemea.ts), Digital Extremes duplicates difficulties when adding hard tier risks. The first risk in higher difficulties (e.g., CD_HARD) is always identical to the one in the normal difficulty (CD_NORMAL), so using `slice(1)` to skip the first risk when processing additional difficulties is the correct approach to avoid duplicates.
Applied to files:
lib/models/Archimedea.ts
🧬 Code graph analysis (1)
lib/models/Kuva.ts (1)
lib/supporting/ExternalMission.ts (1)
ExternalMission(4-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test (20)
- GitHub Check: Coverage
🔇 Additional comments (8)
.gitignore (1)
51-52: LGTM!Adding
docs/to.gitignorealigns with the PR objective to fix documentation build issues.lib/models/Kuva.ts (3)
44-47: LGTM!Adding explicit type annotations for the
parsedobject improves type safety and code clarity.
78-80: LGTM!Using optional chaining for
mission.missiontypeis a defensive improvement that prevents runtime errors if the property is undefined.
82-82: LGTM!The type casting is necessary to satisfy the
scrubfunction signature that expectsRecord<string, unknown>.lib/models/Archimedea.ts (3)
26-26: LGTM! Excellent improvements to type exports and naming.The changes export previously internal types and fix the typo (
diviation→deviation), improving both the public API surface and code correctness.Also applies to: 28-32, 34-39, 41-45
65-65: LGTM!Improving the parameter documentation from "locale" to "Locale" enhances clarity by indicating the type.
59-59: No action needed — the property rename fromdiviationtodeviationis complete.Verification confirms there are no remaining references to the old property name
diviationin the codebase. All instances at lines 59, 75 correctly use the new spellingdeviation, and the breaking change has been properly addressed.lib/supporting/ExternalMission.ts (1)
30-30: Review concern about makingenemyoptional is valid, but requires clarification on data source reliability.Making
enemya breaking change for external consumers is a legitimate concern from a TypeScript perspective. However, the internal codebase contains no direct access to this property. The inconsistency lies in the data flow:SolnodeData.enemyis defined as required (enemy: string), but it's spread intoExternalMissionwhere it's marked optional.Additionally, similar properties like
archwingandsharkwinginclude defensive null-coalescing (??) in the parse function (lines 61-62), butenemylacks equivalent protection despite being optional. Verify that:
- The source API (10o.io) reliably provides the
enemyfield insolnodedata, or- Add defensive handling for
enemymatching the pattern used for other properties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yaml.github/workflows/docs.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test (20)
- GitHub Check: Coverage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/models/Kuva.ts (1)
66-66: Logic is inverted forexpiredflag.The expression
Date.now() - p.expiry.getTime() < 0evaluates totruewhen the mission has NOT expired yet (whennow < expiry). The conditional at lines 68-70 confirms this:p.activation < now && now < p.expirychecks whether the mission is currently active, indicating thatnow < p.expirymeans the mission is not expired. Theexpiredfield should betruewhennow > expiry.Fix
- p.expired = Date.now() - p.expiry.getTime() < 0; + p.expired = Date.now() > p.expiry.getTime();
🧹 Nitpick comments (1)
lib/models/Kuva.ts (1)
44-47: Type mismatch:arbitrationinitialized as empty object but typed asExternalMission.The type annotation indicates
arbitrationshould be anExternalMission, but it's initialized as{}and cast. If no arbitration mission is found in the data, this empty object will be returned, which doesn't satisfy theExternalMissioninterface (missing required fields likeid,activation,expiry, etc.).Consider using
undefinedornullfor uninitialized state:🔎 Suggested fix
- const parsed: { kuva: ExternalMission[]; arbitration: ExternalMission } = { + const parsed: { kuva: ExternalMission[]; arbitration: ExternalMission | undefined } = { kuva: [], - arbitration: {} as ExternalMission, + arbitration: undefined, };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/ci.yaml.github/workflows/docs.yaml.gitignorelib/models/Archimedea.tslib/models/Kuva.tslib/supporting/ExternalMission.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- lib/supporting/ExternalMission.ts
- .github/workflows/docs.yaml
- .github/workflows/ci.yaml
- .gitignore
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-16T02:24:30.860Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-worldstate-parser PR: 702
File: lib/models/Archidemea.ts:74-90
Timestamp: 2025-10-16T02:24:30.860Z
Learning: In Warframe's Archimedea/Conquest data structure (lib/models/Archidemea.ts), Digital Extremes duplicates difficulties when adding hard tier risks. The first risk in higher difficulties (e.g., CD_HARD) is always identical to the one in the normal difficulty (CD_NORMAL), so using `slice(1)` to skip the first risk when processing additional difficulties is the correct approach to avoid duplicates.
Applied to files:
lib/models/Archimedea.ts
🧬 Code graph analysis (1)
lib/models/Kuva.ts (1)
lib/supporting/ExternalMission.ts (1)
ExternalMission(4-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Coverage
- GitHub Check: Test (20)
🔇 Additional comments (7)
lib/models/Kuva.ts (2)
51-63: LGTM!The
ExternalMissionobject construction is well-structured. The explicitarchwingandsharkwingassignments after the spread operator correctly ensure these fields have boolean values even whensolnodedatais missing or incomplete.
74-81: LGTM!The optional chaining on
mission.missiontype?.startsWithadds resilience against undefined values. The separation of arbitration vs. kuva mission handling within the same time-window check is clean and logical.lib/models/Archimedea.ts (5)
26-32: LGTM!The newly exported
Difficultytype andRawArchimedeaMissioninterface provide clear typing for the raw mission data structure. This improves API discoverability for consumers.
34-45: LGTM!Clear separation between
ArchimedeaMissionDifficultyRisk(withisHardflag) andArchimedeaMissionDifficulty(without). Exporting these interfaces enables type-safe consumption by library users.
59-65: Good typo fix and type alignment.The rename from
diviationtodeviationcorrects the spelling. Property types now reference the newly exported interfaces, ensuring consistency.
75-79: LGTM!The
deviationassignment correctly uses the renamed property and populates it with the properly typed structure.
88-97: LGTM!The
slice(1)usage correctly handles DE's data structure where the first risk in higher difficulties duplicates the normal difficulty's risk. Based on learnings, this is the intended approach to avoid duplicates.
What did you fix?
fixes build issue on docs
Reproduction steps
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.