Enhance type safety and validation across fdm monorepo#378
Conversation
🦋 Changeset detectedLatest commit: 5685525 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
WalkthroughApplies widespread type, null-safety, and identifier migrations across fdm-calculator, fdm-core, and fdm-data; adds a GitHub Actions typecheck workflow and workspace/tsconfig path aliases; tightens date/amount handling and Decimal construction; and updates many tests/fixtures to the new shapes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## development #378 +/- ##
===============================================
- Coverage 87.41% 87.20% -0.21%
===============================================
Files 91 91
Lines 4497 4542 +45
Branches 1345 1385 +40
===============================================
+ Hits 3931 3961 +30
- Misses 566 581 +15
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
fdm-core/src/farm.test.ts (1)
60-68: Clarify the rationale foras anycast in signUpEmail calls or add a comment explaining why it's necessary.The
as anycast bypasses TypeScript type checking on better-auth'ssignUpEmailAPI. This pattern is used consistently across test files (farm.test.ts, authorization.test.ts, organization.test.ts, etc.). For a PR focused on type safety, either document why the cast is required for test setup (e.g., better-auth API constraints), or consider properly typing the request body if better-auth's API supports it. Add a comment explaining the trade-off.fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts (1)
176-184: Test name doesn't match test data.The test is named "should handle undefined m_cropresidue as false" but line 183 sets
m_cropresidue: nullinstead ofundefined.Either:
- Update the test name to "should handle null m_cropresidue as false", or
- Change line 183 to use
undefinedor omit the field entirely to test undefined behavior- it("should handle undefined m_cropresidue as false", () => { + it("should handle null m_cropresidue as false", () => {fdm-core/src/soil.ts (1)
544-565: Fix the type narrowing for filtered soil data.The
filter(Boolean)on line 565 correctly removesundefinedvalues at runtime, but TypeScript cannot automatically narrow the type from({ ... } | undefined)[]to the expectedCurrentSoilDatatype. The pipeline failures confirm this type mismatch.Apply this diff to add an explicit type guard:
const currentSoilData = parameters .map((parameter) => { const analysis = soilAnalyses.find( (a: Record<string, string | number>) => a[parameter as keyof typeof a] !== null, ) if (!analysis) { return undefined } return { parameter, value: analysis[parameter as keyof typeof analysis], a_id: analysis.a_id, b_sampling_date: analysis.b_sampling_date, a_depth_upper: analysis.a_depth_upper, a_depth_lower: analysis.a_depth_lower, a_source: analysis.a_source, } }) - .filter(Boolean) + .filter((item): item is NonNullable<typeof item> => item !== undefined)
♻️ Duplicate comments (2)
fdm-core/src/farm.test.ts (2)
73-82: Same type safety bypass as line 67.This
as anycast has the same implications as the one at line 67. See the review comment above for details.
593-600: Same type safety bypass as line 67.This
as anycast has the same implications as the one at line 67. See the review comment above for details.
🧹 Nitpick comments (18)
fdm-calculator/src/norms/nl/2025/value/hoofdteelt.test.ts (1)
58-58: Fix formatting: remove extra space before comma.Apply this diff:
- const result = await determineNLHoofdteelt(cultivations , 2025) + const result = await determineNLHoofdteelt(cultivations, 2025)fdm-core/src/cultivation.test.ts (1)
1132-1135: Consider normalizing indentation.These lines have excessive indentation compared to the rest of the file. While functionally correct, normalizing the indentation would improve consistency.
Apply this diff to normalize indentation:
- b_lu_start_default: null, - b_lu_eom: null, - b_lu_eom_residues: null, - b_date_harvest_default: "09-15", + b_lu_start_default: null, + b_lu_eom: null, + b_lu_eom_residues: null, + b_date_harvest_default: "09-15",fdm-calculator/src/norms/nl/2026/value/input.test.ts (2)
29-33: Consider providing more complete mock data to avoid double type cast.The
as unknown as Fieldpattern bypasses TypeScript's type checking. For a PR focused on type safety, consider providing mock data that matches theFieldtype more completely, eliminating the need for the double cast.Example approach:
const mockField = { b_id: mockFieldId, b_id_farm: "farm-1", b_centroid: { type: "Point", coordinates: [5.0, 52.0] }, -} as unknown as Field + // Add other required Field properties here +} satisfies Partial<Field> as FieldThis preserves type checking while acknowledging intentional partial mocks.
52-54: Same concern: double type cast reduces type safety.Similar to the
mockFieldcast, usingas unknown as CurrentSoilDatabypasses type checking. Consider structuring the mock to match the actualCurrentSoilDatatype, or if this is intentionally a minimal mock, document why the cast is necessary.fdm-core/src/authorization.test.ts (1)
88-88: Type casts weaken test type safety.Both signUpEmail calls now cast their payloads to
any, bypassing TypeScript validation. While this is acceptable in tests, consider aligning the test data with better-auth's expected types to maintain type safety benefits and catch potential API contract issues early.Also applies to: 109-109
fdm-core/src/organization.test.ts (1)
63-63: Type casts weaken test type safety.All three signUpEmail calls cast their payloads to
any, bypassing TypeScript validation. While this is acceptable in tests, consider whether the test data can be properly typed to match better-auth's API expectations, which would provide better type safety.Also applies to: 76-76, 89-89
fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.ts (1)
7-78: Consider usingPartial<Fertilizer>[]instead of double casting.The
as unknown as Fertilizer[]cast bypasses type safety by forcing incomplete mock objects to match the fullFertilizerinterface. This can hide type errors if the interface changes.Consider this alternative:
-] as unknown as Fertilizer[] +] as Partial<Fertilizer>[]Then update the test code to handle the partial nature of the mocks, or create a test-specific type that explicitly defines which fields are required for these tests.
fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts (1)
79-83: Consider using explicit partial types instead ofas anycasts.The
as anycasts on harvestable_analyses objects bypass type safety. If the interface changes, these tests won't catch type errors.Consider using a more explicit approach:
harvestable_analyses: [ { b_lu_yield: 1000, b_lu_n_harvestable: 20, - } as any, + } as Partial<HarvestableAnalysis>, ],This maintains type safety while acknowledging the partial test data.
Also applies to: 280-284
fdm-core/src/authentication.ts (1)
11-12: Consider clarifying the purpose of the type alias.Adding
BetterAuthas an alias forFdmAuthintroduces dual naming for the same type. While this is non-breaking and may serve as a migration path or provide clearer naming, consider:
- If this is for backward compatibility, add a JSDoc comment indicating deprecation of one name
- If both names should coexist, document when to use each
- Otherwise, consolidate to a single name to reduce confusion
Example if deprecating FdmAuth:
+/** + * @deprecated Use BetterAuth instead + */ export type BetterAuth = FdmAuthfdm-calculator/src/norms/nl/2026/value/input.ts (1)
67-73: Type casts improve explicitness, though pattern is slightly redundant.The explicit
as number | nullcasts clarify the expected type from thefind()operation. However, the nullish coalescing?? nullis redundant when the cast already permitsnull. This pattern is defensive but could be simplified:a_p_cc: - (soilAnalysis.find( + soilAnalysis.find( (x: { parameter: string }) => x.parameter === "a_p_cc", - )?.value as number | null) ?? null, + )?.value as number | null, a_p_al: - (soilAnalysis.find( + soilAnalysis.find( (x: { parameter: string }) => x.parameter === "a_p_al", - )?.value as number | null) ?? null, + )?.value as number | null,The current code is correct and safe, so this is just a minor simplification suggestion.
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts (1)
71-72: Redundantnew Date(new Date(...))wrapping.The double
new Date()wrapping is unnecessary sincenew Date("2023-01-01")already returns a Date object. This pattern appears in multiple places (lines 71-72, 96-97, 124-126, 154-155, 183-184, 212, 241-242).const timeFrame: NitrogenBalanceInput["timeFrame"] = { - start: new Date(new Date("2023-01-01")), - end: new Date(new Date("2023-12-31")), + start: new Date("2023-01-01"), + end: new Date("2023-12-31"), }fdm-calculator/src/norms/nl/2026/filling/dierlijke-mest-gebruiksnorm.test.ts (1)
100-106: Consider extracting repeated inline application objects.Multiple test cases create nearly identical inline
FertilizerApplicationobjects with the same boilerplate. Consider extracting a helper function similar tomockApplicationsfor better maintainability.fdm-calculator/src/balance/nitrogen/index.ts (1)
200-201: UseNumber.NEGATIVE_INFINITYandNumber.POSITIVE_INFINITY.Per ES2015 conventions, prefer the
Numbernamespace for infinity constants.- const fieldStartTime = field.b_start ? field.b_start.getTime() : -Infinity; - const fieldEndTime = field.b_end ? field.b_end.getTime() : Infinity; + const fieldStartTime = field.b_start ? field.b_start.getTime() : Number.NEGATIVE_INFINITY; + const fieldEndTime = field.b_end ? field.b_end.getTime() : Number.POSITIVE_INFINITY;fdm-calculator/src/balance/nitrogen/removal/harvest.test.ts (1)
24-41: Consider a test helper to reduce harvestable_analyses boilerplate.The expanded
harvestable_analysesobject has many nullable fields repeated across multiple test cases (lines 28-40, 92-123, 134-148, etc.). A factory function could improve maintainability:function createHarvestableAnalysis(overrides: Partial<HarvestableAnalysis> = {}): HarvestableAnalysis { return { b_id_harvestable_analysis: "", b_lu_yield: null, b_lu_yield_fresh: null, b_lu_yield_bruto: null, b_lu_tarra: null, b_lu_dm: null, b_lu_moist: null, b_lu_uww: null, b_lu_cp: null, b_lu_n_harvestable: null, b_lu_n_residue: null, b_lu_p_harvestable: null, b_lu_p_residue: null, b_lu_k_harvestable: null, b_lu_k_residue: null, ...overrides, } }This would simplify test data to:
createHarvestableAnalysis({ b_lu_yield: 1000, b_lu_n_harvestable: 20 }).fdm-calculator/src/doses/calculate-dose.ts (1)
64-72: Consider typed approach for nutrient rate validation.The
(fert as any)[rate]pattern works but bypasses TypeScript's type checking. A typed approach using a mapped type or explicit property access would be safer:- if ( - fertilizers.some((fert) => - nutrientRates.some((rate) => - (fert as any)[rate] ? (fert as any)[rate] < 0 : false, - ), - ) - ) { + const getNutrientRate = (fert: Fertilizer, rate: string): number | null => { + return fert[rate as keyof Fertilizer] as number | null + } + if ( + fertilizers.some((fert) => + nutrientRates.some((rate) => { + const value = getNutrientRate(fert, rate) + return value !== null && value < 0 + }), + ) + ) {This maintains type awareness while achieving the same dynamic lookup.
fdm-calculator/src/balance/nitrogen/input.test.ts (1)
131-147: Consider completing mock data to avoid type casts.The
as unknown as Type[]pattern indicates incomplete mock data. While acceptable for tests, consider adding the missing required properties to the mocks to improve type safety and catch potential issues earlier.Also applies to: 159-180
fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts (1)
429-432: Approve sort guard, but consider filtering first.The guard in the sort comparator returns 0 when dates are missing, which is safe. However, filtering out cultivations without
b_lu_startbefore sorting would be cleaner and avoid undefined behavior in sorted results.- const sortedVanggewassen = vanggewassenCompleted2024.sort((a, b) => { - if (!a.b_lu_start || !b.b_lu_start) return 0 + const sortedVanggewassen = vanggewassenCompleted2024 + .filter((v) => v.b_lu_start !== undefined) + .sort((a, b) => { return a.b_lu_start.getTime() - b.b_lu_start.getTime() })fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.ts (1)
121-129: Same comment inconsistency as NL2025 version.The comment mentions "b_start" but the code uses
b_lu_start. Consider updating for consistency.const activeCultivation = cultivations.find((c) => { - if (!c.b_lu_start) return false // Ensure b_start exists + if (!c.b_lu_start) return false // Ensure b_lu_start exists const startDate = new Date(c.b_lu_start)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts (2)
82-94: Consider simplifying test data to include only necessary properties.While explicitly setting nullable fields aligns with the type safety goals, this test only uses
b_lu_yieldandb_lu_n_harvestable. The other 13 properties set to null add verbosity without testing additional scenarios. Consider including only the properties needed for this specific test case.{ b_lu_yield: 1000, b_lu_n_harvestable: 20, - b_id_harvestable_analysis: "", - b_lu_yield_fresh: null, - b_lu_yield_bruto: null, - b_lu_tarra: null, - b_lu_dm: null, - b_lu_moist: null, - b_lu_uww: null, - b_lu_cp: null, - b_lu_n_residue: null, - b_lu_p_harvestable: null, - b_lu_p_residue: null, - b_lu_k_harvestable: null, - b_lu_k_residue: null, - }, + } as const,
296-296: Avoid using "as any" type assertion—it undermines type safety.The
as anytype assertion bypasses TypeScript's type checking, which contradicts this PR's goal of enhancing type safety. Instead, either provide a properly typed test object or use a more specific type assertion that preserves type safety.Consider one of these approaches:
Option 1: Use Partial type
{ b_lu_yield: 1000, b_lu_n_harvestable: 20, - } as any, + } as Partial<HarvestableAnalysis>,Option 2: Explicitly set required nullable fields
{ b_lu_yield: 1000, b_lu_n_harvestable: 20, - } as any, + b_id_harvestable_analysis: "", + b_lu_yield_fresh: null, + // ... other required fields + },Option 3: Create a test helper factory
const createMinimalHarvestableAnalysis = ( overrides: Partial<HarvestableAnalysis> ): HarvestableAnalysis => ({ b_id_harvestable_analysis: "", b_lu_yield_fresh: null, // ... all required fields with defaults ...overrides, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
.changeset/spicy-actors-join.md(1 hunks)fdm-calculator/package.json(1 hunks)fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts(3 hunks)fdm-calculator/src/balance/nitrogen/index.ts(3 hunks)fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts(9 hunks)fdm-calculator/src/doses/calculate-dose.test.ts(3 hunks)fdm-calculator/src/doses/calculate-dose.ts(3 hunks)fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.ts(5 hunks)fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.ts(3 hunks)fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.ts(2 hunks)fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts(3 hunks)fdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.ts(4 hunks)fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts(10 hunks)fdm-calculator/src/norms/nl/2026/filling/dierlijke-mest-gebruiksnorm.test.ts(5 hunks)fdm-calculator/src/norms/nl/2026/filling/dierlijke-mest-gebruiksnorm.ts(2 hunks)fdm-calculator/src/norms/nl/2026/filling/fosfaatgebruiksnorm.test.ts(2 hunks)fdm-calculator/src/norms/nl/2026/filling/fosfaatgebruiksnorm.ts(2 hunks)fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.ts(13 hunks)fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.ts(4 hunks)fdm-calculator/src/norms/nl/2026/value/input.ts(1 hunks)fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.ts(10 hunks)fdm-calculator/tsconfig.json(1 hunks)fdm-core/src/cultivation.test.ts(18 hunks)fdm-core/src/fertilizer.d.ts(2 hunks)fdm-core/src/soil.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- fdm-calculator/tsconfig.json
- fdm-calculator/src/norms/nl/2026/filling/dierlijke-mest-gebruiksnorm.ts
- fdm-calculator/src/norms/nl/2026/filling/dierlijke-mest-gebruiksnorm.test.ts
- fdm-calculator/src/norms/nl/2026/value/input.ts
- .changeset/spicy-actors-join.md
- fdm-calculator/src/norms/nl/2026/filling/fosfaatgebruiksnorm.ts
- fdm-core/src/cultivation.test.ts
- fdm-calculator/src/norms/nl/2026/filling/fosfaatgebruiksnorm.test.ts
- fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts
- fdm-core/src/soil.ts
- fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.ts
- fdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.ts
🧰 Additional context used
🧠 Learnings (34)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
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.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
📚 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/2025/filling/dierlijke-mest-gebruiksnorm.tsfdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts
📚 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/2025/filling/dierlijke-mest-gebruiksnorm.tsfdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/balance/nitrogen/supply/mineralization.test.tsfdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.tsfdm-calculator/src/balance/nitrogen/index.tsfdm-core/src/fertilizer.d.tsfdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts
📚 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/filling/dierlijke-mest-gebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts
📚 Learning: 2025-03-04T10:56:35.540Z
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.
Applied to files:
fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.tsfdm-core/src/fertilizer.d.tsfdm-calculator/src/doses/calculate-dose.ts
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.tsfdm-calculator/src/doses/calculate-dose.test.tsfdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/balance/nitrogen/index.tsfdm-core/src/fertilizer.d.tsfdm-calculator/src/doses/calculate-dose.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts
📚 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:
fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.tsfdm-calculator/src/doses/calculate-dose.test.tsfdm-calculator/src/doses/calculate-dose.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.ts
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-calculator/src/doses/calculate-dose.test.tsfdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/balance/nitrogen/supply/mineralization.test.tsfdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.tsfdm-calculator/src/doses/calculate-dose.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts
📚 Learning: 2024-11-27T11:27:27.797Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T11:27:27.797Z
Learning: When cleaning up test data in `afterAll` hooks in `fdm-data/src/cultivations/index.test.ts`, use `fdm.delete(schema.tableName).execute();` to delete rows from a table without dropping the table itself.
Applied to files:
fdm-calculator/src/doses/calculate-dose.test.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:
fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.tsfdm-calculator/package.jsonfdm-core/src/fertilizer.d.ts
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/balance/nitrogen/supply/mineralization.test.tsfdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.tsfdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/balance/nitrogen/supply/mineralization.test.tsfdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.tsfdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts
📚 Learning: 2025-01-24T11:39:57.805Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/cultivation.ts:625-671
Timestamp: 2025-01-24T11:39:57.805Z
Learning: Every cultivation has an associated `cultivationTerminating` record created by `addCultivation`, with `b_terminate_date` initially set to null. Therefore, `updateCultivation` can safely assume the record exists and only needs to handle updates.
Applied to files:
fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/balance/nitrogen/supply/mineralization.test.tsfdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.ts
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
fdm-calculator/package.jsonfdm-core/src/fertilizer.d.ts
📚 Learning: 2025-05-28T07:57:19.217Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 147
File: fdm-docs/package.json:39-39
Timestamp: 2025-05-28T07:57:19.217Z
Learning: In pnpm workspaces, the syntax `"typescript": "catalog:"` is valid and correct. It references the version defined in the workspace's catalog section in pnpm-workspace.yaml, allowing consistent dependency versions across packages in a monorepo.
Applied to files:
fdm-calculator/package.json
📚 Learning: 2025-05-28T07:57:19.217Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 147
File: fdm-docs/package.json:39-39
Timestamp: 2025-05-28T07:57:19.217Z
Learning: In pnpm workspaces, the syntax `"typescript": "catalog:"` is valid and correct. It references the version defined in the workspace's catalog section in pnpm-workspace.yaml, allowing consistent dependency versions across packages in a monorepo. The catalog: syntax without a package name is the shorthand for referencing the default catalog.
Applied to files:
fdm-calculator/package.json
📚 Learning: 2025-01-24T11:46:49.990Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-data/rollup.config.js:7-17
Timestamp: 2025-01-24T11:46:49.990Z
Learning: When suggesting external dependencies in Rollup configuration, only include packages that are actually listed in the package's dependencies or peerDependencies.
Applied to files:
fdm-calculator/package.json
📚 Learning: 2025-03-06T14:38:52.315Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:38:52.315Z
Learning: When writing unit tests for the fdm project, avoid using vi.mock() as it has caused implementation problems in the past. Use direct mock functions with vi.fn() instead.
Applied to files:
fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.ts
📚 Learning: 2025-03-06T15:23:48.352Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's `vi` mocking utilities and prefer manual JavaScript mocks.
Applied to files:
fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.ts
📚 Learning: 2025-03-06T14:58:48.603Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:58:48.603Z
Learning: When writing unit tests for the fdm project, avoid using Vitest's mocking utilities (vi) as it has caused problems in the past not related to the actual code. Instead, use simple object literals with methods that throw errors to test error handling.
Applied to files:
fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.ts
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/balance/nitrogen/index.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.tsfdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-calculator/src/balance/nitrogen/index.tsfdm-core/src/fertilizer.d.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-calculator/src/balance/nitrogen/index.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-calculator/src/balance/nitrogen/index.ts
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-calculator/src/balance/nitrogen/index.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-core/src/fertilizer.d.ts
📚 Learning: 2025-01-31T15:34:20.850Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-core/src/fertilizer.d.ts
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core requires 8 parameters: fdm, b_id (required), and 6 optional parameters (b_name, b_id_source, b_geometry, b_acquiring_date, b_acquiring_method, b_discarding_date).
Applied to files:
fdm-core/src/fertilizer.d.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts will be updated in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5 to include id and email fields, which are necessary for subsequent role updates and user removal operations.
Applied to files:
fdm-core/src/fertilizer.d.ts
📚 Learning: 2025-08-14T14:31:55.384Z
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.
Applied to files:
fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts
📚 Learning: 2025-09-17T08:56:42.489Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 267
File: fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.ts:100-106
Timestamp: 2025-09-17T08:56:42.489Z
Learning: In the fdm-calculator, b_lu_hi (harvest index) represents the ratio of harvested yield to total biomass in agricultural calculations. This value should naturally be non-negative and typically between 0 and 1. Negative values are not expected or physically meaningful in this agricultural domain context.
Applied to files:
fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts
📚 Learning: 2025-08-11T12:14:28.354Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-core/src/db/schema.ts:370-0
Timestamp: 2025-08-11T12:14:28.354Z
Learning: In the FDM database schema, the field `b_lu_rest_oravib` in the `cultivations_catalogue` table is intentionally nullable (can be undefined). When undefined, it should be treated as `false` in the UI layer. This allows distinguishing between "explicitly set to false" and "not set" at the database level while providing simplified boolean logic in the UI.
Applied to files:
fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts
🧬 Code graph analysis (9)
fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.ts (1)
fdm-core/src/db/schema.ts (1)
fertilizers(136-144)
fdm-calculator/src/doses/calculate-dose.test.ts (2)
fdm-core/src/fertilizer.d.ts (2)
FertilizerApplication(52-60)Fertilizer(4-49)fdm-calculator/src/doses/calculate-dose.ts (1)
calculateDose(37-135)
fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.ts (2)
fdm-core/src/db/schema.ts (2)
fertilizers(136-144)cultivations(373-385)fdm-core/src/fertilizer.d.ts (1)
Fertilizer(4-49)
fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.ts (1)
fdm-core/src/db/schema.ts (1)
cultivations(373-385)
fdm-calculator/src/balance/nitrogen/index.ts (2)
fdm-calculator/src/balance/nitrogen/types.d.ts (1)
FertilizerDetail(499-509)fdm-calculator/src/balance/organic-matter/types.ts (1)
FertilizerDetail(234-238)
fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts (3)
fdm-core/src/db/schema.ts (1)
cultivations(373-385)fdm-calculator/src/balance/nitrogen/types.d.ts (1)
FieldInput(460-480)fdm-calculator/src/balance/organic-matter/types.ts (1)
FieldInput(194-218)
fdm-calculator/src/doses/calculate-dose.ts (2)
fdm-core/src/fertilizer.d.ts (1)
Fertilizer(4-49)fdm-core/src/db/schema.ts (1)
fertilizers(136-144)
fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.ts (2)
fdm-core/src/fertilizer.d.ts (2)
Fertilizer(4-49)FertilizerApplication(52-60)fdm-core/src/index.ts (2)
Fertilizer(98-98)FertilizerApplication(99-99)
fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts (1)
fdm-core/src/db/schema.ts (1)
fertilizers(136-144)
🔇 Additional comments (34)
fdm-calculator/src/balance/nitrogen/emission/ammonia/residues.test.ts (1)
189-196: LGTM: Appropriate null handling test.The test correctly verifies that
nullvalues form_cropresidueare treated asfalse, which is appropriate defensive behavior. This aligns well with the PR's type safety enhancements.fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts (3)
22-22: LGTM:as constassertions improve type safety.Adding
as constto the mock cultivation details provides stricter type checking and ensures immutability of test fixtures, which aligns well with the PR's goal of enhancing type safety.Also applies to: 34-34, 46-46
172-177: LGTM: Null handling test is well-defined.The updated test description clearly communicates the test's purpose, and setting
b_lu_endtonullappropriately validates that the mineralization calculation handles ongoing cultivations without defined end dates.
40-40: The test filemineralization.test.tsis newly created (not modified), so there was no change from"corn"to"maize". The test data at line 40 was created with"maize"from the start. Additionally,"corn"is not a valid value in the rotation enum—the valid values forb_lu_croprotationare: "other", "clover", "nature", "potato", "grass", "rapeseed", "starch", "maize", "cereal", "sugarbeet", "alfalfa", and "catchcrop". The use of"maize"for catalogue "3" is correct and consistent with the data model.Likely an incorrect or invalid review comment.
fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.ts (2)
7-49: Test data properly updated to include p_id fields.The addition of
p_idfields to bothmockFertilizersandmockApplicationscorrectly aligns with the updatedFertilizerandFertilizerApplicationinterfaces. The type casting usingas unknown asis an appropriate testing pattern when creating minimal mock objects that don't require all optional fields to be explicitly set.
106-111: Inline test applications correctly updated.The addition of
p_idfields to inline test application objects and the consistent use of type casting maintains the same testing pattern established in the mock data fixtures. This ensures all test scenarios align with the updated type definitions.Also applies to: 136-141, 159-164, 182-187
fdm-calculator/src/doses/calculate-dose.ts (4)
44-46: Nullish coalescing properly guards against undefined amounts.The validation now safely handles potentially undefined
p_app_amountvalues by coalescing to 0 before comparison. This defensive approach prevents runtime errors and aligns with theFertilizerApplicationinterface wherep_app_amountis typed asnumber | null.
64-76: Helper function improves null-safe nutrient rate access.The
getNutrientRatehelper provides a clean abstraction for accessing potentially null nutrient rate properties onFertilizerobjects. The updated validation logic correctly checks forvalue !== null && value < 0, ensuring negative rates are caught while allowing null values to pass through safely.
100-105: Fertilizer lookup correctly updated to use p_id.The change from
f.p_id_cataloguetof.p_idaligns with the updated data model where bothFertilizerandFertilizerApplicationinterfaces include ap_idfield for matching. The error message appropriately referencesapplication.p_idto maintain consistency with the lookup key.
108-123: Dose calculations safely handle missing application amounts.All nutrient dose calculations now consistently use
application.p_app_amount ?? 0and apply nullish coalescing to nutrient rates (e.g.,fertilizer.p_n_rt ?? 0). This defensive approach ensures calculations proceed with sensible defaults when optional numeric fields are undefined, preventing NaN results while maintaining the established unit conversion logic (/1000, /1000000).fdm-calculator/src/doses/calculate-dose.test.ts (3)
23-78: Well-structured test fixtures improve maintainability.The introduction of
baseApplicationandbaseFertilizerfixtures provides a solid foundation for test data. By centralizing common field values and requiring only explicit overrides in individual tests, this approach reduces duplication and makes test intent clearer. The fixtures correctly include all required fields (p_id,p_id_catalogue,p_app_date, etc.) with sensible defaults.
82-117: Test correctly uses fixtures with appropriate overrides.The main test case properly spreads
baseApplicationandbaseFertilizerwhile overriding specific fields (p_app_id,p_id,p_app_amount, nutrient rates) as needed. The comment at line 90 helpfully explains thatp_idmust match between the application and its corresponding fertilizer, which is a key constraint for the lookup logic.
141-184: Edge case tests properly leverage base fixtures.The tests for zero amounts, zero nutrient rates, and negative values all correctly build upon the base fixtures, overriding only the fields relevant to each specific test scenario. This pattern maintains consistency while keeping each test focused on its particular edge case.
fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.ts (3)
6-6: Import path correctly updated for type reorganization.The change from
norms/nl/typesto../../typesindicates a reorganization of type definitions to a more accessible location in the module hierarchy. This improves type sharing across different norm calculation modules.
26-28: Fertilizer map correctly keyed by p_id_catalogue.The
fertilizersMapusesfertilizer.p_id_catalogueas the key, which correctly matches the lookup at line 37 whereapplication.p_id_catalogueis used to retrieve the fertilizer. This ensures the map lookups succeed for matching fertilizer applications with their definitions.
63-63: Safer handling of potentially null application amounts.Using
application.p_app_amount ?? 0ensures that missing or undefined application amounts are treated as zero in the calculation, preventing potential runtime errors when creating theDecimalinstance. This defensive approach aligns with the broader PR goal of enhancing type safety.fdm-core/src/fertilizer.d.ts (3)
6-6: Public catalogue identifier field added to Fertilizer interface.The addition of
p_id_catalogue: stringprovides a secondary identifier field alongsidep_id. Based on usage patterns in the codebase (e.g., maps keyed byp_id_catalogue), this field appears to serve as a catalogue or reference identifier whilep_idis used as the primary identifier for matching applications with fertilizers.
22-23: Organic matter metric fields appropriately typed as nullable numbers.The addition of
p_eom(effective organic matter) andp_eoc(effective organic carbon) fields withnumber | nulltyping maintains consistency with other optional nutrient rate fields in the interface. This allows fertilizers to optionally provide these metrics while defaulting to null when unavailable.
74-75: FertilizerParameters type correctly extended.The
FertilizerParametersunion type is properly updated to include"p_eom"and"p_eoc"string literals, ensuring these new fields can be referenced in parameter descriptions and other type-constrained contexts throughout the codebase.fdm-calculator/src/balance/nitrogen/index.ts (3)
47-49: Fertilizer details map correctly updated to use p_id.The change from
detail.p_id_cataloguetodetail.p_id as stringaligns with the updatedFertilizerDetailtype definition (as shown in the relevant code snippets at fdm-calculator/src/balance/nitrogen/types.d.ts:498-508) which now usesp_idinstead ofp_id_catalogue. This ensures consistent identifier usage across nitrogen balance calculations.
172-174: Essential validation for time frame boundaries.The new validation ensuring both
timeFrame.startandtimeFrame.endare provided prevents potential runtime errors in subsequent time frame calculations. This guard is particularly important given the numeric time comparisons that follow at lines 194-212, which would fail with undefined Date objects.
194-212: Robust time frame intersection logic.The reworked logic explicitly converts Dates to numeric timestamps using
getTime(), then calculates the intersection usingMath.maxfor the start boundary andMath.minfor the end boundary. The additional normalization step (lines 209-212) that clamps the end to match the start when there's no overlap ensures a valid time range is always produced. This approach is clearer and more maintainable than implicit Date comparisons.fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts (3)
10-10: Type reorganization and organic certification constant added.The import path update to
../../typesaligns with the module restructuring seen across other norm calculation files. The newrvoMestcodesOrganicRich75PercentOrganicconstant for mestcode "40" (Varkens - Vaste mest) correctly implements the organic-rich fertilizer stimulus rules for farms with organic certification.Also applies to: 14-14
52-54: Fertilizer map correctly keyed by p_id_catalogue.The
fertilizersMapconstruction usingfertilizer.p_id_catalogueas the key matches the lookup pattern at line 85 whereapplication.p_id_catalogueis used. This resolves the concern raised in the past review comment about map key consistency.
85-98: Essential error handling for missing fertilizers added.The explicit check for fertilizer existence (lines 85-90) with a descriptive error message prevents silent failures when an application references a non-existent fertilizer. After this guard, the direct access to
fertilizer.p_p_rtandfertilizer.p_type_rvo(lines 92-98) is safe and more readable than repeated map lookups. This refactoring improves both error detection and code clarity.fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.ts (4)
12-12: LGTM!The import path refactoring to use a relative path improves code organization.
80-80: LGTM!The nullish coalescing operator provides good defensive handling for potentially null/undefined application amounts, enhancing type safety.
122-124: LGTM!The guard for
b_lu_startexistence and the updated field names (b_lu_start/b_lu_end) align with the cultivation schema and enhance type safety.
40-42: Fix fertilizer lookup to match by catalogue ID consistently.The lookup at lines 40-42 mixes semantic levels by matching
f.p_id(instance ID) againstapplication.p_id_catalogue(catalogue reference). This pattern is inconsistent with the rest of the codebase:
- Test code uses
p_id_catalogue === p_id_catalogue(catalogue-to-catalogue)- Other production code uses
p_id === p_id(instance-to-instance)Change to:
const fertilizer = fertilizers.find( (f) => f.p_id_catalogue === application.p_id_catalogue, )The same issue exists in the 2025 version of this file (fdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.ts).
⛔ Skipped due to learnings
Learnt from: BoraIneviNMI Repo: SvenVw/fdm PR: 272 File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101 Timestamp: 2025-09-24T14:02:48.574Z Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.Learnt from: BoraIneviNMI Repo: SvenVw/fdm PR: 272 File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164 Timestamp: 2025-09-23T10:02:32.123Z Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.Learnt from: SvenVw Repo: SvenVw/fdm PR: 274 File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163 Timestamp: 2025-09-23T12:29:34.184Z Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.Learnt from: BoraIneviNMI Repo: SvenVw/fdm PR: 272 File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101 Timestamp: 2025-09-24T14:02:48.574Z Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.Learnt from: SvenVw Repo: SvenVw/fdm PR: 116 File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154 Timestamp: 2025-04-04T14:27:39.518Z Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.Learnt from: SvenVw Repo: SvenVw/fdm PR: 9 File: fdm-data/src/cultivations/index.test.ts:57-59 Timestamp: 2024-11-27T12:15:36.425Z Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.fdm-calculator/src/norms/nl/2026/value/stikstofgebruiksnorm.ts (3)
147-148: LGTM!The guards for
b_lu_startexistence before accessing date methods prevent runtime errors and enhance type safety across multiple cultivation history checks.Also applies to: 170-171, 187-188, 201-202
272-274: LGTM!The comprehensive guards for date field existence throughout
calculateKortingdemonstrate thorough defensive programming, preventing runtime errors and handling edge cases gracefully. The early return whensowDateis missing (lines 344-349) is particularly good practice.Also applies to: 291-291, 300-305, 319-322, 333-349
534-534: LGTM!The nullish coalescing with a sensible default end date (end of 2026) handles missing termination dates gracefully, which is appropriate for ongoing cultivations in NL2026 calculations.
fdm-calculator/src/norms/nl/2026/filling/stikstofgebruiksnorm.test.ts (2)
17-26: LGTM!The type-safe mock setup with proper TypeScript casting for the dynamic import improves test reliability.
35-54: LGTM!The comprehensive test data updates with complete object structures (including many additional fields set to null) enhance type safety and align with the updated type definitions. The use of Date objects instead of strings for dates is also a good improvement.
Also applies to: 598-655, 682-795
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts (1)
125-126: Remove redundant double Date wrapping.This issue was previously flagged. The pattern
new Date(new Date(...))is redundant and should use a single Date constructor. Note that line 212 introduces an inconsistency within the sametimeFrameobject—startuses double wrapping whileend(line 213) uses single wrapping.Apply this diff to fix all occurrences:
const timeFrame: NitrogenBalanceInput["timeFrame"] = { - start: new Date(new Date("2023-01-01")), - end: new Date(new Date("2023-12-31")), + start: new Date("2023-01-01"), + end: new Date("2023-12-31"), }This change should be applied to lines 125-126, 154-155, 183-184, 212, and 241-242.
Also applies to: 154-155, 183-184, 212-212, 241-242
🧹 Nitpick comments (1)
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts (1)
22-22: Consider whetheras constassertions are necessary.The
as constassertions make these mock objects deeply readonly. While this adds immutability guarantees for test data, it may be unnecessary if theCultivationDetailtype doesn't require literal types. Since all values are null or string literals, this shouldn't cause type compatibility issues, but verify that the narrowed types don't conflict with the expectedCultivationDetailtype.Also applies to: 34-34, 46-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
fdm-calculator/package.json(1 hunks)fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts(9 hunks)fdm-core/package.json(1 hunks)fdm-core/src/soil.ts(3 hunks)pnpm-workspace.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- fdm-calculator/package.json
- fdm-core/src/soil.ts
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
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.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
fdm-core/package.json
📚 Learning: 2025-05-28T07:57:19.217Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 147
File: fdm-docs/package.json:39-39
Timestamp: 2025-05-28T07:57:19.217Z
Learning: In pnpm workspaces, the syntax `"typescript": "catalog:"` is valid and correct. It references the version defined in the workspace's catalog section in pnpm-workspace.yaml, allowing consistent dependency versions across packages in a monorepo. The catalog: syntax without a package name is the shorthand for referencing the default catalog.
Applied to files:
fdm-core/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-05-28T07:57:19.217Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 147
File: fdm-docs/package.json:39-39
Timestamp: 2025-05-28T07:57:19.217Z
Learning: In pnpm workspaces, the syntax `"typescript": "catalog:"` is valid and correct. It references the version defined in the workspace's catalog section in pnpm-workspace.yaml, allowing consistent dependency versions across packages in a monorepo.
Applied to files:
fdm-core/package.jsonpnpm-workspace.yaml
📚 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:
fdm-core/package.json
📚 Learning: 2025-01-24T11:46:49.990Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-data/rollup.config.js:7-17
Timestamp: 2025-01-24T11:46:49.990Z
Learning: When suggesting external dependencies in Rollup configuration, only include packages that are actually listed in the package's dependencies or peerDependencies.
Applied to files:
fdm-core/package.json
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts
📚 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/balance/nitrogen/supply/mineralization.test.ts
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts
📚 Learning: 2025-01-24T11:39:57.805Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/cultivation.ts:625-671
Timestamp: 2025-01-24T11:39:57.805Z
Learning: Every cultivation has an associated `cultivationTerminating` record created by `addCultivation`, with `b_terminate_date` initially set to null. Therefore, `updateCultivation` can safely assume the record exists and only needs to handle updates.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts
🧬 Code graph analysis (1)
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts (2)
fdm-calculator/src/balance/nitrogen/types.d.ts (2)
NitrogenBalanceInput(514-522)FieldInput(460-480)fdm-calculator/src/balance/organic-matter/types.ts (1)
FieldInput(194-218)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: core (24)
🔇 Additional comments (4)
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts (3)
40-40: LGTM: Terminology correction.The change from "corn" to "maize" appears to be a terminology standardization. This aligns with European/international agricultural naming conventions and doesn't affect test logic.
64-65: LGTM: Date object conversion improves type safety.Converting
b_lu_startandb_lu_endfrom string literals to Date objects aligns with theFieldInputtype definitions and improves type safety throughout the test suite.Also applies to: 89-90, 118-119, 147-148, 176-177, 205-206, 234-235
172-172: LGTM: Explicit null handling improves clarity.Changing from
undefinedtonullfor the missing cultivation end date and updating the test description accordingly makes the intent explicit and aligns with database conventions for representing missing values.Also applies to: 177-177
pnpm-workspace.yaml (1)
15-15: LGTM! Catalog entry enables centralized version management.The addition of
@types/nodeto the catalog is properly formatted and enables consistent version management across the monorepo. The version verification has already been requested in the fdm-core/package.json review.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
fdm-calculator/src/balance/nitrogen/index.ts (1)
194-212: Improved clarity and edge case handling for time frame intersection.The refactored logic explicitly computes the intersection between the field's time bounds and the input time frame, properly handling:
- Missing field start/end dates using infinity constants
- Edge cases where there's no overlap by clamping to an empty interval
This makes the code more maintainable and reduces potential for bugs.
For additional clarity, consider extracting this logic into a helper function like
calculateTimeFrameIntersection(fieldStart, fieldEnd, timeFrameStart, timeFrameEnd)if this pattern is used elsewhere in the codebase.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.ts(9 hunks)fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.ts(2 hunks)fdm-calculator/src/balance/nitrogen/index.test.ts(2 hunks)fdm-calculator/src/balance/nitrogen/index.ts(3 hunks)fdm-calculator/src/balance/nitrogen/performance.test.ts(3 hunks)fdm-calculator/src/balance/nitrogen/supply/fertilizers.test.ts(4 hunks)fdm-calculator/src/balance/nitrogen/supply/fertilizers.ts(1 hunks)fdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts(5 hunks)fdm-calculator/src/balance/organic-matter/supply/fertilizers.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.ts
- fdm-calculator/src/balance/nitrogen/supply/fertilizers.test.ts
- fdm-calculator/src/balance/nitrogen/emission/ammonia/fertilizers.test.ts
- fdm-calculator/src/balance/nitrogen/performance.test.ts
- fdm-calculator/src/balance/nitrogen/index.test.ts
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
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.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:70-76
Timestamp: 2025-04-18T14:20:40.975Z
Learning: The organization schema in fdm-core/src/db/schema-authn.ts is managed by better-auth, and modifications to field constraints (like making the slug field non-nullable) should maintain compatibility with better-auth's expectations, even if application code assumes non-null values.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts will be updated in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5 to include id and email fields, which are necessary for subsequent role updates and user removal operations.
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/fertilizers.tsfdm-calculator/src/balance/organic-matter/supply/fertilizers.tsfdm-calculator/src/balance/nitrogen/index.tsfdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts
📚 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/balance/nitrogen/supply/fertilizers.tsfdm-calculator/src/balance/organic-matter/supply/fertilizers.tsfdm-calculator/src/balance/nitrogen/index.tsfdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/fertilizers.tsfdm-calculator/src/balance/organic-matter/supply/fertilizers.tsfdm-calculator/src/balance/nitrogen/index.ts
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/fertilizers.tsfdm-calculator/src/balance/organic-matter/supply/fertilizers.tsfdm-calculator/src/balance/nitrogen/index.ts
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-calculator/src/balance/organic-matter/supply/fertilizers.tsfdm-calculator/src/balance/nitrogen/index.ts
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-calculator/src/balance/nitrogen/index.ts
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-calculator/src/balance/nitrogen/index.ts
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts
📚 Learning: 2024-11-27T11:27:27.797Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T11:27:27.797Z
Learning: When cleaning up test data in `afterAll` hooks in `fdm-data/src/cultivations/index.test.ts`, use `fdm.delete(schema.tableName).execute();` to delete rows from a table without dropping the table itself.
Applied to files:
fdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts
📚 Learning: 2025-03-06T15:23:48.352Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's `vi` mocking utilities and prefer manual JavaScript mocks.
Applied to files:
fdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts
🧬 Code graph analysis (1)
fdm-calculator/src/balance/nitrogen/index.ts (2)
fdm-calculator/src/balance/nitrogen/types.d.ts (1)
FertilizerDetail(499-509)fdm-calculator/src/balance/organic-matter/types.ts (1)
FertilizerDetail(234-238)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: core (24)
🔇 Additional comments (7)
fdm-calculator/src/balance/nitrogen/supply/fertilizers.ts (1)
35-39: LGTM! Enhanced error message improves debuggability.The addition of the fertilizer catalogue ID (
application.p_id_catalogue) to the error message provides valuable context when a fertilizer detail lookup fails. This makes it easier to identify which specific fertilizer is missing from the details map.fdm-calculator/src/balance/nitrogen/index.ts (1)
172-174: Good validation for required timeFrame properties.This validation ensures that
timeFrame.startandtimeFrame.endare provided before proceeding with calculations, preventing potential runtime errors and providing a clear error message.fdm-calculator/src/balance/organic-matter/supply/fertilizers.test.ts (3)
3-3: LGTM: Import path updated correctly.The import path change from
../types.dto../typesaligns with moving type definitions from a declaration file to a regular TypeScript file.
44-44: LGTM: Field renamed for clarity.The renaming from
p_amounttop_app_amountis more descriptive and consistently applied across all test cases.Also applies to: 61-63, 92-92
117-122: LGTM: Test data updated to match error handling.The test data structure now uses
p_app_id, which aligns with the error message in the implementation that referencesapplication.p_app_id.fdm-calculator/src/balance/organic-matter/supply/fertilizers.ts (2)
48-50: LGTM: Enhanced error messaging.The error message now includes both the application ID and catalogue ID, which will help with debugging data integrity issues.
61-61: No action needed—p_app_amount is correctly defined as optional.The
p_app_amountfield is intentionally optional in theFertilizerApplicationtype (number | nullin fertilizer.d.ts) and allows NULL values in the database schema. Test data explicitly includes cases wherep_app_amountis null, confirming this is expected. The nullish coalescing operator (?? 0) appropriately handles this optional field without masking data quality issues.Likely an incorrect or invalid review comment.
BoraIneviNMI
left a comment
BoraIneviNMI
left a comment
There was a problem hiding this comment.
Looks good. I only found one small issue.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts (1)
22-22: Consider whetheras constassertions are necessary here.The
as constassertions on these mock objects provide stricter type inference, but since all fields arenull, the practical benefit is minimal. This is fine to keep if it aligns with broader patterns in the codebase, but it could also be omitted without affecting type safety.Also applies to: 34-34, 46-46
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts(9 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
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.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts
📚 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/balance/nitrogen/supply/mineralization.test.ts
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts
📚 Learning: 2025-01-24T11:39:57.805Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/cultivation.ts:625-671
Timestamp: 2025-01-24T11:39:57.805Z
Learning: Every cultivation has an associated `cultivationTerminating` record created by `addCultivation`, with `b_terminate_date` initially set to null. Therefore, `updateCultivation` can safely assume the record exists and only needs to handle updates.
Applied to files:
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts
🧬 Code graph analysis (1)
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts (2)
fdm-calculator/src/balance/nitrogen/types.d.ts (1)
FieldInput(460-480)fdm-calculator/src/balance/organic-matter/types.ts (1)
FieldInput(194-218)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: core (24)
🔇 Additional comments (4)
fdm-calculator/src/balance/nitrogen/supply/mineralization.test.ts (4)
64-65: Excellent type safety improvement!The migration from string literals to
Dateobjects for cultivation dates (b_lu_start,b_lu_end) aligns perfectly with the PR objectives and ensures type correctness. The explicitnullvalue at line 177 properly tests the null handling case.Also applies to: 89-90, 118-119, 147-148, 176-177, 205-206, 234-235
172-172: Clear and accurate test description.The updated test name explicitly states that it handles null cultivation end dates, which accurately reflects the test implementation and improves test documentation.
40-40: Good consistency improvement.Changing
b_lu_croprotationto"maize"for catalogue"3"aligns the mock data with the test case at line 60, which verifies zero mineralization for non-grassland cultivation. This makes the test data more semantically correct.
71-72: Date construction is consistent.All
timeFramedates use a singlenew Date()constructor, which is correct and consistent throughout the file. The previous double-wrapping issue has been properly resolved.Also applies to: 96-97, 125-126, 154-155, 183-184, 212-213, 241-242
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.