Update fdm-agents dependencies, agent class, and constraints#592
Merged
Conversation
… organic matter balance if constrained
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a crop-specific fertilizer guidance skill and assets, registers a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as User
participant Agent as FertilizerPlannerAgent
participant Tools as PlannerTools
participant FS as SkillAssets
Client->>Agent: Request fertilizer plan
Agent->>Tools: getFarmFields()
Tools-->>Agent: fields (with b_lu_catalogues)
Agent->>Tools: getCropFertilizerGuide(b_lu_catalogues)
Tools->>FS: read assets/crop-index.json & references/*.md
FS-->>Tools: matched guides (concatenated)
Tools-->>Agent: crop guides + matchedFiles
Agent->>Tools: getFarmNutrientAdvice(), getFarmLegalNorms(), searchFertilizers(), simulateFarmPlan(all fields)
Tools-->>Agent: advice, norms, fertilizers, simulationResults
Agent-->>Client: plan (per-field plan + fieldSummary)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 35 minutes and 27 seconds.Comment |
coderabbitai
Bot
changed the title
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
fdm-agents/src/agents/gerrit/agent.ts (1)
38-47: Consolidate duplicated prioritization guidance in the prompt.Line 38 and Line 47 both restate the same NPK-over-OM constraint. Keeping one canonical instruction would reduce prompt bloat and avoid interpretation drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-agents/src/agents/gerrit/agent.ts` around lines 38 - 47, The prompt guidance block contains a duplicated prioritization rule that NPK advice (especially N) takes priority over improving organic matter (OM) — remove the redundant sentence and keep a single canonical statement that NPK-over-OM applies; locate the guidance block that mentions p_type, b_bufferstrip and p_app_method_options (the rules list including application amounts and realistic dates) and delete the repeated NPK-over-OM line, retaining the clearest phrasing, then renumber/adjust surrounding items and any references to that rule so only one authoritative prioritization sentence remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@fdm-agents/src/agents/gerrit/agent.ts`:
- Around line 38-47: The prompt guidance block contains a duplicated
prioritization rule that NPK advice (especially N) takes priority over improving
organic matter (OM) — remove the redundant sentence and keep a single canonical
statement that NPK-over-OM applies; locate the guidance block that mentions
p_type, b_bufferstrip and p_app_method_options (the rules list including
application amounts and realistic dates) and delete the repeated NPK-over-OM
line, retaining the clearest phrasing, then renumber/adjust surrounding items
and any references to that rule so only one authoritative prioritization
sentence remains.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: be842186-81e5-4077-84a3-7e96aec25c2a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.changeset/eleven-moments-relate.mdfdm-agents/package.jsonfdm-agents/src/agents/gerrit/agent.tsfdm-agents/src/tools/fertilizer-planner/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/boomkwekerij.md (1)
56-56: Polish phrasing for clearer English readability.At Line 56, consider changing “yellow old needles” to “yellowing of older needles” (or “yellow older needles”) to avoid awkward adjective order in an otherwise polished reference doc.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/boomkwekerij.md` at line 56, Update the phrasing in the reference sentence "K, Mg and S per advice — Mg deficiency causes the typical yellow old needles." by replacing "yellow old needles" with a clearer wording such as "yellowing of older needles" (or "yellow older needles") so the sentence reads e.g. "K, Mg and S per advice — Mg deficiency causes the typical yellowing of older needles."; locate and edit the exact sentence text in the document (the string "K, Mg and S per advice — Mg deficiency causes the typical yellow old needles.").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/funky-papers-allow.md:
- Line 5: Update the release note sentence "Add skill for crop specific
fertilizer preferences" to use a hyphenated compound adjective by changing it to
"Add skill for crop-specific fertilizer preferences" so the phrase is
grammatically correct; locate the sentence in the .changeset release note (the
line containing "crop specific") and replace it with the hyphenated form.
In `@fdm-agents/src/skills/crop-specific-fertilizer-preferences/SKILL.md`:
- Around line 83-84: The phrase "soil-type dependent" in SKILL.md should be
hyphenated consistently as "soil-type-dependent"; locate the exact text
"soil-type dependent" in the sentence starting "When soil-type dependent
recommendations apply..." and replace it with "soil-type-dependent" to match the
document's compound adjective style.
- Around line 42-44: The doc's rule that agents "pick the subsection whose
catalogue codes match the field" is unsafe for shared codes (e.g.,
aardappelen.md's "consumptie"); update SKILL.md to add an explicit
disambiguation rule: when catalogue codes are shared across subsections, the
agent must also match a secondary context field (e.g., "destination" or "use"
such as frites vs tafel) before selecting a subsection, and if that secondary
context is missing it must either request the field from the user or fall back
to not selecting that ambiguous subsection; mention "consumptie" as an example
and apply the same clarification to the similar sentence at lines 46–48.
In `@fdm-agents/tsdown.config.ts`:
- Line 1: The skills-copy step uses CWD-relative paths causing failures when
builds run outside fdm-agents; update the path construction used with cpSync and
mkdirSync to derive absolute paths from the config file location (use
import.meta.url -> fileURLToPath -> path.dirname or __dirname equivalent) and
then path.join/path.resolve to build both source and destination for the skills
copy, and adjust the cpSync/mkdirSync calls to use those resolved paths (add
required imports from "node:path" and "node:url" if needed).
---
Nitpick comments:
In
`@fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/boomkwekerij.md`:
- Line 56: Update the phrasing in the reference sentence "K, Mg and S per advice
— Mg deficiency causes the typical yellow old needles." by replacing "yellow old
needles" with a clearer wording such as "yellowing of older needles" (or "yellow
older needles") so the sentence reads e.g. "K, Mg and S per advice — Mg
deficiency causes the typical yellowing of older needles."; locate and edit the
exact sentence text in the document (the string "K, Mg and S per advice — Mg
deficiency causes the typical yellow old needles.").
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a2ebc2be-e3a7-4ac8-a038-4b52ead334e2
📒 Files selected for processing (41)
.changeset/funky-papers-allow.mdfdm-agents/src/agents/gerrit/agent.tsfdm-agents/src/skills/crop-specific-fertilizer-preferences/SKILL.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/assets/.gitkeepfdm-agents/src/skills/crop-specific-fertilizer-preferences/assets/crop-index.jsonfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/aardappelen.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/aardperen.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/asperges.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/bloembollen.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/boomkwekerij.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/consumptieaardappelen.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/extensiefgrasland.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/fruitteelt.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/grasland.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/graszaad.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/haver.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/klaver.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/knolselderij.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/koolgewassen.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/koolzaad.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/luzerne.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/mais.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/natuurgrasland.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/peen.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/peulvruchten.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/prei.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/rogge.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/sla-andijvie.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/spinazie.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/suikerbieten.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/uien.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/vlas.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/voederbieten.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/vroegeaardappelen.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/wintergerst-zomergerst.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/wintertarwe-zomertarwe.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/witlofwortel.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/scripts/.gitkeepfdm-agents/src/tools/fertilizer-planner/index.test.tsfdm-agents/src/tools/fertilizer-planner/index.tsfdm-agents/tsdown.config.ts
✅ Files skipped from review due to trivial changes (26)
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/assets/.gitkeep
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/scripts/.gitkeep
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/haver.md
- fdm-agents/src/tools/fertilizer-planner/index.test.ts
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/natuurgrasland.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/witlofwortel.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/rogge.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/koolzaad.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/koolgewassen.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/suikerbieten.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/luzerne.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/prei.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/extensiefgrasland.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/aardperen.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/knolselderij.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/vroegeaardappelen.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/peen.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/voederbieten.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/fruitteelt.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/wintergerst-zomergerst.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/bloembollen.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/vlas.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/spinazie.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/mais.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/uien.md
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/klaver.md
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-agents/src/tools/fertilizer-planner/index.ts
…nerated by Gerrit to gain more insights in the provided plan
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.gerrit.tsx (1)
822-829:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftServer-side
farmTotalscan now disagree with Gerrit’s simulation.This PR updated
simulateFarmPlanto compute legal norms from all eligible farm fields, butcomputePlanMetrics()still recomputes totals from the enriched plan fields only. ReturningserverMetricshere means the UI can show different farm limits than the numbers Gerrit optimized against. Please align the recomputation logic withfdm-agents/src/tools/fertilizer-planner/index.tsbefore overwriting the tool output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.gerrit.tsx around lines 822 - 829, Server-side farmTotals are being recomputed from enrichedPlan via computePlanMetrics and can diverge from Gerrit’s simulation which now computes legal norms from all eligible farm fields in simulateFarmPlan; update the code so serverMetrics uses the same recomputation logic as fdm-agents/src/tools/fertilizer-planner/index.ts (i.e., compute totals from all eligible farm fields, not only enrichedPlan fields) by either (A) changing the call to computePlanMetrics to pass the full eligible-fields list used by simulateFarmPlan or (B) updating computePlanMetrics implementation to mirror the fertilizer-planner logic; ensure you reference computePlanMetrics, simulateFarmPlan, enrichedPlan and serverMetrics and preserve the original tool output’s farmTotals rather than overwriting them with inconsistent recomputed values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/violet-ravens-wonder.md:
- Line 6: Replace the misspelled word "explaination" with the correct spelling
"explanation" in the release note text (the sentence "Add a short field-level
explaination for the fertilizer plan generated by Gerrit to gain more insights
in the provided plan" in .changeset/violet-ravens-wonder.md) so the user-facing
changelog uses the correct term.
In `@fdm-agents/src/tools/fertilizer-planner/index.ts`:
- Around line 1000-1007: The tool currently requires b_lu_catalogues to be
string[] but getFarmFields can return nulls; update the parameters schema for
b_lu_catalogues to accept nullable entries (e.g., array of string | null) and
inside execute (the execute: async (input: { b_lu_catalogues: string[] }) => {
... }) filter out falsy/null values before use (e.g., const cleaned =
input.b_lu_catalogues.filter(Boolean)) so downstream code only receives real
strings; reference the parameters object and the execute handler when making
these changes.
- Around line 692-725: The current mapping over allFarmFields swallows errors in
the try/catch (collectInputForNorms / calculateNormForManure /
calculateNormForPhosphate / calculateNormForNitrogen) by returning null, which
silently removes fields from allFarmFieldNorms and causes incorrect farm-level
totals; instead either let the Promise fail (remove the catch so Promise.all
rejects) or capture and propagate failing field IDs: replace the empty catch
with logic that records f.b_id into a shared failedFields array (or throw a new
Error listing the failed IDs) and ensure the downstream consumer that builds
farmNormsKg checks for failedFields and aborts the simulation or surfaces those
IDs rather than omitting them. Ensure references to allFarmFieldNorms and
farmNormsKg handle the propagated error/failed list accordingly.
In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.gerrit.tsx:
- Around line 177-185: The local variable named escape is shadowing the global
escape and fails lint; rename it (e.g., to isEscaped or escaping) throughout the
helper (the variable declared on Line 178 and all uses in the loop: the if
(escape) check, escape = false assignment, and escape = true assignment) so the
loop that iterates over trimmed with for (const ch of trimmed) no longer uses
the global name and passes lint.
- Around line 736-753: After calling repairTruncatedJson(truncated) and parsing
into parsedPlan, validate that the repaired parsedPlan contains all required
non-buffer fields (the core plan entries you later expand into enrichedPlan)
before proceeding; if any required fields are missing or shortened compared to
expected schema, return dataWithError(null, "Gerrit gaf een ongeldig plan terug.
Probeer het opnieuw.") instead of accepting the partial plan. Locate the block
that slices rawResult (using firstBrace/lastBrace), the repairTruncatedJson
call, and the parsedPlan variable, then add a lightweight schema/field presence
check (e.g., required top-level keys and non-buffer arrays/objects) and fail
closed when validation fails so enrichedPlan and subsequent fertilizer
application logic never operate on a truncated plan.
---
Outside diff comments:
In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.gerrit.tsx:
- Around line 822-829: Server-side farmTotals are being recomputed from
enrichedPlan via computePlanMetrics and can diverge from Gerrit’s simulation
which now computes legal norms from all eligible farm fields in
simulateFarmPlan; update the code so serverMetrics uses the same recomputation
logic as fdm-agents/src/tools/fertilizer-planner/index.ts (i.e., compute totals
from all eligible farm fields, not only enrichedPlan fields) by either (A)
changing the call to computePlanMetrics to pass the full eligible-fields list
used by simulateFarmPlan or (B) updating computePlanMetrics implementation to
mirror the fertilizer-planner logic; ensure you reference computePlanMetrics,
simulateFarmPlan, enrichedPlan and serverMetrics and preserve the original tool
output’s farmTotals rather than overwriting them with inconsistent recomputed
values.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: dd3ecaa5-aa8a-4731-9715-77cccaca0798
📒 Files selected for processing (10)
.changeset/funky-papers-allow.md.changeset/violet-ravens-wonder.mdfdm-agents/src/agents/gerrit/agent.tsfdm-agents/src/skills/crop-specific-fertilizer-preferences/SKILL.mdfdm-agents/src/skills/crop-specific-fertilizer-preferences/references/boomkwekerij.mdfdm-agents/src/tools/fertilizer-planner/index.tsfdm-agents/tsdown.config.tsfdm-app/app/components/blocks/gerrit/plan-table.tsxfdm-app/app/components/blocks/gerrit/types.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.gerrit.tsx
✅ Files skipped from review due to trivial changes (3)
- .changeset/funky-papers-allow.md
- fdm-app/app/components/blocks/gerrit/types.ts
- fdm-agents/src/skills/crop-specific-fertilizer-preferences/references/boomkwekerij.md
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-agents/tsdown.config.ts
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.
Summary by CodeRabbit
Dependencies
New Features
Improvements
Documentation