Skip to content

Conversation

@TastyPumPum
Copy link
Contributor

@TastyPumPum TastyPumPum commented Dec 31, 2025

Enhances the farming system with auto-farming capabilities, preference settings, and comprehensive testing.

Auto-Farming Features:

  • Introduces autoFarm() in autoFarm.ts for automated farming tasks.
  • Adds handleCombinedAutoFarm() in combinedAutoFarmActivity.ts for aggregating farming steps.
  • Implements executeFarmingStep() in farmingStep.ts for detailed step execution.

Preferences and Configurations:

  • Adds per-patch seed preferences in farming.ts.
  • Updates config.ts to include toggles for auto-farm features.
  • Database and Models:
  • Adds minion_farmingPreferContract and minion_farmingPreferredSeeds fields in prisma/schema.prisma.

Testing:

  • Adds integration tests in autoFarmTask.test.ts.

  • Adds unit tests in autoFarm.test.ts and runCommand.test.ts.

  • I have tested all my changes thoroughly.


Important

Enhances the farming system with auto-farming capabilities, preference settings, and comprehensive testing.

  • Auto-Farming Features:
    • Adds autoFarm() in autoFarm.ts for automated farming tasks.
    • Implements handleCombinedAutoFarm() in combinedAutoFarmActivity.ts for aggregating farming steps.
    • Introduces executeFarmingStep() in farmingStep.ts for detailed step execution.
  • Preferences and Configurations:
    • Adds per-patch seed preferences in farming.ts.
    • Updates config.ts to include toggles for auto-farm features.
    • Adds minion_farmingPreferredContract and minion_farmingPreferredSeeds fields in prisma/schema.prisma.
  • Testing:
    • Adds integration tests in autoFarmTask.test.ts.
    • Adds unit tests in autoFarm.test.ts, runCommand.test.ts, and other files.
  • Misc:
    • Updates minionStatus() in minionStatus.ts to reflect new auto-farming status.
    • Refactors runCommand() in settings.ts to handle new command logic.

This description was created by Ellipsis for 3eebb50. You can customize this summary. It will automatically update as commits are pushed.

@TastyPumPum
Copy link
Contributor Author

@ellipsis-dev review this pr

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed everything up to 06f8cfe in 4 minutes and 10 seconds. Click for details.
  • Reviewed 6055 lines of code in 45 files
  • Skipped 0 files when reviewing.
  • Skipped posting 9 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. data/authors.json:2
  • Draft comment:
    In the entry for "getting-started/changelog.mdx", the author name "alexandermenteer" is all lowercase, unlike the other names. Please confirm if the casing is intentional or if this is a typographical error.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to confirm if the casing of a name is intentional or a typographical error. This falls under asking the author to confirm their intention, which is against the rules.
2. data/authors.json:17
  • Draft comment:
    The author name "Porfet" in the "osb/Skills/farming/README.md" entry appears to be a possible typo. Did you intend to write "Profit" or use another spelling?
  • Reason this comment was not posted:
    Comment was on unchanged code.
3. packages/robochimp/prisma/bso_schema.prisma:367
  • Draft comment:
    Typo detected: The field 'minion_farmingPreferredContract' is mapped to 'minion.farmingPreferContract'. Should the mapped value be 'minion.farmingPreferredContract' to match the field name? Please verify.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The comment is about a change in the diff (line 367 is a new line being added). There's a clear inconsistency: the field name has "Preferred" but the mapping drops it to "Prefer". Looking at line 368, the next field keeps "Preferred" in both the field name and mapping. This could be intentional (maybe the database column is named differently), but it's unusual and worth flagging. However, the comment asks the author to "verify" rather than stating definitively that it's wrong. According to the rules, comments that ask to verify or confirm are not useful - we should only comment if there's definitely an issue. The mapping could be intentional - perhaps the database column was already created with the shorter name "farmingPreferContract" and the developer is just mapping to the existing column. Without seeing the actual database schema or migration files, I can't be 100% sure this is a mistake. The comment is phrased as "Please verify" which violates the rule about not asking authors to confirm their intentions. While the phrasing asks for verification, the comment does identify a genuine inconsistency between the field name and its mapping, especially when compared to the very next line which maintains consistency. This is a concrete observation about the code, not pure speculation. However, the "Please verify" framing does make it fall into the category of asking the author to confirm rather than stating a definite issue. The comment identifies a real inconsistency in naming, but frames it as a request for verification rather than a definite issue. Given the rule against asking authors to verify or confirm, and the possibility that this mapping is intentional, the comment should be deleted.
