Skip to content

Hotfix/2025062#179

Merged
SvenVw merged 7 commits into
mainfrom
hotfix/2025062
Jun 27, 2025
Merged

Hotfix/2025062#179
SvenVw merged 7 commits into
mainfrom
hotfix/2025062

Conversation

@SvenVw
Copy link
Copy Markdown
Collaborator

@SvenVw SvenVw commented Jun 26, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Corrected the calculation of ammonia emissions from fertilizers, ensuring manure, mineral, and compost types are properly excluded.
    • Fixed the display of emission values on the nitrogen balance chart.
    • Resolved an issue with saving forms when optional date fields are left empty.
  • Improvements

    • Updated the nitrogen balance chart to use the term "volatilization" instead of "emission" for improved clarity.
    • Enhanced handling of unsupported application methods for organic fertilizers by returning zero emissions instead of errors.
    • Expanded support for additional application methods for manure on cropland.
    • Adjusted emission calculations to exclude manure, mineral, and compost fertilizers from "other" fertilizer emissions.
  • Tests

    • Expanded and updated test cases for cultivation parameters and fertilizer emission calculations.
    • Improved test coverage for new cultivation properties and fertilizer types.
    • Modified tests to reflect updated emission factor behaviors and fertilizer type handling.

@SvenVw SvenVw self-assigned this Jun 26, 2025
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 26, 2025

🦋 Changeset detected

Latest commit: db7698f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@svenvw/fdm-data Patch
@svenvw/fdm-calculator Patch
@svenvw/fdm-app Patch
@svenvw/fdm-core Patch

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 26, 2025

Codecov Report

Attention: Patch coverage is 71.42857% with 6 lines in your changes missing coverage. Please review.

Project coverage is 93.42%. Comparing base (0a4fb8b) to head (b35096a).

Files with missing lines Patch % Lines
...alance/nitrogen/volatization/fertilizers/manure.ts 66.66% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #179      +/-   ##
==========================================
- Coverage   93.47%   93.42%   -0.06%     
==========================================
  Files          74       74              
  Lines        8879     8894      +15     
  Branches     1120     1126       +6     
==========================================
+ Hits         8300     8309       +9     
- Misses        577      583       +6     
  Partials        2        2              
Flag Coverage Δ
fdm-calculator 94.86% <71.42%> (-0.31%) ⬇️
fdm-core 93.01% <ø> (ø)
fdm-data 94.21% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 26, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This update refactors the nitrogen balance chart to rename the "emission" property to "volatilization" throughout the app, adjusts ammonia emission calculations for fertilizers to exclude manure, mineral, and compost, modifies manure emission factor logic to return zero for unsupported application methods, and updates test cases and cultivation catalogue data with new parameters.

Changes

File(s) Change Summary
.changeset/afraid-poems-end.md, .changeset/bright-kings-stay.md, .changeset/dull-carrots-fetch.md, .changeset/floppy-kiwis-swim.md, .changeset/metal-ants-repair.md Added changeset entries describing patches for cultivation tests, emission calculation fixes, display fixes, and form save handling.
fdm-app/app/components/blocks/balance/nitrogen-chart.tsx, fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx, fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen._index.tsx Refactored all references from emission to volatilization in chart component and its usage.
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.ts, fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.test.ts Modified manure ammonia emission factor logic to return 0 for unsupported methods and updated tests accordingly.
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.ts, fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.test.ts Updated logic to exclude manure, mineral, and compost from "other" fertilizer emissions and revised test expectations.
fdm-calculator/src/balance/nitrogen/index.test.ts Updated fertilizer test input structure and added new properties for rates and emission factors.
fdm-data/src/cultivations/hash.test.ts, fdm-data/src/cultivations/index.test.ts Extended cultivation tests with new parameters and updated expected hash values.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant NitrogenBalanceChart
  participant App
  User->>App: View nitrogen balance
  App->>NitrogenBalanceChart: Pass volatilization value
  NitrogenBalanceChart->>User: Display chart with volatilization data
Loading
sequenceDiagram
  participant Calculator
  participant EmissionLogic
  participant FertilizerInput

  FertilizerInput->>Calculator: Provide fertilizer type and details
  Calculator->>EmissionLogic: Request ammonia emission calculation
  alt Fertilizer type is manure, mineral, or compost
    EmissionLogic->>Calculator: Return 0 emission
  else Fertilizer type is other
    EmissionLogic->>Calculator: Compute and return emission
  end
Loading

