Implement nitrogen reduction for grassland renewal and destruction#415
Conversation
…d renewal and destruction for 2025 and 2026
…land renewal and destruction reduction rules.
🦋 Changeset detectedLatest commit: 9bf9265 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughImplements grassland renewal (50 kg N/ha) and destruction (65 kg N/ha) korting for NL 2025 and 2026, adding region, derogation and NV-area-aware date windows, throwing localized Dutch errors for invalid dates, expanding unit tests, and updating documentation and changesets. Changes
Sequence Diagram(s)(Skipped — changes are internal library logic and tests; no multi-component runtime sequence added that requires visualization.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-07-21T12:06:07.070ZApplied to files:
📚 Learning: 2025-08-11T11:55:26.053ZApplied to files:
📚 Learning: 2025-10-22T08:09:17.727ZApplied to files:
🔇 Additional comments (7)
✏️ Tip: You can disable this entire section by setting 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #415 +/- ##
===============================================
+ Coverage 87.72% 88.04% +0.31%
===============================================
Files 91 91
Lines 4466 4591 +125
Branches 1394 1468 +74
===============================================
+ Hits 3918 4042 +124
- Misses 548 549 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
coderabbitai
Bot
changed the title
coderabbitai
Bot
added
branch:development
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.ts (1)
276-280: Sorting with missing dates returns 0, which may lead to inconsistent ordering.When
b_lu_startis null/undefined, the comparison returns 0, resulting in unstable sort order for those items. While the subsequent loop skips entries withoutb_lu_end, consider filtering out incomplete cultivations before sorting for clarity.♻️ Optional: Filter before sorting
- // Sort cultivations by start date - const sortedCultivations = [...cultivations].sort((a, b) => { - if (!a.b_lu_start || !b.b_lu_start) return 0 - return a.b_lu_start.getTime() - b.b_lu_start.getTime() - }) + // Sort cultivations by start date, filtering out incomplete records + const sortedCultivations = [...cultivations] + .filter((c) => c.b_lu_start && c.b_lu_end) + .sort((a, b) => a.b_lu_start!.getTime() - b.b_lu_start!.getTime())fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.test.ts (1)
655-773: Good coverage, consider adding a test for consumption/factory potato destruction.The tests cover key scenarios well:
- Valid maize transition discount
- Catch crop exclusion
- Seed potato exclusion
- Invalid date error handling
However, there's no test verifying the 65 kg discount applies when transitioning grass to consumption or factory potatoes (the valid potato case). The 2025 test file includes this scenario (lines 728-758).
Consider adding a test case similar to this:
it("should apply 65 discount on Sand (Consumption Potato, Feb 1 - May 10)", async () => { const mockInput: NL2026NormsInput = { farm: { has_grazing_intention: false }, field: { b_id: "1", b_centroid: sandCentroid, } as Field, cultivations: [ { b_lu_catalogue: "nl_265", // Grass b_lu_start: new Date(2026, 0, 1), b_lu_end: new Date(2026, 2, 15), // March 15 }, { b_lu_catalogue: "nl_2014", // Consumption Potato b_lu_variety: "Agria", b_lu_start: new Date(2026, 2, 16), b_lu_end: new Date(2026, 9, 1), }, ] as NL2026NormsInputForCultivation[], soilAnalysis: { a_p_al: 20, a_p_cc: 0.9 }, } const result = await calculateNL2026StikstofGebruiksNorm(mockInput) expect(result.normSource).toContain("Korting: 65kg N/ha: graslandvernietiging") })fdm-docs/docs/insights/fertilizer-application-norms/nl/2025/stikstofgebruiksnorm.md (1)
70-86: Markdown list indentation inconsistency flagged by linter.Static analysis (markdownlint MD007) indicates nested lists use 4-space indentation instead of the expected 2-space indentation. If your project enforces this rule, consider adjusting the indentation.
🔧 Suggested fix for list indentation
* **Sand and Loess Soils**: June 1st – August 31st. * **Clay and Peat Soils**: - * **Derogation Farm + NV-Area**: June 1st – August 31st. - * **Derogation Farm + Non-NV-Area**: June 1st – September 15th. - * **Non-Derogation Farm**: February 1st – September 15th. + * **Derogation Farm + NV-Area**: June 1st – August 31st. + * **Derogation Farm + Non-NV-Area**: June 1st – September 15th. + * **Non-Derogation Farm**: February 1st – September 15th.* **Allowed Periods**: - * **Sand and Loess Soils**: February 1st – May 10th. - * **Clay and Peat Soils**: - * **NV-Area**: February 1st – March 15th. - * **Non-NV-Area**: February 1st – May 31st. + * **Sand and Loess Soils**: February 1st – May 10th. + * **Clay and Peat Soils**: + * **NV-Area**: February 1st – March 15th. + * **Non-NV-Area**: February 1st – May 31st.fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts (1)
365-368: Sort comparator could produce unstable results with undefined dates.When
b_lu_startis undefined for either cultivation, returning0treats them as equal, which can lead to non-deterministic ordering. Consider moving cultivations with undefined dates to the end or filtering them out before sorting.♻️ Suggested improvement
const sortedCultivations = [...cultivations].sort((a, b) => { - if (!a.b_lu_start || !b.b_lu_start) return 0 - return a.b_lu_start.getTime() - b.b_lu_start.getTime() + if (!a.b_lu_start && !b.b_lu_start) return 0 + if (!a.b_lu_start) return 1 // Push undefined to end + if (!b.b_lu_start) return -1 + return a.b_lu_start.getTime() - b.b_lu_start.getTime() })
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/nitrogen-korting-docs.md.changeset/nitrogen-korting-grassland.mdfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.tsfdm-docs/docs/insights/fertilizer-application-norms/nl/2025/stikstofgebruiksnorm.md
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 156
File: fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.ts:295-303
Timestamp: 2025-07-21T12:06:07.070Z
Learning: Functions in the fdm-calculator with "NL2025" in their names are specifically designed for Netherlands 2025 agricultural norms calculation and hardcoded 2025 dates are appropriate in this context, as different years would have separate calculation modules.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 88
File: fdm-calculator/src/doses/calculate-dose.ts:18-18
Timestamp: 2025-03-04T10:56:35.540Z
Learning: In the FDM calculator, fertilizer nutrient rates (p_n_rt, p_p_rt, p_k_rt) are measured in g/kg, and are converted to fractions by dividing by 10 during dose calculations.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 285
File: fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts:6-9
Timestamp: 2025-10-22T08:09:17.727Z
Learning: In fdm-calculator for NL 2025 phosphate stimulus ("Stimuleren organische stofrijke meststoffen"), the correct RVO mestcode groupings are: 25% discount group includes ["111", "112"] (Compost and Zeer schone compost), and the 75% discount group for organic farms includes ["40"] (Varkens - Vaste mest). The 75% base group includes ["110", "10", "61", "25", "56"].
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 343
File: fdm-calculator/src/balance/organic-matter/types.d.ts:12-132
Timestamp: 2025-11-21T10:02:25.556Z
Learning: In the organic matter balance calculation system (fdm-calculator), degradation values are negative by definition. This means the balance calculation using supply.total.plus(degradation.total) is correct, as it effectively computes supply - |degradation|. This follows the same pattern as the nitrogen balance system.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 236
File: fdm-calculator/src/balance/nitrogen/index.ts:173-0
Timestamp: 2025-08-14T14:31:55.384Z
Learning: In nitrogen balance calculations for agricultural systems, the balance should only include ammonia emissions (emission.ammonia.total) and should not include nitrate leaching from the emission calculation. The nitrate component (emission.nitrate) should be excluded from the balance formula.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 134
File: fdm-calculator/src/balance/nitrogen/index.ts:162-168
Timestamp: 2025-05-26T10:32:00.674Z
Learning: In the nitrogen balance calculation system (fdm-calculator), removal and volatilization values are negative by definition. This means the balance calculation using supply.total.add(removal.total).add(volatilization.total) is correct, as it effectively computes supply - |removal| - |volatilization|.
📚 Learning: 2025-07-21T12:06:07.070Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 156
File: fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.ts:295-303
Timestamp: 2025-07-21T12:06:07.070Z
Learning: Functions in the fdm-calculator with "NL2025" in their names are specifically designed for Netherlands 2025 agricultural norms calculation and hardcoded 2025 dates are appropriate in this context, as different years would have separate calculation modules.
Applied to files:
fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts.changeset/nitrogen-korting-grassland.mdfdm-docs/docs/insights/fertilizer-application-norms/nl/2025/stikstofgebruiksnorm.md
📚 Learning: 2025-10-22T08:09:17.727Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 285
File: fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts:6-9
Timestamp: 2025-10-22T08:09:17.727Z
Learning: In fdm-calculator for NL 2025 phosphate stimulus ("Stimuleren organische stofrijke meststoffen"), the correct RVO mestcode groupings are: 25% discount group includes ["111", "112"] (Compost and Zeer schone compost), and the 75% discount group for organic farms includes ["40"] (Varkens - Vaste mest). The 75% base group includes ["110", "10", "61", "25", "56"].
Applied to files:
fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.ts.changeset/nitrogen-korting-grassland.mdfdm-docs/docs/insights/fertilizer-application-norms/nl/2025/stikstofgebruiksnorm.md
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.ts
📚 Learning: 2025-08-13T10:33:05.313Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-08-13T10:33:05.313Z
Learning: In the fdm project, fdm-calculator integration for new features like b_lu_variety is handled in separate updates from the core data model changes. When fdm-core functions are updated to support new fields, fdm-calculator can consume these enhanced APIs without requiring changes in the same PR that introduces the core functionality.
Applied to files:
.changeset/nitrogen-korting-grassland.md
📚 Learning: 2025-02-14T09:56:37.606Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 75
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.fertilizer.tsx:68-71
Timestamp: 2025-02-14T09:56:37.606Z
Learning: The `calculateDose` function in `svenvw/fdm-calculator` is a synchronous function that includes built-in validation for negative application amounts and nutrient rates.
Applied to files:
.changeset/nitrogen-korting-grassland.md
📚 Learning: 2024-11-25T14:50:16.074Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx:63-99
Timestamp: 2024-11-25T14:50:16.074Z
Learning: i18n will be added in future PRs; for now, hardcoded Dutch text is acceptable until a translation system is implemented.
Applied to files:
.changeset/nitrogen-korting-grassland.md
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
fdm-docs/docs/insights/fertilizer-application-norms/nl/2025/stikstofgebruiksnorm.md
🧬 Code graph analysis (2)
fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.test.ts (2)
fdm-calculator/src/norms/nl/2025/value/types.d.ts (2)
NL2025NormsInput(14-26)NL2025NormsInputForCultivation(6-9)fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts (1)
calculateNL2025StikstofGebruiksNorm(721-838)
fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.ts (2)
fdm-core/src/db/schema.ts (1)
cultivations(373-385)fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm-data.ts (1)
nitrogenStandardsData(1-2739)
🪛 markdownlint-cli2 (0.18.1)
fdm-docs/docs/insights/fertilizer-application-norms/nl/2025/stikstofgebruiksnorm.md
72-72: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
73-73: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
83-83: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
84-84: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
85-85: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
86-86: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
🔇 Additional comments (15)
.changeset/nitrogen-korting-grassland.md (1)
1-5: LGTM!The changeset correctly declares a minor version bump for the new korting feature, with a clear description of the added functionality.
fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.ts (3)
141-145: LGTM!The maize derogation handling correctly reflects that derogation is no longer possible in 2026, always returning "non-derogatie". Based on learnings, year-specific hardcoded logic is appropriate for these NL20XX functions.
306-324: LGTM!The grassland renewal logic correctly:
- Applies 50 kg N/ha discount for grass-to-grass transitions
- Validates the June 1 – August 31 window for Sand/Loess regions
- Throws a descriptive Dutch error message for invalid renewal dates
326-366: LGTM!The grassland destruction logic correctly implements:
- 65 kg N/ha discount for grass→maize or grass→consumption/factory potato transitions
- Exclusion of seed potatoes (pootaardappelen, uitgroeiteelt)
- Catch crop detection via
is_vanggewasflag or autumn sowing inference- February 1 – May 10 window validation for Sand/Loess
- Descriptive Dutch error message for invalid destruction dates
.changeset/nitrogen-korting-docs.md (1)
1-5: LGTM!The changeset correctly declares a patch version bump for the documentation update, with a clear description of the added content.
fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.test.ts (2)
559-561: LGTM!Good practice to extract centroid constants for reuse across tests, improving readability and maintainability.
564-653: LGTM!Comprehensive test coverage for grassland renewal:
- Valid discount application on Sand/Loess within the allowed window
- Correct exclusion of Clay regions (2026 rule)
- Error handling for invalid renewal dates
The tests align well with the implementation requirements.
fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.test.ts (3)
592-594: LGTM!Consistent pattern with the 2026 test file for centroid constants.
597-694: LGTM!Excellent test coverage for 2025 grassland renewal:
- Sand/Loess valid window test
- Clay with derogation extended window test (up to Sep 15)
- Invalid date error handling
The tests correctly reflect the 2025 rules where Clay/Peat eligibility depends on derogation status.
696-821: LGTM!Comprehensive test coverage for 2025 grassland destruction:
- Valid maize transition on Sand
- Valid consumption potato transition on Clay (tests broader 2025 window)
- Seed potato exclusion
- Invalid date error handling
The tests properly validate the 2025 rules including Clay region eligibility.
fdm-docs/docs/insights/fertilizer-application-norms/nl/2025/stikstofgebruiksnorm.md (1)
62-93: Documentation accurately reflects the new korting implementation.The new section comprehensively documents the grassland renewal (50 kg N/ha) and destruction (65 kg N/ha) reductions with soil-type specific windows and derogation conditions. This aligns well with the TypeScript implementation in
stikstofgebruiksnorm.ts.fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts (4)
371-524: Verify behavior with multiple grassland transitions in the same year.The loop returns early upon finding the first valid renewal or destruction transition. If a field has multiple grassland transitions in 2025 (e.g., Grass→Grass→Maize), only the first detected transition's korting is applied. Confirm this is the intended business logic.
388-457: Grassland renewal logic correctly implements the date windows per soil type and derogation status.The nested conditions properly handle all combinations:
- Sand/Loess: June 1 – August 31
- Clay/Peat + Derogation + NV: June 1 – August 31
- Clay/Peat + Derogation + Non-NV: June 1 – September 15
- Clay/Peat + Non-Derogation: February 1 – September 15
Error messages are descriptive and in Dutch, matching the target audience. Based on learnings, hardcoded 2025 dates are appropriate for this NL2025-specific module.
824-826: Call site correctly updated with new parameters.The
calculateKortinginvocation now passesis_derogatie_bedrijfandis_nv_areaas required by the updated signature. Both values are properly derived earlier in the function.
461-467: Seed potato detection logic is complete and correct.The verification confirms all seed potato entries in the BRP catalogue contain either "pootaardappelen" or "uitgroeiteelt" in their cultivation_rvo_table2 field. The detection logic correctly captures all three seed potato types in the data:
- "Akkerbouwgewas, pootaardappelen"
- "Pootaardappelen, uitgroeiteelt (loofvernietiging na 15 augustus)"
- And correctly excludes non-seed potatoes like "Akkerbouwgewas, zetmeelaardappelen"
The catalogue code checks (nl_2015, nl_2016) provide redundant coverage alongside the name-based checks, ensuring no seed potatoes are incorrectly flagged for the 65 kg N/ha grassland destruction discount.
| } else { | ||
| throw new Error( | ||
| "Graslandvernieuwing op zand- en lössgrond is alleen toegestaan tussen 1 juni en 31 augustus.", | ||
| ) |
There was a problem hiding this comment.
this implies that we do not allow users to adjust from the rules. that seems fine to me, but for nutrients we usually give a warning (so allowing exceedance) isn't? Here you trow out an error. is that by purpose?
There was a problem hiding this comment.
In this case we are not able to calculate the Korting as there is no value outside the valid ranges. This is different to a nutrient advice higher than the norm, in that case we are still able to calculate the value.
| } | ||
|
|
||
| // 2. Grassland Destruction (Gras-naar-Bouwland) -> 65 kg N/ha korting | ||
| // Applies if New Crop is Maize OR Consumption/Factory/Starch Potatoes (NOT Seed Potatoes) |
There was a problem hiding this comment.
check question: is this korting also applicable for fertilizer recommendation? usually the current endpoint ignores the past if i remember it well
There was a problem hiding this comment.
I think so, but this should be handled by AgNuRe
| const isPrevGrass = | ||
| prevStandard?.type === "grasland" || | ||
| prevStandard?.type === "grasland_tijdelijk" | ||
| const isCurrGrass = |
There was a problem hiding this comment.
graszaad is considered not grassland?
There was a problem hiding this comment.
Yes, that is arable
gerardhros
left a comment
gerardhros
left a comment
There was a problem hiding this comment.
LGTM. all these exceptions makes it challenging ;-)
2 participants
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Closes #412