Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
github-actions
bot
added
package:skyhelper
Contributor
Review Summary by QodoAdd calculator command for candles with modal interface WalkthroughsDescription• Add calculator command for regular, seasonal, and ascended candles • Implement modal-based calculator with daily/weekly reward tracking • Add shard utility methods for date range queries • Update planner data schema with dailies/event/season checkin fields • Pin discord-api-types to version 0.38.38 across packages Diagramflowchart LR
CMD["Calculator Command"]
MODAL["Modal Input<br/>current/target candles"]
CALC["Calculate Days<br/>Needed"]
RESULT["Display Results<br/>with scenarios"]
CMD -->|launch| MODAL
MODAL -->|submit| CALC
CALC -->|process| RESULT
SHARDS["Shard Utils<br/>getNextShard"]
DATES["Date Utils<br/>getDatesBetween"]
CALC -->|query| SHARDS
CALC -->|iterate| DATES
File Changes1. packages/skyhelper/src/bot/handlers/calculator.ts
|
Contributor
Code Review by Qodo
1. sc case leaves component undefined
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+81
to
+86
| case "sc": { | ||
| break; | ||
| } | ||
| } | ||
| await helper.editReply({ components: [component], flags: MessageFlags.IsComponentsV2 }); | ||
| } |
Contributor
There was a problem hiding this comment.
1. sc case leaves component undefined 📘 Rule violation ⛯ Reliability
The sc branch in handleCalculatorModal() does not assign component, but the code always uses it in editReply(), which can cause a runtime failure for Seasonal Candle calculations. This violates the requirement to handle edge cases and failure paths gracefully.
Agent Prompt
## Issue description
`handleCalculatorModal()` can hit the `sc` switch branch without assigning `component`, but still calls `editReply({ components: [component] })`, which can crash or send an invalid response.
## Issue Context
The calculator command allows selecting Seasonal Candles (`sc`), and the modal custom id can include `type="sc"`. The handler must either build a valid component for `sc` or exit early with a user-safe message.
## Fix Focus Areas
- packages/skyhelper/src/bot/handlers/calculator.ts[19-86]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+15
to
+26
| const [_, type = "c", guid = ""] = int.data.custom_id.split(";"); | ||
| const current = client.utils.getModalComponent(int, "input_have", ComponentType.TextInput, true).value; | ||
| const checkboxes = client.utils.getModalComponent(int, "checkboxes", ComponentType.CheckboxGroupAction)?.values; | ||
|
|
||
| let component: APIContainerComponent; | ||
| switch (type) { | ||
| case "c": { | ||
| const target = client.utils.getModalComponent(int, "input_need", ComponentType.TextInput, true).value; | ||
|
|
||
| const { daysWithBase, daysWithBaseQuests, daysWithMax, daysWithMaxQuests } = | ||
| calculateCandles(Number(current), Number(target)) ?? {}; | ||
|
|
Contributor
There was a problem hiding this comment.
2. Modal numbers parsed without validation 📘 Rule violation ⛨ Security
User-provided modal text input values are converted with Number(...) and used without validating NaN, non-integers, or negative/boundary values. This can produce incorrect outputs (e.g., NaN, misleading day counts) instead of a clear, safe error response.
Agent Prompt
## Issue description
Modal text input values are treated as numbers via `Number(...)` without validating that they are finite, non-negative, and within expected ranges. Invalid inputs (empty string, non-numeric, negative, very large) can yield `NaN`/nonsensical results instead of a controlled error.
## Issue Context
These values come from Discord modal text inputs (user-controlled). Compliance requires explicit input validation and edge-case handling.
## Fix Focus Areas
- packages/skyhelper/src/bot/handlers/calculator.ts[15-26]
- packages/skyhelper/src/bot/handlers/calculator.ts[42-59]
- packages/skyhelper/src/bot/handlers/calculator.ts[88-102]
- packages/skyhelper/src/bot/handlers/calculator.ts[116-223]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+15
to
+17
| const [_, type = "c", guid = ""] = int.data.custom_id.split(";"); | ||
| const current = client.utils.getModalComponent(int, "input_have", ComponentType.TextInput, true).value; | ||
| const checkboxes = client.utils.getModalComponent(int, "checkboxes", ComponentType.CheckboxGroupAction)?.values; |
Contributor
There was a problem hiding this comment.
3. Checkbox type mismatch 🐞 Bug ✓ Correctness
The handler reads checkbox values using ComponentType.CheckboxGroupAction, but the modal is built with ComponentType.CheckboxGroup; getModalComponent enforces exact type matching and will throw if these differ.
Agent Prompt
### Issue description
The calculator modal defines the checkboxes component as `ComponentType.CheckboxGroup`, but the submit handler tries to read it as `ComponentType.CheckboxGroupAction`. `getModalComponent` throws on mismatched types.
### Issue Context
`Utils.getModalComponent` checks `comp.type !== type` and throws when they differ.
### Fix Focus Areas
- packages/skyhelper/src/bot/handlers/calculator.ts[15-17]
- packages/skyhelper/src/bot/modules/inputCommands/utility/calculator.ts[71-79]
- packages/skyhelper/src/bot/utils/classes/Utils.ts[214-227]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+134
to
+141
| const shard = ShardsUtil.getShard(DateTime.now().setZone(zone)); | ||
| if (shard && shard.type === "red") { | ||
| checkboxes.push({ | ||
| label: "Have you cleared today's shard?", | ||
| value: "today_shard", | ||
| default: PlannerDataService.shardsCleared(settings.plannerData), | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
4. Shard checkbox unreachable 🐞 Bug ✓ Correctness
The AC modal’s “today’s shard” checkbox never appears because it checks shard.type, but
ShardsUtil.getShard() returns { info, timings } with the type nested under info.
Agent Prompt
### Issue description
AC checkbox rendering checks `shard.type`, but `ShardsUtil.getShard()` returns `{ info, timings }`. This makes the “today_shard” option unreachable.
### Issue Context
`type` lives under `shard.info.type`.
### Fix Focus Areas
- packages/skyhelper/src/bot/modules/inputCommands/utility/calculator.ts[132-141]
- packages/utils/src/classes/shardsUtil.ts[131-138]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+110
to
+121
| // eslint-disable-next-line | ||
| const showDailies = (activeSeason && type === "sc") || (type === "c" && !activeSeason); | ||
| if (showDailies) { | ||
| checkboxes.push({ | ||
| label: "Did you complete your dailies today?", | ||
| value: "dailies", | ||
| default: PlannerDataService.hasDoneDailies( | ||
| settings.plannerData, | ||
| activeSeason?.guid ?? "", | ||
| type === "c" ? "season" : "dailies", | ||
| ), | ||
| }); |
Contributor
There was a problem hiding this comment.
5. Wrong dailies checkin 🐞 Bug ✓ Correctness
The dailies checkbox default uses the wrong checkin type: regular candles (no season) query season.checkin, while seasonal candles query dailies.checkin, so defaults will be incorrect.
Agent Prompt
### Issue description
The calculator dailies checkbox default reads from the wrong planner checkin key, producing incorrect default state.
### Issue Context
`UserPlannerData` defines different storage for season dailies vs non-season dailies.
### Fix Focus Areas
- packages/skyhelper/src/bot/modules/inputCommands/utility/calculator.ts[110-121]
- packages/skyhelper/src/bot/handlers/planner/helpers/data.service.ts[359-363]
- packages/skyhelper/src/bot/handlers/planner/helpers/data.service.ts[453-460]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation 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.
This depends on unreleased modal comps, so will have to wait for that