4. prisma/schema.prisma:308
  • Draft comment:
    Typo in the mapping: the field 'minion_farmingPreferredContract' is mapped to 'minion.farmingPreferContract'. Consider changing it to 'minion.farmingPreferredContract' for consistency with the field name.
  • Reason this comment was not posted:
    Marked as duplicate.
5. src/lib/minions/functions/farmingTripHelpers.ts:96
  • Draft comment:
    The error message 'There are no available patches to you.' is a bit awkward. Consider rephrasing it to 'There are no patches available for you.' for clarity.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment is about a very minor wording preference in an error message. Both phrasings are grammatically correct and convey the same meaning clearly. The rules state "Do NOT make comments that are obvious or unimportant" and "Do NOT comment unless there is clearly a code change required." This is not a code change that's required - it's a subjective preference about wording. The original message is perfectly clear and understandable. Additionally, if this were truly important, the comment should address both instances (lines 96 and 110), but it only addresses one. This suggests it's not a significant issue. Could there be a legitimate reason to prefer one phrasing over the other, such as consistency with other error messages in the codebase? Perhaps the suggested wording is more natural or follows a style guide I'm not aware of. Even if there were a style guide preference, without evidence of that in the context provided, this remains a subjective wording preference. The comment doesn't cite any style guide or consistency issue. Both messages are clear and functional, making this an unimportant change that doesn't meet the threshold for a required code change. This comment should be deleted. It's a minor stylistic preference about error message wording that doesn't represent a required code change. Both phrasings are clear and correct, making this an unimportant comment.
6. src/lib/minions/functions/farmingTripHelpers.ts:155
  • Draft comment:
    Typo detected: 'canCompostandPay' might be intended to be 'canCompostAndPay' (camelCase consistency). Please verify the correct property name.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is asking the author to "verify" the property name, which violates the rule "Do NOT ask the PR author to confirm their intention, to explain, to double-check things, to ensure the behavior is intended". The comment is also speculative - it says "might be intended" which indicates uncertainty. Without seeing the Plant type definition, I cannot confirm this is actually a typo. If the property is correctly named canCompostandPay in the Plant type, then this code is correct. If it's a typo, TypeScript would catch it at compile time (build error), which falls under "Do NOT comment on anything that would be obviously caught by the build". This comment lacks strong evidence and is asking for verification rather than pointing out a definite issue. However, if this is genuinely a typo that exists in the type definition itself and was propagated here, it could be a legitimate code quality issue worth fixing. The naming inconsistency could indicate a broader problem in the codebase. Even if it's a typo in the type definition, without seeing that definition, I cannot confirm this is an issue. The rule states I must see "STRONG EVIDENCE that the comment is correct" to keep it. The comment itself is phrased as asking for verification ("Please verify"), which is explicitly against the rules. If the property name is wrong, TypeScript will catch it at build time anyway. This comment should be deleted. It asks the author to verify something rather than stating a definite issue, it's speculative ("might be intended"), and any actual typo would be caught by TypeScript at build time. There's no strong evidence this is a real issue.
7. src/mahoji/lib/abstracted_commands/farmingCommand.ts:149
  • Draft comment:
    Typo: Consider rephrasing "There are no available patches to you." to "There are no patches available to you." for clarity.
  • Reason this comment was not posted:
    Comment was on unchanged code.
8. tests/unit/calcFarmingContracts.test.ts:77
  • Draft comment:
    Typo: In the test description on line 77, "throws when there is no suitable plant for the players level", consider using "player's level" to be grammatically correct.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% This is a minor grammatical correction in a test description string. While technically correct, I need to consider if this violates the rule about "Do NOT make comments that are obvious or unimportant." A typo in a test description is quite minor and doesn't affect functionality. The rules emphasize not making comments unless there is "clearly a code change required" and to avoid "obvious or unimportant" comments. A missing apostrophe in a test description is arguably unimportant - it doesn't affect the test's functionality, and test descriptions are primarily for developer understanding. This seems like the kind of nitpicky comment that would be considered "obvious or unimportant." However, one could argue that clear, grammatically correct test descriptions improve code quality and maintainability. The comment is actionable and specific. It's not speculative or asking for confirmation - it's a straightforward fix. Some teams do care about proper grammar in their codebase, including test descriptions. While some teams care about grammar, the rules explicitly state to avoid "obvious or unimportant" comments. A missing apostrophe in a test description is minor enough that it falls into the "unimportant" category. The test is perfectly understandable as-is, and this doesn't represent a meaningful code quality issue that needs to be addressed. This comment should be deleted. It's a minor grammatical correction that falls under "obvious or unimportant" comments. The test description is perfectly understandable without the apostrophe, and this doesn't represent a meaningful code quality issue.