Possibly related PRs

  • SvenVw/fdm#155: Also modifies the NitrogenBalanceChart component, focusing on prop handling for emission and removal values.
  • SvenVw/fdm#9: Introduces the cultivation feature set, upon which the current PR builds with additional tests and data.
  • SvenVw/fdm#95: Implements farm-level catalogue selection and syncing, which relates to catalogue and cultivation data changes in this PR.

Suggested reviewers

  • gerardhros

Poem

A rabbit hopped through fields of green,
Where nitrogen charts now gleam and preen.
Emissions renamed, logic refined,
Fertilizer rules neatly aligned.
With tests and hashes, all in sync,
This patch is smoother than you think!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b35096a and db7698f.

📒 Files selected for processing (1)
  • fdm-data/src/cultivations/hash.test.ts (9 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Post Copyable Unit Tests in Comment
  • Commit Unit Tests in branch hotfix/2025062

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@SvenVw SvenVw marked this pull request as ready for review June 27, 2025 07:14
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🔭 Outside diff range comments (1)
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.ts (1)

101-101: Update function documentation to reflect new behavior.

The JSDoc comment mentions that the function "throws Error if an unsupported application method is provided," but the function now logs warnings and returns zero instead of throwing errors.

- * @throws Error if an unsupported application method is provided for the given land type.
+ * @returns A Decimal representing the ammonia emission factor. Returns zero and logs a warning for unsupported application methods.
🧹 Nitpick comments (2)
fdm-data/src/cultivations/hash.test.ts (1)

218-220: Remove console.log statements from production test code.

Console logging should be removed from test files as it adds unnecessary noise to test output and is not needed for automated testing.

-        console.log(hash1)
-        console.log(hash2)
-        console.log(hash3)
fdm-app/app/components/blocks/balance/nitrogen-chart.tsx (1)

39-42: Consider updating the display label for consistency.

While the property is renamed to volatilization, the display label still shows "Emissie" (emission in Dutch). Consider updating this to align with the new terminology.

 volatilization: {
-    label: "Emissie",
+    label: "Vervluchtiging",
     color: "hsl(var(--chart-3))",
 },
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a4fb8b and b35096a.

⛔ Files ignored due to path filters (1)
  • fdm-app/app/lib/form.ts is excluded by !fdm-app/app/lib/**
📒 Files selected for processing (15)
  • .changeset/afraid-poems-end.md (1 hunks)
  • .changeset/bright-kings-stay.md (1 hunks)
  • .changeset/dull-carrots-fetch.md (1 hunks)
  • .changeset/floppy-kiwis-swim.md (1 hunks)
  • .changeset/metal-ants-repair.md (1 hunks)
  • fdm-app/app/components/blocks/balance/nitrogen-chart.tsx (3 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx (1 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen._index.tsx (1 hunks)
  • fdm-calculator/src/balance/nitrogen/index.test.ts (1 hunks)
  • fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.test.ts (4 hunks)
  • fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.ts (2 hunks)
  • fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.test.ts (2 hunks)
  • fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.ts (1 hunks)
  • fdm-data/src/cultivations/hash.test.ts (9 hunks)
  • fdm-data/src/cultivations/index.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: SvenVw
PR: SvenVw/fdm#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
PR: SvenVw/fdm#134
File: fdm-calculator/src/balance/nitrogen/index.ts:236-238
Timestamp: 2025-05-26T10:32:15.538Z
Learning: In nitrogen balance calculations for agricultural systems, removal and volatilization are calculated as negative values by definition since they represent nitrogen losses from the system. The balance calculation uses addition (.add()) for all components because removal and volatilization are already negative, so adding them effectively subtracts the losses from the supply.
Learnt from: SvenVw
PR: SvenVw/fdm#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|.
Learnt from: SvenVw
PR: SvenVw/fdm#143
File: fdm-app/app/components/custom/balance/nitrogen-chart.tsx:73-85
Timestamp: 2025-05-27T19:56:48.556Z
Learning: In nitrogen balance charts, supply should be in a separate stack from removal and emission. Supply represents nitrogen inputs while removal and emission represent different types of nitrogen outputs, so they should be visually grouped differently using different stackId values.
.changeset/afraid-poems-end.md (7)
Learnt from: SvenVw
PR: SvenVw/fdm#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();`.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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: When using `updateField` from fdm-core, all 8 parameters must be provided in order: fdm, b_id, b_name, b_geometry, b_area, b_id_source, b_id_farm, and b_id_farm_source.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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
PR: SvenVw/fdm#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).
.changeset/bright-kings-stay.md (4)
Learnt from: SvenVw
PR: SvenVw/fdm#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
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#88
File: fdm-calculator/src/doses/calculate-dose.ts:18-18
Timestamp: 2025-03-04T10:56:35.540Z
Learning: In the FDM calculator, fertilizer nutrient rates (p_n_rt, p_p_rt, p_k_rt) are measured in g/kg, and are converted to fractions by dividing by 10 during dose calculations.
Learnt from: SvenVw
PR: SvenVw/fdm#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|.
.changeset/dull-carrots-fetch.md (3)
Learnt from: SvenVw
PR: SvenVw/fdm#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
PR: SvenVw/fdm#143
File: fdm-app/app/components/custom/balance/nitrogen-chart.tsx:73-85
Timestamp: 2025-05-27T19:56:48.556Z
Learning: In nitrogen balance charts, supply should be in a separate stack from removal and emission. Supply represents nitrogen inputs while removal and emission represent different types of nitrogen outputs, so they should be visually grouped differently using different stackId values.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
.changeset/metal-ants-repair.md (7)
Learnt from: SvenVw
PR: SvenVw/fdm#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
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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
PR: SvenVw/fdm#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
PR: SvenVw/fdm#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 needed to include id and email fields for users, which was fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Learnt from: SvenVw
PR: SvenVw/fdm#49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.028Z
Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.
Learnt from: SvenVw
PR: SvenVw/fdm#17
File: fdm-app/app/routes/signup.tsx:279-283
Timestamp: 2024-12-10T13:14:54.639Z
Learning: In `fdm-app/app/routes/signup.tsx`, the `form.get("agreed")` returns `"true"` when the checkbox is checked, not `"on"`.
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen._index.tsx (4)
Learnt from: SvenVw
PR: SvenVw/fdm#134
File: fdm-calculator/src/balance/nitrogen/index.ts:236-238
Timestamp: 2025-05-26T10:32:15.538Z
Learning: In nitrogen balance calculations for agricultural systems, removal and volatilization are calculated as negative values by definition since they represent nitrogen losses from the system. The balance calculation uses addition (.add()) for all components because removal and volatilization are already negative, so adding them effectively subtracts the losses from the supply.
Learnt from: SvenVw
PR: SvenVw/fdm#143
File: fdm-app/app/components/custom/balance/nitrogen-chart.tsx:73-85
Timestamp: 2025-05-27T19:56:48.556Z
Learning: In nitrogen balance charts, supply should be in a separate stack from removal and emission. Supply represents nitrogen inputs while removal and emission represent different types of nitrogen outputs, so they should be visually grouped differently using different stackId values.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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|.
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx (4)
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#134
File: fdm-calculator/src/balance/nitrogen/index.ts:236-238
Timestamp: 2025-05-26T10:32:15.538Z
Learning: In nitrogen balance calculations for agricultural systems, removal and volatilization are calculated as negative values by definition since they represent nitrogen losses from the system. The balance calculation uses addition (.add()) for all components because removal and volatilization are already negative, so adding them effectively subtracts the losses from the supply.
Learnt from: SvenVw
PR: SvenVw/fdm#143
File: fdm-app/app/components/custom/balance/nitrogen-chart.tsx:73-85
Timestamp: 2025-05-27T19:56:48.556Z
Learning: In nitrogen balance charts, supply should be in a separate stack from removal and emission. Supply represents nitrogen inputs while removal and emission represent different types of nitrogen outputs, so they should be visually grouped differently using different stackId values.
Learnt from: SvenVw
PR: SvenVw/fdm#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|.
.changeset/floppy-kiwis-swim.md (3)
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.test.ts (6)
Learnt from: SvenVw
PR: SvenVw/fdm#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();`.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
fdm-calculator/src/balance/nitrogen/index.test.ts (3)
Learnt from: SvenVw
PR: SvenVw/fdm#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();`.
Learnt from: SvenVw
PR: SvenVw/fdm#134
File: fdm-calculator/src/balance/nitrogen/index.ts:236-238
Timestamp: 2025-05-26T10:32:15.538Z
Learning: In nitrogen balance calculations for agricultural systems, removal and volatilization are calculated as negative values by definition since they represent nitrogen losses from the system. The balance calculation uses addition (.add()) for all components because removal and volatilization are already negative, so adding them effectively subtracts the losses from the supply.
Learnt from: SvenVw
PR: SvenVw/fdm#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|.
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.ts (3)
Learnt from: SvenVw
PR: SvenVw/fdm#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|.
Learnt from: SvenVw
PR: SvenVw/fdm#134
File: fdm-calculator/src/balance/nitrogen/index.ts:236-238
Timestamp: 2025-05-26T10:32:15.538Z
Learning: In nitrogen balance calculations for agricultural systems, removal and volatilization are calculated as negative values by definition since they represent nitrogen losses from the system. The balance calculation uses addition (.add()) for all components because removal and volatilization are already negative, so adding them effectively subtracts the losses from the supply.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
fdm-data/src/cultivations/index.test.ts (4)
Learnt from: SvenVw
PR: SvenVw/fdm#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();`.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
fdm-app/app/components/blocks/balance/nitrogen-chart.tsx (3)
Learnt from: SvenVw
PR: SvenVw/fdm#143
File: fdm-app/app/components/custom/balance/nitrogen-chart.tsx:73-85
Timestamp: 2025-05-27T19:56:48.556Z
Learning: In nitrogen balance charts, supply should be in a separate stack from removal and emission. Supply represents nitrogen inputs while removal and emission represent different types of nitrogen outputs, so they should be visually grouped differently using different stackId values.
Learnt from: SvenVw
PR: SvenVw/fdm#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|.
Learnt from: SvenVw
PR: SvenVw/fdm#134
File: fdm-calculator/src/balance/nitrogen/index.ts:236-238
Timestamp: 2025-05-26T10:32:15.538Z
Learning: In nitrogen balance calculations for agricultural systems, removal and volatilization are calculated as negative values by definition since they represent nitrogen losses from the system. The balance calculation uses addition (.add()) for all components because removal and volatilization are already negative, so adding them effectively subtracts the losses from the supply.
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.ts (1)
Learnt from: SvenVw
PR: SvenVw/fdm#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.
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.test.ts (3)
Learnt from: SvenVw
PR: SvenVw/fdm#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();`.
Learnt from: SvenVw
PR: SvenVw/fdm#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|.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
fdm-data/src/cultivations/hash.test.ts (6)
Learnt from: SvenVw
PR: SvenVw/fdm#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();`.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
🧬 Code Graph Analysis (2)
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.test.ts (2)
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.ts (1)
  • determineManureAmmoniaEmissionFactor (103-207)
fdm-core/src/db/schema.ts (2)
  • fertilizerApplication (162-179)
  • cultivations (271-282)
fdm-data/src/cultivations/hash.test.ts (1)
fdm-data/src/cultivations/hash.ts (1)
  • hashCultivation (4-28)
⏰ Context from checks skipped due to timeout of 300000ms (2)
  • GitHub Check: calculator (22)
  • GitHub Check: core (22)
🔇 Additional comments (25)
.changeset/afraid-poems-end.md (1)

1-5: LGTM! Changeset properly documents the new cultivation parameters.

The changeset correctly follows the standard format and appropriately documents the patch-level changes for the new cultivation parameters.

fdm-data/src/cultivations/index.test.ts (1)

54-59: LGTM! Test coverage extended for new cultivation properties.

The new property checks ensure that all catalogue items have the required new cultivation parameters, maintaining comprehensive test coverage for the extended data model.

fdm-data/src/cultivations/hash.test.ts (4)

15-20: LGTM! New cultivation properties consistently added to all test objects.

The new properties are properly integrated into all cultivation test objects with appropriate values for agricultural parameters.

Also applies to: 40-45, 57-62, 81-86, 109-114, 137-142, 166-171, 195-200


28-28: LGTM! Hash value correctly updated to reflect new properties.

The expected hash value was properly updated from "9e15c11b" to "c514800f" to account for the additional properties in the cultivation object, which would change the hash calculation as implemented in fdm-data/src/cultivations/hash.ts.


209-212: LGTM! New test case effectively validates hash differentiation.

The third cultivation object with different b_lu_croprotation value ("grass" vs "other") appropriately tests that the hash function is sensitive to changes in the new properties.


223-223: LGTM! Additional assertion validates hash uniqueness for new properties.

The assertion properly verifies that changing the new b_lu_croprotation property results in a different hash, ensuring the hash function is working correctly with the new cultivation parameters.

.changeset/metal-ants-repair.md (1)

1-6: LGTM - Well-documented changeset

The changeset properly documents a fix for form saving with optional Date parameters. The format follows standard conventions and the description is clear and specific.

.changeset/dull-carrots-fetch.md (1)

1-6: LGTM - Properly documents the chart fix

The changeset correctly documents the fix for displaying emission values in the nitrogen balance chart, which aligns with the emission-to-volatilization terminology update throughout the application.

fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx (1)

269-272: LGTM - Improved semantic consistency

The property name change from emission to volatilization aligns with the nitrogen balance calculation terminology and provides better semantic clarity. This change is consistent with the backend calculations where volatilization represents nitrogen losses from the system.

.changeset/bright-kings-stay.md (1)

1-6: LGTM - Documents important calculation fix

The changeset properly documents a precision improvement in ammonia emission calculations by excluding specific fertilizer types (manure, mineral, and compost) from "other" fertilizer emissions. This backend fix complements the frontend terminology improvements.

fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen._index.tsx (1)

235-237: LGTM - Maintains consistency across routes

The property name change from emission to volatilization is consistent with the other route file changes and maintains semantic clarity across the application. This ensures uniform terminology throughout the nitrogen balance visualization components.

.changeset/floppy-kiwis-swim.md (1)

1-6: Changeset documentation is accurate and well-structured.

The changeset correctly documents the behavioral change from error throwing to returning 0 for unsupported organic fertilizer application methods. This aligns with the code changes in the related files.

fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.test.ts (3)

323-348: Test correctly reflects the updated error handling behavior.

The test updates appropriately expect the function to return 0 instead of throwing errors for unsupported application methods. This aligns with the implementation change in determineManureAmmoniaEmissionFactor which now logs warnings and returns Decimal(0) for better error handling.


351-376: Test correctly reflects the updated error handling behavior.

The test updates appropriately expect the function to return 0 instead of throwing errors for unsupported application methods on cropland. This change improves the robustness of the emission calculation system.


379-396: Test correctly reflects the updated error handling behavior.

The test updates appropriately expect the function to return 0 instead of throwing errors for unsupported application methods on bare soil. This maintains consistency with the updated error handling approach across all land use types.

fdm-calculator/src/balance/nitrogen/index.test.ts (1)

81-85: Test input structure correctly updated to align with fertilizer type refactoring.

The changes from boolean type flags to a string p_type property and the addition of new rate properties (p_no3_rt, p_nh4_rt, p_s_rt, p_ef_nh3) properly reflect the updated fertilizer detail structure. This is a cleaner approach that better represents the mutually exclusive nature of fertilizer types.

fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.ts (1)

51-53: Logic inversion correctly fixes fertilizer categorization.

The conditional logic inversion properly excludes manure, mineral, and compost fertilizers from the "other" fertilizer emission calculations. This prevents incorrect attribution of emissions and ensures that only truly "other" fertilizer types are processed by this function.

fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.test.ts (2)

107-111: Test expectations correctly updated to reflect logic inversion.

The test expectations now properly validate that manure, mineral, and compost fertilizers return 0 emissions in the "other" fertilizer calculation, which aligns with the corrected logic that excludes these well-defined types from the "other" category.


147-150: Test expectations correctly validate emissions for truly "other" fertilizer types.

The test now properly expects non-zero emissions (~0.01) for fertilizers of type "other", which validates that the function correctly calculates emissions for fertilizers that don't fall into the standard categories (manure, mineral, compost).

fdm-app/app/components/blocks/balance/nitrogen-chart.tsx (3)

15-16: LGTM: Property rename is consistent.

The function signature changes correctly rename the parameter from emission to volatilization with proper typing.

Also applies to: 20-21


26-26: LGTM: Chart data mapping is consistent.

The property mapping in chartData correctly uses the new volatilization parameter while preserving the absolute value logic for negative values.


82-85: LGTM: Bar component configuration is consistent.

The dataKey and fill color variable are correctly updated to use volatilization, and the stacking logic properly groups volatilization with removal (both in stack "b") separate from supply (stack "a").

fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.ts (3)

154-157: LGTM: Improved error handling for robustness.

Changing from throwing errors to logging warnings and returning zero emission factors makes the calculation more robust and prevents crashes from unsupported application methods.


160-177: LGTM: New cropland application methods with appropriate factors.

The addition of new application methods ("broadcasting", "incorporation 2 tracks", "slotted coulter", "incorporation") with their respective emission factors follows a logical pattern where more intensive incorporation methods have lower emission factors.


178-181: LGTM: Consistent error handling approach.

The warning + return zero pattern is consistently applied across all land types (grassland, cropland, bare soil), providing uniform behavior for unsupported application methods.

Also applies to: 203-206

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant