Implement Organic Matter balance calculation system#343
Merged
Conversation
…e` to calculate organanic matter balances of a farm
…vation catalogue to list the input of effective organic matter content via the crop and the residue
🦋 Changeset detectedLatest commit: e16b86d The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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 |
Contributor
WalkthroughAdds an Organic Matter (OM) balance feature end-to-end: new database/catalogue fields for cultivation EOM, calculator modules (input collection, supply, degradation, aggregation, conversion), app UI/pages/components for farm and field OM balances, shared soil/conversion utilities, docs, and changesets. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as fdm-app
participant Route as Balance Route
participant Core as fdm-core
participant Calc as fdm-calculator
participant DB as Database
User->>App: open OM balance page
App->>Route: route loader
Route->>Core: request farm & fields
Core->>DB: query farm/fields
DB-->>Core: farm/fields
Core-->>Route: field list
Route->>Calc: collectInputForOrganicMatterBalance
Calc->>Core: fetch cultivations, soil analyses, fertilizers (per-field)
Core->>DB: queries
DB-->>Core: data
Core-->>Calc: assembled input
Route->>Calc: calculateOrganicMatterBalance(input)
loop batch per 50 fields
Calc->>Calc: calculateOrganicMatterBalanceField
Calc->>Calc: calculateOrganicMatterSupply (fertilizers/cultivations/residues)
Calc->>Calc: calculateOrganicMatterDegradation (soil analysis → annual & timeframe)
Calc->>Calc: balance = supply + degradation
end
Calc->>Calc: aggregate area-weighted farm result
Calc->>Calc: convertOrganicMatterBalanceToNumeric
Calc-->>Route: numeric result
Route-->>App: render overview (stats, chart, fields)
App-->>User: display
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas to pay extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (7)📓 Common learnings📚 Learning: 2025-01-09T16:03:37.764ZApplied to files:
📚 Learning: 2025-01-09T16:03:37.764ZApplied to files:
📚 Learning: 2025-09-23T12:29:34.184ZApplied to files:
📚 Learning: 2025-01-09T16:03:37.764ZApplied to files:
📚 Learning: 2025-01-09T16:03:37.764ZApplied to files:
📚 Learning: 2025-06-02T10:31:27.097ZApplied to files:
🔇 Additional comments (5)
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 #343 +/- ##
===============================================
- Coverage 87.70% 87.34% -0.36%
===============================================
Files 70 79 +9
Lines 3675 3872 +197
Branches 1045 1099 +54
===============================================
+ Hits 3223 3382 +159
- Misses 452 490 +38
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
… in fertilizer list
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
fdm-docs/docs/insights/balance/02-organic-matter.md (2)
94-94: Fix temperature constant to match implementation.The documentation states "currently a constant of 11.7°C is used", but the implementation in
degradation.ts(line 54) uses 9.3°C.Apply this diff:
- 3. **Temperature Correction:** A correction factor is calculated to adjust the degradation rate based on the average yearly temperature (currently a constant of 11.7°C is used). + 3. **Temperature Correction:** A correction factor is calculated to adjust the degradation rate based on the average yearly temperature (currently a constant of 9.3°C is used).
99-111: Fix markdown formatting and list structure.Multiple formatting issues remain:
- Line 103: Heading is indented with spaces (violates MD023)
- Lines 105-106: Bullets are inconsistently indented (violates MD007)
- Line 99: Step 6 appears nested under step 5
Apply this diff to fix:
5. **Capping:** The calculated annual degradation is capped at a maximum of `3500` kg OM/ha/year to prevent unrealistic values. - 6. **Total Degradation:** The annual degradation rate is multiplied by the number of years in the calculation `Timeframe` to get the total degradation for the period. The result of this calculation is then multiplied by -1 to represent a loss from the system. + 6. **Total Degradation:** The annual degradation rate is multiplied by the number of years in the calculation `Timeframe` to get the total degradation for the period. The result of this calculation is then multiplied by -1 to represent a loss from the system. *Note: The degradation formula is based on models developed for specific regions (e.g., Flanders, Belgium). The dimensional consistency of the empirical formula should be considered within the context of its source.* - - ## 4. Field and Farm Level Balance - - * **Field Balance:** For each field, the balance is calculated as: - `OM Balance = Total EOM Supply + Total OM Degradation` -* **Farm Balance:** + +## 4. Field and Farm Level Balance + +- **Field Balance:** For each field, the balance is calculated as: + `OM Balance = Total EOM Supply + Total OM Degradation` +- **Farm Balance:** 1. The total EOM supply and OM degradation for each field are weighted by the field's area (e.g., `supply_kg_per_field = supply_kg_per_ha * area_ha`). 2. These weighted totals are summed across all fields that were successfully calculated. 3. The total farm supply and degradation (in kg) are then divided by the total area of the calculated fields to provide an average farm-level balance in kg/ha.This fixes heading indentation, aligns list markers, and corrects step 6 nesting.
🧹 Nitpick comments (1)
fdm-calculator/src/balance/organic-matter/degradation.ts (1)
61-63: Consider removing redundant null check.Lines 61-63 check for null
a_som_loianda_density_sa, but these are already checked and return early at lines 38-40. This is likely defensive programming, but it's redundant since no intervening code can nullify these values.If you prefer to keep this as a defensive safeguard, consider adding a comment explaining it's intentional:
+ // Defensive check: ensure soil properties are still present after temperature calculation if (soilAnalysis.a_som_loi == null || soilAnalysis.a_density_sa == null) { throw new Error(`"Soil analysis data (SOM or bulk density) is missing."`) }Alternatively, remove the redundant check:
- if (soilAnalysis.a_som_loi == null || soilAnalysis.a_density_sa == null) { - throw new Error(`"Soil analysis data (SOM or bulk density) is missing."`) - } -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.changeset/slimy-parks-shine.md(1 hunks)fdm-app/app/components/blocks/balance/organic-matter-details.tsx(1 hunks)fdm-app/app/components/blocks/sidebar/apps.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx(1 hunks)fdm-calculator/src/balance/organic-matter/degradation.test.ts(1 hunks)fdm-calculator/src/balance/organic-matter/degradation.ts(1 hunks)fdm-calculator/src/balance/organic-matter/index.test.ts(1 hunks)fdm-calculator/src/balance/organic-matter/supply/fertilizers.ts(1 hunks)fdm-calculator/src/balance/organic-matter/supply/residues.ts(1 hunks)fdm-calculator/src/balance/organic-matter/types.d.ts(1 hunks)fdm-calculator/src/balance/shared/soil.ts(1 hunks)fdm-docs/docs/README.md(1 hunks)fdm-docs/docs/insights/balance/02-organic-matter.md(1 hunks)fdm-docs/docs/insights/balance/_category_.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- fdm-docs/docs/insights/balance/category.json
🚧 Files skipped from review as they are similar to previous changes (5)
- fdm-calculator/src/balance/organic-matter/supply/fertilizers.ts
- fdm-calculator/src/balance/organic-matter/supply/residues.ts
- fdm-app/app/components/blocks/balance/organic-matter-details.tsx
- .changeset/slimy-parks-shine.md
- fdm-calculator/src/balance/organic-matter/degradation.test.ts
🧰 Additional context used
🧠 Learnings (22)
📓 Common learnings
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.491Z
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: 343
File: fdm-app/app/components/blocks/balance/organic-matter-chart.tsx:19-25
Timestamp: 2025-11-24T10:43:09.262Z
Learning: In the organic matter balance chart (fdm-app), degradation values are multiplied by -1 before plotting to show supply and degradation bars side-by-side with positive magnitudes, enabling direct visual comparison. This is a deliberate design choice for the organic matter chart visualization, different from the nitrogen balance chart which uses separate stacks.
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: 343
File: fdm-calculator/src/balance/organic-matter/types.d.ts:12-132
Timestamp: 2025-11-21T10:02:25.491Z
Learning: In the organic matter balance calculations, EOM (Effective Organic Matter) and OM (Organic Matter) are the same units and can be used together in calculations. EOM refers to the organic matter that remains in the soil after one year.
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.
📚 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/index.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/organic-matter/index.test.tsfdm-calculator/src/balance/shared/soil.tsfdm-calculator/src/balance/organic-matter/degradation.tsfdm-calculator/src/balance/organic-matter/types.d.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/organic-matter/index.test.ts
📚 Learning: 2025-11-21T10:02:25.491Z
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.491Z
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.
Applied to files:
fdm-calculator/src/balance/organic-matter/index.test.tsfdm-docs/docs/insights/balance/02-organic-matter.mdfdm-docs/docs/README.mdfdm-calculator/src/balance/organic-matter/degradation.tsfdm-calculator/src/balance/organic-matter/types.d.ts
📚 Learning: 2025-11-24T10:43:09.262Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 343
File: fdm-app/app/components/blocks/balance/organic-matter-chart.tsx:19-25
Timestamp: 2025-11-24T10:43:09.262Z
Learning: In the organic matter balance chart (fdm-app), degradation values are multiplied by -1 before plotting to show supply and degradation bars side-by-side with positive magnitudes, enabling direct visual comparison. This is a deliberate design choice for the organic matter chart visualization, different from the nitrogen balance chart which uses separate stacks.
Applied to files:
fdm-calculator/src/balance/organic-matter/index.test.tsfdm-docs/docs/insights/balance/02-organic-matter.mdfdm-calculator/src/balance/organic-matter/degradation.tsfdm-calculator/src/balance/organic-matter/types.d.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/index.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/balance/organic-matter/index.test.ts
📚 Learning: 2025-11-21T10:02:25.491Z
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.491Z
Learning: In the organic matter balance calculations, EOM (Effective Organic Matter) and OM (Organic Matter) are the same units and can be used together in calculations. EOM refers to the organic matter that remains in the soil after one year.
Applied to files:
fdm-docs/docs/insights/balance/02-organic-matter.mdfdm-calculator/src/balance/organic-matter/types.d.ts
📚 Learning: 2025-05-26T10:32:00.674Z
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|.
Applied to files:
fdm-docs/docs/insights/balance/02-organic-matter.mdfdm-calculator/src/balance/organic-matter/types.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-docs/docs/insights/balance/02-organic-matter.mdfdm-docs/docs/README.md
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/components/blocks/sidebar/apps.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/sidebar/apps.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx
📚 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-app/app/components/blocks/sidebar/apps.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsxfdm-docs/docs/README.md
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx
📚 Learning: 2024-12-19T13:20:44.152Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx
📚 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-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx
📚 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-docs/docs/README.md
📚 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-docs/docs/README.mdfdm-calculator/src/balance/organic-matter/types.d.ts
📚 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-docs/docs/README.md
📚 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-calculator/src/balance/organic-matter/types.d.ts
🧬 Code graph analysis (3)
fdm-calculator/src/balance/organic-matter/index.test.ts (3)
fdm-calculator/src/balance/organic-matter/index.ts (2)
calculateOrganicMatterBalanceField(130-186)calculateOrganicMatterBalancesFieldToFarm(201-263)fdm-calculator/src/balance/organic-matter/types.d.ts (2)
OrganicMatterBalanceFieldResult(153-162)OrganicMatterBalanceField(138-147)fdm-calculator/src/balance/shared/conversion.ts (1)
convertOrganicMatterBalanceToNumeric(64-89)
fdm-calculator/src/balance/shared/soil.ts (3)
fdm-calculator/src/balance/organic-matter/types.d.ts (1)
SoilAnalysisPicked(186-191)fdm-calculator/src/conversions/soil.ts (4)
calculateOrganicCarbon(13-30)calculateOrganicMatter(40-57)calculateCarbonNitrogenRatio(69-89)calculateBulkDensity(102-131)fdm-core/src/db/schema.ts (1)
soilAnalysis(721-764)
fdm-calculator/src/balance/organic-matter/types.d.ts (1)
fdm-calculator/src/index.ts (10)
FieldInput(9-9)OrganicMatterBalanceInput(33-33)OrganicMatterSupplyFertilizersNumeric(37-37)OrganicMatterSupplyCultivationsNumeric(36-36)OrganicMatterSupplyResiduesNumeric(39-39)OrganicMatterSupplyNumeric(38-38)OrganicMatterDegradationNumeric(35-35)OrganicMatterBalanceFieldNumeric(31-31)OrganicMatterBalanceFieldResultNumeric(32-32)OrganicMatterBalanceNumeric(34-34)
🔇 Additional comments (9)
fdm-app/app/components/blocks/sidebar/apps.tsx (4)
1-27: LGTM! Imports support the new collapsible balance menu.All new imports (Plus/Minus icons, Collapsible components, and SidebarMenuSub* components) are necessary for implementing the nested balance menu structure with expand/collapse functionality.
49-56: LGTM! Nitrogen balance link now points directly to the nitrogen route.The link correctly points to
/balance/nitrogen(line 53), avoiding the unnecessary redirect through/balance. This addresses the previous review concern and maintains consistency with the OM balance link pattern.
76-83: LGTM! OM balance link initialization follows established patterns.The link initialization correctly handles the create farm wizard case and invalid farm IDs, and points to the expected
/balance/organic-matterroute. Implementation is consistent with other link initialization logic in this component.
109-165: LGTM! Collapsible balance menu correctly implemented with previous issues resolved.The collapsible structure properly replaces the single balance item with a nested menu containing Stikstof and Organische stof sub-items. Key improvements:
- Both sub-items now correctly use
SidebarMenuSubButton(lines 126, 146), addressing the previous review concern- Plus/Minus icons properly indicate expand/collapse state using CSS group data selectors
- Consistent enabled/disabled state handling for both sub-items
- Proper component nesting follows the UI library patterns
fdm-docs/docs/README.md (1)
18-18: LGTM!The link update correctly reflects the new directory structure for balance-related documentation.
fdm-calculator/src/balance/organic-matter/index.test.ts (1)
1-195: LGTM!The test suite comprehensively covers the organic matter balance calculation workflow with proper mocking, negative-by-design degradation values, weighted aggregation logic, error handling, and numeric conversion. All test expectations align with the implementation.
fdm-calculator/src/balance/shared/soil.ts (1)
26-114: LGTM!The soil analysis aggregation function correctly preserves zero values using nullish coalescing, validates missing parameters by checking for both
nullandundefined, and provides robust estimation logic for missing soil properties.fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx (1)
1-336: LGTM!The route component is well-structured with proper error handling, async data loading with Suspense, user-friendly error messages, and a clean UI for displaying the organic matter balance. The NavLink path correctly uses an absolute path to avoid navigation issues.
fdm-calculator/src/balance/organic-matter/types.d.ts (1)
1-330: LGTM!The type definitions are comprehensive, well-documented, and correctly structured. The unit specifications clearly distinguish between EOM (supply) and OM (degradation), and the numeric counterparts properly mirror the Decimal-based types for final outputs.
gerardhros
reviewed
Nov 24, 2025
| `/balance/nitrogen/${b_id}`, | ||
| `/balance/nitrogen/${option.b_id}`, | ||
| )} | ||
| to={`/farm/${b_id_farm}/${calendar}/balance/${currentPath.includes("nitrogen") ? "nitrogen" : "organic-matter"}/${option.b_id}`} |
Collaborator
There was a problem hiding this comment.
are the ifelse statements in this balance.tsx the best approach to use same page for N and OM? if you later expand to other nutrients, is this still valid?
Collaborator
Author
There was a problem hiding this comment.
Yes, when other nutrients are added we have to use a different method
gerardhros
reviewed
Nov 24, 2025
| `Organic matter balance data not found for field ${b_id}`, | ||
| ) | ||
| } | ||
| if (fieldResult.errorMessage) { |
Collaborator
There was a problem hiding this comment.
does this imply that if an error occurs, the page remains empty?
Collaborator
Author
There was a problem hiding this comment.
No, we show than a card with an error message. For example if data of required soil parameters are missing we tell them to fill in those values. No tractor page is shown
gerardhros
reviewed
Nov 24, 2025
| ) | ||
|
|
||
| // Expect degradation over 3 years to be 3 times the degradation of 1 year. | ||
| expect(degradationMultiYear.total.toNumber()).toBeCloseTo( |
Collaborator
There was a problem hiding this comment.
why to you take the sum and not the average?
Collaborator
Author
There was a problem hiding this comment.
The unit is kg / ha not kg/ha / yr. Otherwise the supply has also to be averaged first. However in fdm-app we have currently only single year timeframes
gerardhros
reviewed
Nov 24, 2025
| .times(b_depth) | ||
| .times(a_density_sa) | ||
| .times(a_som_loi.ln().times(-0.008934).add(0.038228)) | ||
| .times(temperatureCorrection) |
Collaborator
There was a problem hiding this comment.
can you add documentation of the empirical formula as comment above? this function reads as you do first the ln transformation, then multiply and then add.
Collaborator
Author
There was a problem hiding this comment.
Yes, it follows the order according to decimal.js
gerardhros
reviewed
Nov 24, 2025
|
|
||
| // Calculate the average values per hectare for the entire farm. | ||
| const avgFarmSupply = totalFarmArea.isZero() | ||
| ? new Decimal(0) |
Collaborator
There was a problem hiding this comment.
should you also divide over number of years?
gerardhros
reviewed
Nov 24, 2025
|
|
||
| // Calculate the EOM supply for this specific application. | ||
| // (g EOM / kg product) * (kg product / ha) / (1000 g / kg) = kg EOM / ha | ||
| const applicationValue = p_amount.times(p_eom).dividedBy(1000) |
Collaborator
There was a problem hiding this comment.
please check if EOM and EOC are not mixed up (just to be sure). See handboekbodemenbemesting.nl
Collaborator
Author
There was a problem hiding this comment.
The OM balance is expressed as OM not OC
gerardhros
reviewed
Nov 24, 2025
|
|
||
| /** | ||
| * Represents the organic matter supply from crop residues. | ||
| * All values are in kilograms of effective organic matter per hectare (kg EOM / ha). |
Collaborator
There was a problem hiding this comment.
the kluit en perspotjes will become a separate supply category besides fertilizer, crop residue?
gerardhros
reviewed
Nov 24, 2025
| } | ||
|
|
||
| /** Numeric version of `OrganicMatterSupply`. */ | ||
| export type OrganicMatterSupplyNumeric = { |
Collaborator
There was a problem hiding this comment.
can this be non-numeric then?
Collaborator
Author
There was a problem hiding this comment.
Yes as Decimal which is the js type to prevent floating point issues, but this type can not be used on the web page so it needs to converted in the end to a standard numeric class
gerardhros
reviewed
Nov 24, 2025
Collaborator
gerardhros
left a comment
gerardhros
left a comment
There was a problem hiding this comment.
LGTM. A few minor questions.
gerardhros
approved these changes
Nov 24, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.
Closes #342