9. tests/unit/calcsFarming.test.ts:157
  • Draft comment:
    Typo: The test description refers to "limpwort plants" but the plant name is set to "Limpwurt". Consider correcting for consistency.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% This comment is about a very minor inconsistency - "limpwort" vs "Limpwurt". The test description uses lowercase "limpwort" while the plant name uses capitalized "Limpwurt". This is purely cosmetic and doesn't affect functionality. The rules state not to make comments that are "obvious or unimportant" and to only comment when "there is clearly a code change required." A minor spelling/capitalization inconsistency in a test description doesn't clearly require a code change - the test works fine as is. This seems like an overly pedantic comment that doesn't add real value. However, consistency in naming could be important for searchability and clarity. If "Limpwurt" is the correct canonical name in the game (Old School RuneScape), then the test description should match it for accuracy. While consistency has some value, this is still a very minor issue in a test description that doesn't affect code functionality. The rules explicitly say not to make comments that are "obvious or unimportant," and a capitalization difference in a test description falls into that category. The test is clear and functional as written. This comment should be deleted. It's pointing out a very minor cosmetic inconsistency that doesn't affect functionality and falls under "obvious or unimportant" comments that should be avoided per the rules.

Workflow ID: wflow_MgrOGkfIWHlYkeI3

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@TastyPumPum
Copy link
Contributor Author

@ellipsis-dev review this pr

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed everything up to 3eebb50 in 5 minutes and 22 seconds. Click for details.
  • Reviewed 7323 lines of code in 49 files
  • Skipped 0 files when reviewing.
  • Skipped posting 14 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/tasks/minions/farmingStep.ts:668
  • Draft comment:
    The code hard-codes the crop name 'Limpwurt'. Extract this magic string to a constant to improve maintainability and avoid typos.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a code quality/refactoring suggestion. The comment suggests extracting a magic string to a constant. However, looking at the broader context, the codebase consistently uses string literals for plant names throughout this file ('hespori', 'seaweed', 'bush', 'redwood', etc.). The string 'Limpwurt' only appears once, so extracting it to a constant wouldn't provide much benefit. The rules state "Comments that suggest code quality refactors are good! But only if they are actionable and clear." However, this seems like a minor suggestion that doesn't significantly improve maintainability, especially given the pattern used elsewhere in the file. The rules also say to remove comments that are "obvious or unimportant" and to require "STRONG EVIDENCE that the comment is correct." The suggestion is technically valid - extracting magic strings is generally good practice. However, the codebase pattern shows consistent use of string literals for plant names throughout. Extracting just this one string would be inconsistent with the rest of the file's approach. The comment doesn't acknowledge this broader pattern. While extracting magic strings can be good practice, this comment doesn't consider the existing codebase patterns. If this were to be done, it should be done consistently for all plant name strings in the file, not just 'Limpwurt'. The comment is too narrow in scope and doesn't provide enough value as a standalone suggestion. This falls into the category of minor/unimportant refactoring suggestions. This comment should be deleted. It's a minor code quality suggestion that doesn't meet the threshold for keeping. The codebase consistently uses string literals for plant names throughout, and singling out 'Limpwurt' for extraction would be inconsistent. This is not an important or actionable enough suggestion.
2. src/tasks/minions/farmingStep.ts:401
  • Draft comment:
    handleTreeRemoval() computes coins owed using Math.max(0, gpToCutTree - prePaid). Verify that the rounding logic meets your design in low-GP scenarios.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify the rounding logic in a specific scenario. This falls under asking the author to double-check or ensure behavior, which is against the rules. Therefore, this comment should be removed.
3. tests/unit/userutil.ts:100
  • Draft comment:
    serializeSnapshotItem() mutates its input 'item' directly. Consider copying the object before modifying it to prevent unintended side effects.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. src/mahoji/commands/farming.ts:367
  • Draft comment:
    In the 'set_preferred' subcommand, providing a seed input without a patch returns an error; ensure the UX instructs users to specify a patch.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
5. tests/unit/runCommand.test.ts:111
  • Draft comment:
    Test asserts that 'ignoreUserIsBusy' is undefined for normal invocations. Confirm that downstream logic correctly handles undefined instead of false.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
6. data/authors.json:17
  • Draft comment:
    There appears to be a potential typographical error in the author name 'Porfet'. Double-check if it should be 'Prophet'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
7. data/osb/skills/farming-plants.json:68
  • Draft comment:
    Typo fixed: The property name was changed from 'can_compostand_pay' to 'can_compost_and_pay'.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, as it only states that a typo was fixed. It does not provide any actionable feedback or suggestions for improvement.
8. src/lib/skilling/skills/farming/herbPlants.ts:119
  • Draft comment:
    Typographical correction: The variable name was updated from 'canCompostandPay' to 'canCompostAndPay', which resolves the camelCasing inconsistency.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, pointing out a typographical correction that resolves a camelCasing inconsistency. It does not provide a suggestion, ask for confirmation, or highlight a potential issue. Therefore, it violates the rule against making purely informative comments.
9. src/lib/skilling/skills/farming/specialPlants.ts:74
  • Draft comment:
    Potential typographical error: In the comment, "Etceteria patch (1)" might be a misspelling. Verify if it should be "Etcetera patch (1)".
  • Reason this comment was not posted:
    Comment was on unchanged code.
10. src/lib/skilling/skills/farming/specialPlants.ts:149
  • Draft comment:
    Typo fixed: The property name "canCompostandPay" has been correctly changed to "canCompostAndPay", matching the camelCase convention.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, as it only acknowledges a change that was made without providing any actionable feedback or suggestions. It does not ask for confirmation or suggest improvements.
11. src/lib/skilling/skills/farming/specialPlants.ts:414
  • Draft comment:
    Typo fix: The property name was corrected from canCompostandPay to canCompostAndPay. Please ensure that this change is consistent with the rest of the codebase.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is informative and asks the author to ensure consistency across the codebase, which violates the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
12. src/lib/skilling/skills/farming/trees.ts:159
  • Draft comment:
    Typographical error: The property was renamed from 'canCompostandPay' to 'canCompostAndPay'. This fixes the casing to match the camelCase convention.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, pointing out a typographical error that was corrected. It doesn't suggest any action or ask for confirmation, which violates the rule against purely informative comments.
13. src/lib/skilling/types.ts:253
  • Draft comment:
    Typo/consistency check: The new property "logDepletionChance" uses a singular "log" while the related properties (e.g., "outputLogs" and "outputLogsQuantity") use the plural form. Please verify if this was intentional or if it should be renamed to "logsDepletionChance" for consistency.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment is explicitly asking the author to "verify if this was intentional" which directly violates the rule "Do NOT ask the PR author to confirm their intention, to explain, to double-check things, to ensure the behavior is intended". The comment is not stating that there IS an inconsistency that needs to be fixed - it's asking the author to check if it's intentional. This is a speculative comment about naming consistency, not a definitive issue. The naming could be intentional (e.g., "log depletion chance" as a singular concept vs "output logs" as a plural quantity). Without strong evidence that this is wrong, this comment should be deleted. Could there be a legitimate naming inconsistency here that should be fixed? Perhaps the codebase has strict naming conventions that require consistency between related properties, and this is a clear violation of that convention. Even if there were strict naming conventions, the comment is phrased as a request for verification ("Please verify if this was intentional") rather than a clear statement that something needs to be fixed. This type of comment explicitly violates the rules. If it were a real issue, the comment should state definitively that it needs to be changed, not ask the author to verify. This comment should be deleted because it asks the author to verify their intention rather than pointing out a definitive issue. It's a speculative comment about naming consistency that violates the rule against asking authors to confirm, explain, or double-check things.
14. src/mahoji/lib/abstracted_commands/farmingCommand.ts:63
  • Draft comment:
    Typographical issue: The string 'Ring of endurance' might be a typo in terms of capitalization. Typically, it appears as 'Ring of Endurance'. Please confirm which version is correct and adjust accordingly.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_Lag9JuUPeVZbWthO

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant