Implement Server-Side Calculation Caching in a Dedicated fdm-calculator Schema#300
Conversation
🦋 Changeset detectedLatest commit: 5b0ae45 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds a database-backed calculation cache and versioned cached wrappers for calculator functions; updates app routes to call cached APIs with an FDM DB instance; moves nutrient-advice logic into fdm-calculator; and embeds package version into the calculator bundle. Changes
Sequence Diagram(s)sequenceDiagram
participant Route as App Route Loader
participant Wrapper as withCalculationCache
participant DB as FDM Database
participant Calc as Calculator Function
Route->>Wrapper: getX(fdm, input)
activate Wrapper
Wrapper->>Wrapper: generateCalculationHash(name, version, input)
Wrapper->>DB: getCachedCalculation(hash)
DB-->>Wrapper: cachedResult | null
alt cache hit
Wrapper-->>Route: return cachedResult
else cache miss
Wrapper->>Calc: execute original calculation(input)
activate Calc
alt success
Calc-->>Wrapper: result
Wrapper->>DB: setCachedCalculation(hash, result)
Wrapper-->>Route: return result
else error
Calc-->>Wrapper: throw error
Wrapper->>DB: setCalculationError(...error...)
Wrapper-->>Route: rethrow error
end
deactivate Calc
end
deactivate Wrapper
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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 #300 +/- ##
===============================================
- Coverage 93.35% 93.31% -0.05%
===============================================
Files 91 96 +5
Lines 15028 15325 +297
Branches 1523 1557 +34
===============================================
+ Hits 14030 14301 +271
- Misses 996 1022 +26
Partials 2 2
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
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
🧹 Nitpick comments (14)
fdm-calculator/rollup.config.js (1)
33-42: Consider adding validation to the version replacement plugin.The string replacement approach works but could be made more robust:
Potential improvements:
- Verify that exactly one replacement occurred to catch mismatches between the placeholder in
package.tsand the search string- Warn if the placeholder isn't found or appears multiple times
- Consider using a more targeted replacement (e.g., regex with word boundaries)
Example enhancement:
{ renderChunk: (code) => { + const placeholder = "fdm-calculator:{FDM_CALCULATOR_VERSION}" + const occurrences = (code.match(new RegExp(placeholder.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g')) || []).length + + if (occurrences === 0) { + console.warn(`⚠️ Version placeholder "${placeholder}" not found in bundle`) + } else if (occurrences > 1) { + console.warn(`⚠️ Version placeholder "${placeholder}" appears ${occurrences} times`) + } + return { - code: code.replace( - "fdm-calculator:{FDM_CALCULATOR_VERSION}", - `fdm-calculator:${packageJson.version}`, - ), + code: code.replace(placeholder, `fdm-calculator:${packageJson.version}`), } }, },This would help catch issues during build time if the placeholder string gets out of sync with
package.ts.fdm-calculator/src/balance/nitrogen/index.ts (1)
123-123: Consider using a more specific cache key to avoid collisions.The cache key
"calculateNitrogenBalance"is a simple string. If multiple calculation types use similar naming patterns, there's a risk of key collisions. Consider using a namespaced key like"nitrogen.balance"or including the module/package name.#!/bin/bash # Check for potential cache key collisions across the codebase rg -n 'withCalculationCache\(' --type=ts -A1 | rg '"[^"]*"'fdm-calculator/src/norms/nl/2025/dierlijke-mest-gebruiksnorm.test.ts (1)
3-3: Verify test coverage for the caching layer.Similar to the phosphate norm tests, these tests now call
calculateNL2025DierlijkeMestGebruiksNormdirectly, bypassing the cached wrapper. Consider adding integration tests that verify the caching behavior for the publicgetNL2025DierlijkeMestGebruiksNormfunction.fdm-calculator/src/index.ts (1)
3-6: Document the difference betweencalculateNitrogenBalanceandgetNitrogenBalance.The public API now exports both the cached (
getNitrogenBalance) and uncached (calculateNitrogenBalance) versions. Consider adding JSDoc comments to clarify:
- When to use each version
- The signature difference (getNitrogenBalance requires
fdmas first parameter)- The performance/caching implications
Example documentation:
/** * Calculates nitrogen balance without caching. * Use this for testing or when you need to bypass the cache. * @param input - The nitrogen balance input */ export { calculateNitrogenBalance } /** * Calculates nitrogen balance with database-backed caching. * Results are cached based on calculator version and input hash. * @param fdm - The FDM database instance * @param input - The nitrogen balance input */ export { getNitrogenBalance }fdm-calculator/src/norms/nl/2025/fosfaatgebruiksnorm.test.ts (1)
2-2: Verify test coverage for the caching layer.The tests call
calculateNL2025FosfaatGebruiksNormdirectly, bypassing the cached wrappergetNL2025FosfaatGebruiksNormintroduced in this PR. While generic caching behavior is tested infdm-core/src/calculator.test.ts, the norms-specific functions lack dedicated cache verification tests. Consider adding tests inindex.test.tsor a new test file to verify:
- Cache hits on repeated calls with identical inputs
- Cache misses or invalidation when
pkg.calculatorVersionchanges- Consistency of cached results across function calls
fdm-core/src/calculator.test.ts (1)
2-2: Unused type import.drizzle import is unused; consider removing to keep tests tidy.
fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.ts (1)
623-631: Prevent negative norms after korting.Clamp at zero to avoid accidental negatives in edge cases.
- normValue = new Decimal(normValue).minus(kortingAmount).toNumber() + normValue = Math.max( + 0, + new Decimal(normValue).minus(kortingAmount).toNumber(), + )fdm-calculator/src/norms/nl/2025/dierlijke-mest-gebruiksnorm.ts (1)
22-24: Naming consistency: GWBG vs function name.
isFieldInGWGBGebiedlikely intends GWBG. Consider renaming toisFieldInGWBGGebied(orisFieldInGWBG) for clarity.fdm-core/src/db/schema-calculator.ts (4)
10-10: Avoid hyphens in schema names to reduce quoting friction.
pgSchema("fdm-calculator")forces quoted identifiers and can break tooling/migrations. Preferfdm_calculator.This rename requires a migration and config update (drizzle-kit, grants, search_path).
16-22: Prefer JSONB for inputs/results to enable indexing and better ops.Switch
json()tojsonb()forinputs,result, and errorinputsto support GIN indexes if needed.-import { - json, +import { + jsonb, @@ - inputs: json(), - result: json(), + inputs: jsonb(), + result: jsonb(), @@ - inputs: json(), + inputs: jsonb(),Also applies to: 37-44
11-11: Type alias name is misleading.
fdmSchemaAuthNTypeSelectdoesn’t match this module; considerFdmCalculatorSchemaTableType.
13-32: Plan retention and indexes.
- Add retention policy (e.g., drop
calculation_cacherows older than N days if inputs change frequently; keepcalculation_errorsfor M days).- Consider indexes for common queries, e.g.,
(calculation_type, created_at)on errors.Also applies to: 34-45
fdm-core/src/calculator.ts (2)
30-47: Add LIMIT 1 to the cache read.Micro-optimization and intent clarity.
.from(calculationCacheTable) .where( and( eq(calculationCacheTable.calculation_type, calculation_type), eq( calculationCacheTable.input_hash, getCalculationInputHash(calculator_version, inputs), ), ), ) + .limit(1)
9-14: Stabilize hash parameters and document the contract.If array order or incidental fields differ, cache hits will be missed. Consider centralizing hash options and documenting that
T_Inputmust be JSON-serializable and order-stable.Example:
-import hash from "object-hash" +import hash from "object-hash" +const HASH_OPTS = { algorithm: "sha256", encoding: "hex" } as const @@ - return hash([calculator_version, inputs]) + return hash([calculator_version, inputs], HASH_OPTS)And add JSDoc on
T_Inputexpectations.Please confirm the
object-hashversion is pinned (major at least). Upgrading majors changes hash outputs, invalidating cache.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
fdm-core/src/db/migrations/0016_mushy_marvel_zombies.sqlis excluded by!fdm-core/src/db/migrations/**fdm-core/src/db/migrations/meta/0016_snapshot.jsonis excluded by!fdm-core/src/db/migrations/**fdm-core/src/db/migrations/meta/_journal.jsonis excluded by!fdm-core/src/db/migrations/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.changeset/shaggy-bars-rule.md(1 hunks).changeset/whole-lies-jog.md(1 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen._index.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx(1 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx(3 hunks)fdm-calculator/rollup.config.js(2 hunks)fdm-calculator/src/balance/nitrogen/index.ts(3 hunks)fdm-calculator/src/index.ts(1 hunks)fdm-calculator/src/norms/nl/2025/dierlijke-mest-gebruiksnorm.test.ts(8 hunks)fdm-calculator/src/norms/nl/2025/dierlijke-mest-gebruiksnorm.ts(3 hunks)fdm-calculator/src/norms/nl/2025/fosfaatgebruiksnorm.test.ts(3 hunks)fdm-calculator/src/norms/nl/2025/fosfaatgebruiksnorm.ts(3 hunks)fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.test.ts(22 hunks)fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.ts(3 hunks)fdm-calculator/src/package.ts(1 hunks)fdm-core/drizzle.config.ts(1 hunks)fdm-core/package.json(2 hunks)fdm-core/src/calculator.test.ts(1 hunks)fdm-core/src/calculator.ts(1 hunks)fdm-core/src/db/schema-calculator.ts(1 hunks)fdm-core/src/db/schema.ts(1 hunks)fdm-core/src/index.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: SvenVw
PR: SvenVw/fdm#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.
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
PR: SvenVw/fdm#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/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
PR: SvenVw/fdm#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-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx
📚 Learning: 2025-07-21T12:06:07.070Z
Learnt from: SvenVw
PR: SvenVw/fdm#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/fosfaatgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/dierlijke-mest-gebruiksnorm.ts
🧬 Code graph analysis (11)
fdm-calculator/src/norms/nl/2025/fosfaatgebruiksnorm.test.ts (1)
fdm-calculator/src/norms/nl/2025/fosfaatgebruiksnorm.ts (1)
calculateNL2025FosfaatGebruiksNorm(140-175)
fdm-core/src/calculator.test.ts (3)
fdm-core/src/index.ts (4)
FdmServerType(80-80)createFdmServer(79-79)withCalculationCache(30-30)setCachedCalculation(28-28)fdm-core/src/calculator.ts (3)
withCalculationCache(93-136)setCachedCalculation(50-64)getCalculationInputHash(9-14)fdm-core/src/db/schema-calculator.ts (2)
calculationCache(13-32)calculationErrors(34-45)
fdm-calculator/src/norms/nl/2025/dierlijke-mest-gebruiksnorm.test.ts (1)
fdm-calculator/src/norms/nl/2025/dierlijke-mest-gebruiksnorm.ts (1)
calculateNL2025DierlijkeMestGebruiksNorm(150-194)
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen._index.tsx (2)
fdm-calculator/src/index.ts (1)
getNitrogenBalance(5-5)fdm-calculator/src/balance/nitrogen/index.ts (1)
getNitrogenBalance(122-126)
fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.test.ts (1)
fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.ts (1)
calculateNL2025StikstofGebruiksNorm(527-637)
fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx (3)
fdm-core/src/index.ts (1)
withCalculationCache(30-30)fdm-calculator/src/index.ts (2)
fdmCalculator(2-2)calculateDose(25-25)fdm-app/app/integrations/nmi.ts (1)
getNutrientAdvice(231-287)
fdm-core/src/calculator.ts (1)
fdm-core/src/index.ts (5)
getCachedCalculation(27-27)FdmType(78-78)setCachedCalculation(28-28)setCalculationError(29-29)withCalculationCache(30-30)
fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.ts (1)
fdm-core/src/index.ts (1)
withCalculationCache(30-30)
fdm-calculator/src/balance/nitrogen/index.ts (2)
fdm-calculator/src/index.ts (2)
getNitrogenBalance(5-5)calculateNitrogenBalance(4-4)fdm-core/src/index.ts (1)
withCalculationCache(30-30)
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx (2)
fdm-calculator/src/index.ts (1)
getNitrogenBalance(5-5)fdm-calculator/src/balance/nitrogen/index.ts (1)
getNitrogenBalance(122-126)
fdm-calculator/src/norms/nl/2025/dierlijke-mest-gebruiksnorm.ts (1)
fdm-core/src/index.ts (1)
withCalculationCache(30-30)
🔇 Additional comments (19)
fdm-core/src/db/schema.ts (1)
42-45: LGTM! Formatting improvement.The multi-line formatting for the nl_03 entry improves readability and aligns with the formatting of other multi-line entries in the same array.
fdm-calculator/rollup.config.js (1)
6-6: LGTM! Using standard import attributes syntax.The import attributes syntax with
{ type: "json" }is the correct approach for importing JSON files in modern JavaScript environments.fdm-core/drizzle.config.ts (1)
10-10: LGTM! Proper schema integration.The addition of the calculator schema to the drizzle config enables proper migration generation and management for the caching infrastructure.
fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.test.ts (2)
4-4: LGTM! Appropriate test refactoring.The import change from
getNL2025StikstofGebruiksNormtocalculateNL2025StikstofGebruiksNormcorrectly targets the internal calculation function for unit testing, separating concerns between calculation logic and caching infrastructure.
37-561: LGTM! Consistent function rename across all tests.All test cases have been updated to use
calculateNL2025StikstofGebruiksNorm, maintaining test coverage while aligning with the new naming convention where internal calculation functions use the "calculate" prefix.fdm-calculator/src/package.ts (1)
1-3: LGTM! Clean version placeholder approach.The default export provides a version placeholder that gets replaced at build time by the Rollup plugin. This enables version-tagged cache keys for calculations.
Note: The placeholder string
"fdm-calculator:{FDM_CALCULATOR_VERSION}"must remain synchronized with the replacement logic inrollup.config.js(line 37). Consider the validation suggestion in the rollup config review to catch any mismatches during build.fdm-core/package.json (1)
55-55: The specified @types/object-hash version exists and is correct.The review comment's concern is based on incorrect information. The npm registry confirms that
@types/object-hash@3.0.6is the latest published version and exists without issues. While there is a version mismatch (types at 3.0.6, runtime at 3.0.0), this is not problematic—@types packages are commonly maintained independently and may be ahead of runtime package versions. No changes are needed.Likely an incorrect or invalid review comment.
fdm-core/src/index.ts (1)
26-31: LGTM! Clean public API exposure.The caching utilities are appropriately re-exported from the core module, making them available to dependent packages like fdm-calculator and fdm-app.
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx (1)
101-101: LGTM! Correct usage of the cached API.The call site correctly uses the new cached function
getNitrogenBalancewith thefdminstance as the first parameter, enabling database-backed caching for this expensive calculation.fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen._index.tsx (1)
97-100: LGTM! Consistent cached API usage.The nitrogen balance calculation correctly uses the cached wrapper with the
fdminstance, consistent with the other route changes in this PR.fdm-calculator/src/balance/nitrogen/index.ts (1)
122-126: No issues found. The parameter threading inwithCalculationCacheis correct.The wrapper function correctly:
- Accepts
(fdm: FdmType, inputs: T_Input)from call sites- Uses
fdmonly for cache operations (getCachedCalculation,setCachedCalculation)- Passes only
inputstocalculateNitrogenBalance(line 113 in calculator.ts:calculationFunction(inputs))calculateNitrogenBalancesignature accepts onlyNitrogenBalanceInput, notfdmAll call sites in fdm-app correctly invoke
getNitrogenBalance(fdm, input)with the expected parameters.fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx (1)
138-149: The function signatures are correct. No changes needed.The
withCalculationCachewrapper returns functions with the signature(fdm: FdmType, inputs: T_Input) => Promise<T_Output>, which means the functions returned bycreateFunctionsForNorms(calculateNormForManure,calculateNormForPhosphate,calculateNormForNitrogen) correctly expectfdmas the first parameter andinputas the second parameter. The calls in lines 138-149 match this signature exactly.fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.ts (2)
639-643: Wrapper looks good.Caching via
withCalculationCache("calculateNL2025StikstofGebruiksNorm", pkg.calculatorVersion, ...)is consistent with the new infra.
73-78: Good progress. I can now see a clear pattern. Let me investigate why the code uses mixed versioning:Let me search the code for any comments or documentation explaining the dataset choice:
Now let me search the code for any comments or README documentation about this:
Based on my analysis of the codebase and domain research, I can now provide the rewritten review comment:
Grondsoorten dataset year appears intentional—no fix needed.
Grondsoorten map hasn't been updated since 2005, making it stable and non-annual. The codebase consistently uses 2024 versions for grondsoorten, gwbg, and natura2000 across all 2025 calculators, reflecting how these data sources are versioned by release year rather than calendar year. Your grondsoort and the gewas you grow determine the nitrogen norm height, so using the authoritative 2024 release is appropriate for 2025 nitrogen norms calculations. If a 2025 version exists in FDM's public data, verify and update; otherwise, this usage is correct and could benefit from an inline comment explaining the versioning rationale.
fdm-calculator/src/norms/nl/2025/dierlijke-mest-gebruiksnorm.ts (2)
196-200: Wrapper looks good.Cached public wrapper aligns with the pattern used elsewhere.
22-44: Verify dataset year alignment in GWBG and Natura2000 TIFF references.The codebase shows inconsistency: within the same 2025 norms file,
derogatievrije_zones.tiffcorrectly uses2025, butgwbg.tiffandnatura2000.tiffuse2024. No comments explain this rationale. Web search did not confirm whether 2025 versions are available. This requires verification with your data source team to confirm:
- Whether 2025 TIFF layers exist and are approved for use
- Whether 2024 is intentional (if so, add a comment explaining why) or an oversight requiring update
Also applies to: 57-79
fdm-calculator/src/norms/nl/2025/fosfaatgebruiksnorm.ts (3)
1-3: LGTM: Imports are correct for caching implementation.The imports of
withCalculationCacheand package metadata are necessary and properly structured for the calculation caching feature.
140-175: No issues found—code structure is correct.The function rename and refactoring are properly implemented:
- Implementation function (
calculateNL2025FosfaatGebruiksNorm, line 140): Exported for direct calls and as the cache target- Cached wrapper (
getNL2025FosfaatGebruiksNorm, line 177): Properly exported and used by production code- Test isolation: Unit tests correctly call the implementation directly (fosfaatgebruiksnorm.test.ts, lines 20, 39)
- Production routing: Index files correctly import and use the cached wrapper (norms/index.ts line 3, norms/index.test.ts line 8)
No other call sites found in production code, confirming all routing flows through the intended cached wrapper.
177-181: Code is correct as-is; the suggested refactor would break cache functionality.The string literal
"calculateNL2025FosfaatGebruiksNorm"is intentionally hardcoded because it serves as a database cache identifier. UsingcalculateNL2025FosfaatGebruiksNorm.nameinstead would be problematic: if the function is renamed, the cache key would change, breaking lookups for existing cached entries in the database. The string literal design ensures cache consistency across function renames.
pkg.calculatorVersionis properly defined infdm-calculator/src/package.tsand is valid.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
fdm-core/src/calculator.ts (2)
58-64: Fix race on duplicate inserts (use upsert).Two concurrent misses can both insert the same key and violate the unique constraint. Make the write idempotent via ON CONFLICT DO NOTHING on (calculation_type, input_hash).
- await fdm.insert(calculationCacheTable).values({ - calculation_type: calculation_type, - calculator_version: calculator_version, - inputs: inputs, - input_hash: getCalculationInputHash(calculator_version, inputs), - result: result, - }) + await fdm + .insert(calculationCacheTable) + .values({ + calculation_type, + calculator_version, + inputs, + input_hash: getCalculationInputHash(calculator_version, inputs), + result, + }) + .onConflictDoNothing({ + target: [ + calculationCacheTable.calculation_type, + calculationCacheTable.input_hash, + ], + })Run to confirm a unique index exists on (calculation_type, input_hash):
#!/bin/bash # Verify unique/PK covering (calculation_type, input_hash) fd -a schema-calculator.ts | while read -r f; do echo "== $f"; rg -n -C3 -e 'calculation_cache' -e 'unique' -e 'index' "$f" done
109-111: Cached falsy values are treated as misses.Return cached 0/false/""; only null should mean “not found”.
- if (cachedResult) { + if (cachedResult !== null) { return cachedResult }
🧹 Nitpick comments (6)
fdm-calculator/rollup.config.js (1)
33-58: Good approach to version injection, consider hardening placeholder validation.The renderChunk plugin correctly replaces the version placeholder and attempts to preserve source maps via padding. The warnings are helpful for debugging.
Consider these improvements:
Stricter validation: Make missing or multiple placeholders a build error rather than a warning to catch issues earlier:
if (occurrences.length === 0) { - console.warn(`⚠️ Version placeholder "${placeholder}" not found in bundle`) + throw new Error(`Version placeholder "${placeholder}" not found in bundle`) } else if (occurrences.length > 1) { - console.warn(`⚠️ Version placeholder "${placeholder}" appears ${occurrences.length} times`) + throw new Error(`Version placeholder "${placeholder}" appears ${occurrences.length} times`) }Simplified regex escaping (line 38): The current pattern works but is non-standard. Consider:
const occurrences = code.match(new RegExp(placeholder.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), "g")) || []Source map handling: When the replacement is longer than the placeholder (line 46-48), the build continues despite the broken source map. Consider making this an error in production builds.
fdm-core/src/calculator.ts (5)
9-14: Strengthen hash determinism and collision resistance.Use a stronger algorithm and fixed encoding; optional: allow excluding ephemeral keys.
export function getCalculationInputHash<T_Input>( calculator_version: string, inputs: T_Input, ) { - return hash([calculator_version, inputs]) + // Use a stronger hash and stable encoding to reduce collisions across engines. + return hash([calculator_version, inputs], { + algorithm: "sha256", + encoding: "hex", + // Optionally: excludeKeys: (k) => k === "timestamp" || k === "requestId", + }) }
30-48: Tighten lookup and trim payload.Add calculator_version to WHERE (defensive if hash strategy changes) and stop selecting unused fields.
- const result = await fdm - .select({ - calculation_type: calculationCacheTable.calculation_type, - calculator_version: calculationCacheTable.calculator_version, - result: calculationCacheTable.result, - created_at: calculationCacheTable.created_at, - }) + const result = await fdm + .select({ result: calculationCacheTable.result }) .from(calculationCacheTable) .where( and( eq(calculationCacheTable.calculation_type, calculation_type), - eq( - calculationCacheTable.input_hash, - getCalculationInputHash(calculator_version, inputs), - ), + eq(calculationCacheTable.input_hash, getCalculationInputHash(calculator_version, inputs)), + eq(calculationCacheTable.calculator_version, calculator_version), ), ) .limit(1)
124-132: Harden error capture for non-Error throws.Ensure messages/stacks are recorded even if a string/object is thrown.
- } catch (e: any) { - await setCalculationError( + } catch (e: unknown) { + const err = e instanceof Error ? e : new Error(String(e)) + await setCalculationError( fdm, calculation_type, calculator_version, inputs, - e.message, - e.stack, + err.message, + err.stack, )
58-64: Minor TS/style: use object shorthand in inserts.Simplify to property shorthands for clarity.
- calculation_type: calculation_type, - calculator_version: calculator_version, - inputs: inputs, + calculation_type, + calculator_version, + inputs,
94-137: Add cache stampede mitigation and observability metrics.Verification confirms no metrics or distributed locking currently exist. Multiple concurrent cache misses can trigger duplicate calculations. Implement per-key advisory locks (or equivalent) around the calculation + insert step, and add counters for cache_hit, cache_miss, and cache_error to enable monitoring and alerting.
Example flow:
- Acquire advisory lock keyed by input hash
- Recheck cache (another thread may have populated it)
- Execute calculation and upsert result
- Release lock
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
fdm-calculator/rollup.config.js(2 hunks)fdm-core/src/calculator.ts(1 hunks)fdm-core/src/db/schema-calculator.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-core/src/db/schema-calculator.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: SvenVw
PR: SvenVw/fdm#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.
🧬 Code graph analysis (1)
fdm-core/src/calculator.ts (1)
fdm-core/src/index.ts (5)
getCachedCalculation(27-27)FdmType(78-78)setCachedCalculation(28-28)setCalculationError(29-29)withCalculationCache(30-30)
🔇 Additional comments (1)
fdm-calculator/rollup.config.js (1)
6-6: LGTM: Modern JSON import syntax.The
with { type: "json" }syntax is the current standard for JSON import assertions, replacing the olderassertkeyword.
Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
fdm-core/src/calculator.ts (4)
91-98: Fix race: duplicate inserts cause PK violation and spurious error logging.Two concurrent cache misses for the same key will both compute and
insert, causing a unique constraint error that bubbles up. Make the write idempotent by adding.onConflictDoNothing()or.onConflictDoUpdate()to handle duplicate keys gracefully.
95-95: Privacy/compliance: persisting raw inputs may store PII.Caching raw inputs may include user identifiers or sensitive farm data, creating GDPR/CCPA risks. Consider storing only the input_hash or implementing redaction for sensitive fields before persisting.
124-124: Privacy/compliance: error logs persist raw inputs.Similar to the caching function, storing raw inputs in error records may capture PII. Consider redacting sensitive fields or storing only the input_hash for compliance.
209-214: Cached 0/false/"" are treated as misses.The conditional
if (cachedResult)treats any falsy result as a cache miss, causing unnecessary recomputation. Check fornullexplicitly to distinguish genuine misses from valid falsy cached values.
🧹 Nitpick comments (7)
.changeset/witty-animals-march.md (1)
5-5: Tighten the phrasingTiny typo: “enables adds” reads awkwardly—let’s tidy it up.
Apply this diff:
-Add new db schema `fdm-calculator` to cache calculation results and store calculation errors. The decorator function `withCalculationCache` enables adds the functionality to add caching to calculator functions +Add new db schema `fdm-calculator` to cache calculation results and store calculation errors. The decorator function `withCalculationCache` adds caching capabilities to calculator functionsfdm-calculator/src/balance/nitrogen/index.ts (1)
122-137: Document the required FDM handleConsumers now need to pass the FDM instance into
getNitrogenBalance; the JSDoc should call that out to avoid confusion.Apply this diff:
* includes a caching mechanism to improve performance for repeated calls with the * same input. The cache is managed by `withCalculationCache` and uses the * `pkg.calculatorVersion` as part of its cache key. * + * @param fdm - Database handle used by the cache layer for lookups and persistence. * @param nitrogenBalanceInput - The input data for the nitrogen balance calculation. * @returns A promise that resolves with the calculated nitrogen balance, with numeric values as numbers. */fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts (1)
289-303: Call out the FDM dependencyThe cached wrapper expects the FDM connection up front; let’s note that in the JSDoc so the signature stays accurate.
Apply this diff:
- * @param {NL2025NormsFillingInput} input - The standardized input object containing all necessary data. + * @param fdm - Database handle used by the calculation cache. + * @param {NL2025NormsFillingInput} input - The standardized input object containing all necessary data.fdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.ts (1)
247-260: Mention the FDM handle in the memoized exportSame story here—the wrapper now needs the FDM instance; documenting it keeps the contract clear.
Apply this diff:
- * @param {NL2025NormsFillingInput} input - The standardized input object containing all necessary data. + * @param fdm - Database handle required by the caching layer. + * @param {NL2025NormsFillingInput} input - The standardized input object containing all necessary data.fdm-calculator/src/nutrient-advice/index.test.ts (1)
1-9: Consider alternatives to Vitest'svimocking utilities.This test uses Vitest's
vi.stubGlobal(),vi.mocked(), and other mocking utilities. However, based on learnings, the FDM project has experienced issues with Vitest's mocking utilities in the past and prefers simpler approaches.While mocking
fetchis a legitimate testing requirement, consider whether a manual mock approach would be more consistent with the project's testing conventions.Based on learnings
Also applies to: 52-62
fdm-core/src/db/schema-calculator.ts (1)
18-32: Consider making error_message non-nullable.The
calculation_errorstable design effectively supports error investigation. However, consider makingerror_messagenon-nullable, as capturing the error message is the primary purpose of this table. An error entry without an error message would provide limited debugging value.Apply this diff if error messages should always be captured:
export const calculationErrors = fdmCalculatorSchema.table( "calculation_errors", { calculation_error_id: text().primaryKey(), calculation_function: text(), calculator_version: text(), input: jsonb(), - error_message: text(), + error_message: text().notNull(), stack_trace: text(), created_at: timestamp({ withTimezone: true }).notNull().defaultNow(), }, )fdm-core/src/calculator.ts (1)
210-242: Remove or replace commented-out debug logs.The commented-out
console.logstatements should either be removed or replaced with a proper logging infrastructure (e.g., using a logging library with configurable log levels).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
fdm-core/src/db/migrations/0017_first_ezekiel.sqlis excluded by!fdm-core/src/db/migrations/**fdm-core/src/db/migrations/meta/0017_snapshot.jsonis excluded by!fdm-core/src/db/migrations/**fdm-core/src/db/migrations/meta/_journal.jsonis excluded by!fdm-core/src/db/migrations/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (34)
.changeset/clean-friends-sniff.md(1 hunks).changeset/deep-words-pick.md(1 hunks).changeset/public-rabbits-mix.md(1 hunks).changeset/tricky-ears-dress.md(1 hunks).changeset/witty-animals-march.md(1 hunks)fdm-app/app/integrations/nmi.ts(0 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx(3 hunks)fdm-calculator/src/balance/nitrogen/index.ts(3 hunks)fdm-calculator/src/balance/nitrogen/target.ts(1 hunks)fdm-calculator/src/index.ts(2 hunks)fdm-calculator/src/norms/index.test.ts(2 hunks)fdm-calculator/src/norms/index.ts(2 hunks)fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.ts(8 hunks)fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.ts(2 hunks)fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.ts(17 hunks)fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.ts(3 hunks)fdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.test.ts(8 hunks)fdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.ts(4 hunks)fdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.test.ts(1 hunks)fdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.ts(2 hunks)fdm-calculator/src/norms/nl/2025/value/fosfaatgebruiksnorm.ts(2 hunks)fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.test.ts(21 hunks)fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts(2 hunks)fdm-calculator/src/nutrient-advice/index.test.ts(1 hunks)fdm-calculator/src/nutrient-advice/index.ts(1 hunks)fdm-calculator/src/nutrient-advice/types.d.ts(1 hunks)fdm-core/package.json(2 hunks)fdm-core/src/calculator.test.ts(1 hunks)fdm-core/src/calculator.ts(1 hunks)fdm-core/src/db/schema-calculator.ts(1 hunks)fdm-core/src/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- fdm-app/app/integrations/nmi.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx
- fdm-core/src/calculator.test.ts
🧰 Additional context used
🧠 Learnings (36)
📓 Common learnings
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-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/balance/nitrogen/index.ts.changeset/public-rabbits-mix.mdfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts.changeset/deep-words-pick.mdfdm-calculator/src/norms/index.ts.changeset/witty-animals-march.mdfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsxfdm-core/package.jsonfdm-core/src/index.ts.changeset/tricky-ears-dress.mdfdm-core/src/calculator.tsfdm-core/src/db/schema-calculator.tsfdm-calculator/src/index.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/src/balance/nitrogen/index.ts.changeset/public-rabbits-mix.mdfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts.changeset/deep-words-pick.md.changeset/witty-animals-march.mdfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsxfdm-core/package.json
📚 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/balance/nitrogen/index.ts.changeset/public-rabbits-mix.mdfdm-calculator/src/norms/nl/2025/value/fosfaatgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsx.changeset/deep-words-pick.md.changeset/clean-friends-sniff.mdfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsx.changeset/tricky-ears-dress.mdfdm-calculator/src/nutrient-advice/index.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/index.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.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-calculator/src/balance/nitrogen/index.tsfdm-calculator/src/norms/nl/2025/value/fosfaatgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.test.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.tsfdm-calculator/src/norms/index.tsfdm-calculator/src/norms/index.test.tsfdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsxfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.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/balance/nitrogen/index.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsxfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.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/index.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-calculator/src/index.ts
📚 Learning: 2024-11-25T12:42:32.783Z
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`.
Applied to files:
.changeset/public-rabbits-mix.md.changeset/deep-words-pick.mdfdm-core/package.json
📚 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/nutrient-advice/index.test.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.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/nutrient-advice/index.test.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.ts
📚 Learning: 2025-03-06T15:23:29.958Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/catalogues.ts:96-132
Timestamp: 2025-03-06T15:23:29.958Z
Learning: When writing tests for the FDM codebase, avoid using Vitest's `vi` mocking utilities (`vi.spyOn()`, etc.). Prefer using natural errors, real function behavior, and actual inputs that trigger expected failures.
Applied to files:
fdm-calculator/src/nutrient-advice/index.test.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.ts
📚 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/nutrient-advice/index.test.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.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.test.tsfdm-calculator/src/norms/index.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.test.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-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsxfdm-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/value/dierlijke-mest-gebruiksnorm.test.tsfdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.tsfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.test.tsfdm-calculator/src/nutrient-advice/index.tsfdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.tsfdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.test.tsfdm-calculator/src/nutrient-advice/types.d.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-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsx
📚 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-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-calculator/src/index.ts
📚 Learning: 2025-05-26T10:32:15.538Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx
📚 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-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsxfdm-core/src/calculator.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-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsx
📚 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-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx
📚 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-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsxfdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.test.ts
📚 Learning: 2025-01-23T15:17:23.028Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsxfdm-core/src/calculator.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-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsx
📚 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-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsxfdm-core/src/calculator.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-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsx
📚 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/norms/nl/2025/filling/stikstofgebruiksnorm.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-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.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: 2025-01-24T11:28:01.882Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-24T11:28:01.882Z
Learning: The `addField` function in fdm-core should use database transactions to ensure atomicity, and since transactions provide ACID guarantees, awaiting the inserts is sufficient to ensure field availability - no additional verification queries are needed.
Applied to files:
fdm-core/src/calculator.ts
📚 Learning: 2025-01-23T15:17:23.027Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.027Z
Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.
Applied to files:
fdm-core/src/calculator.ts
📚 Learning: 2025-03-04T11:09:08.169Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 88
File: fdm-core/src/cultivation.ts:246-246
Timestamp: 2025-03-04T11:09:08.169Z
Learning: In the FDM codebase, the `fdm` parameter should be documented as "The FDM instance providing the connection to the database. The instance can be created with {link createFdmServer}." in JSDoc comments.
Applied to files:
fdm-core/src/calculator.ts
📚 Learning: 2025-04-18T14:20:40.975Z
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 schema defined in fdm-core/src/db/schema-authn.ts follows better-auth's structure and requirements. While the schema is defined in the application code, modifications to it should maintain compatibility with better-auth's expectations.
Applied to files:
fdm-core/src/db/schema-calculator.ts
📚 Learning: 2025-04-18T14:20:40.975Z
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.
Applied to files:
fdm-core/src/db/schema-calculator.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/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.ts
🔇 Additional comments (39)
fdm-core/package.json (1)
75-75: Addition of stable serialization dependency is appropriate.The
safe-stable-stringifypackage supports deterministic hashing for the caching layer, aligning with the PR's calculation caching objectives.fdm-calculator/src/balance/nitrogen/target.ts (1)
31-39: UsingforEachmatches the intentSwitching to
forEachdrops the unused array allocation while keeping the side effects intact. ✅fdm-calculator/src/norms/nl/2025/filling/stikstofgebruiksnorm.test.ts (1)
1-763: LGTM! Function rename is consistent throughout the test suite.The rename from
calculateNL2025FertilizerApplicationFillingForNitrogentocalculateNL2025FertilizerApplicationFillingForStikstofGebruiksNormis applied consistently across imports, test suite description, and all function invocations. The test logic and assertions remain unchanged, ensuring behavioral consistency..changeset/public-rabbits-mix.md (1)
1-5: LGTM! Changeset correctly documents the switch to cached calculator functions.The changeset appropriately marks this as a patch-level change for the fdm-app package.
.changeset/tricky-ears-dress.md (1)
1-5: LGTM! Changeset correctly documents the new cached calculator functions.The changeset appropriately marks this as a minor version bump for the fdm-calculator package, which is correct for new feature additions.
.changeset/deep-words-pick.md (1)
1-5: LGTM! Changeset correctly documents the migration of getNutrientAdvice.The changeset appropriately marks this as a patch-level change for moving the function to fdm-calculator and using its cached version.
.changeset/clean-friends-sniff.md (1)
1-5: LGTM! Changeset correctly documents the new nutrient advice functions.The changeset appropriately marks this as a minor version bump for adding new public exports to fdm-calculator.
fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.test.ts (1)
1-589: LGTM! Formatting consistency improvements.The changes remove extra spaces after
awaitkeywords throughout the test file, improving code consistency without affecting test behavior.fdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.test.ts (1)
9-37: LGTM! Improved test coverage with explicit derogation scenarios.The tests now explicitly cover both derogation and non-derogation cases with expanded mock inputs including field, cultivations, and soilAnalysis. The first test now correctly tests the default (non-derogation) scenario, while a new dedicated test covers the derogation case.
fdm-core/src/index.ts (1)
26-31: LGTM! New calculator cache utilities added to public API.The exports for
getCachedCalculation,setCachedCalculation,setCalculationError, andwithCalculationCacheappropriately expand the public API surface to enable external packages to leverage the calculation caching layer introduced in this PR.fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx (1)
3-3: LGTM! Clean migration to cached API.The import and function call have been cleanly updated to use the new
getNitrogenBalancewrapper, which adds database-backed caching while maintaining the existing calculation logic.Also applies to: 101-101
fdm-calculator/src/norms/index.test.ts (1)
10-17: LGTM! Test updates align with API renames.The test imports and expectations have been correctly updated to reference the new memoized
getNL2025...wrapper functions, ensuring the factory functions return the cached variants.Also applies to: 23-30, 59-67
fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts (1)
1-1: LGTM! Well-documented memoized wrapper.The cached wrapper follows a clean pattern with version-aware cache invalidation via
pkg.calculatorVersion. The JSDoc clearly explains the caching behavior and delegates to the underlying calculation function.Also applies to: 5-5, 647-663
fdm-calculator/src/norms/nl/2025/value/fosfaatgebruiksnorm.ts (1)
1-1: LGTM! Consistent caching implementation.The memoized wrapper follows the same clean pattern as other norm calculation functions, with version-aware caching and clear documentation.
Also applies to: 3-3, 177-192
fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.test.ts (1)
3-3: LGTM! Comprehensive test updates.All test imports, describe blocks, and function calls have been consistently updated to use the renamed
calculateNL2025FertilizerApplicationFillingForDierlijkeMestGebruiksNormfunction.Also applies to: 6-6, 45-45, 65-65, 89-89, 116-116, 136-136, 156-156, 175-175
fdm-calculator/src/nutrient-advice/index.ts (2)
93-97: LGTM! Consistent caching pattern.The
getNutrientAdvicewrapper follows the established pattern with version-aware caching, consistent with other calculator functions in this PR.
20-85: The search results are about Network Merchants payment gateway, not the Dutch agricultural NMI API. Let me search more specifically for the Dutch agricultural API:Looking at the codebase findings and the existing usage patterns, I can now generate the rewritten review comment:
No format validation needed; the nl_ prefix is enforced upstream.
The split operation on line 52 is safe because
b_lu_catalogueoriginates exclusively from BRP (BasisRegistratiePercelen) cultivation codes, which consistently follow the "nl_XXXX" format. This is evident from:
- Line 222 of
fdm-app/app/routes/.../upload.tsxexplicitly constructs:const b_lu_catalogue =nl_${GEWASCODE}``- All actual BRP values in the codebase use the "nl_" prefix (nl_2014, nl_265, nl_1010, etc.)
- Two existing app routes already use identical split logic without error handling (lines 184 and 157), indicating this pattern is accepted architectural practice
- Test data with non-nl_ formats (test-id, cat-cult-1) are mock fixtures that never reach this function in production
The requestNutrientAdvice function specifically expects BRP codes for the NMI API, so format validation is unnecessary.
fdm-calculator/src/norms/nl/2025/filling/fosfaatgebruiksnorm.test.ts (1)
3-3: LGTM! Test updates align with function rename.The test file has been comprehensively updated to use the renamed
calculateNL2025FertilizerApplicationFillingForFosfaatGebruiksNormfunction across all test cases.Also applies to: 80-80
fdm-calculator/src/index.ts (3)
1-2: LGTM! Calculator version export for cache invalidation.The fdmCalculator export provides access to the calculator version metadata, enabling version-aware cache invalidation across the caching layer.
4-6: LGTM! Dual export pattern for cached and direct calculation.Exporting both
getNitrogenBalance(cached) andcalculateNitrogenBalance(direct) gives consumers the flexibility to choose between cached and uncached computation.
51-59: LGTM! Complete nutrient advice API surface.The nutrient advice exports provide both the cached wrapper (
getNutrientAdvice) and direct request function (requestNutrientAdvice) along with all necessary types, following the established pattern from nitrogen balance.fdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.ts (2)
1-4: LGTM! Imports support caching infrastructure.The added imports provide the necessary infrastructure for the memoized wrapper:
withCalculationCachefor the caching HOF andpkgfor version-aware cache invalidation.
196-210: LGTM! Well-documented memoized wrapper.The cached wrapper implementation is clean and well-documented, using the underlying function name as the cache key and the calculator version for cache invalidation. This pattern ensures cached results are properly invalidated when the calculator logic changes.
fdm-calculator/src/norms/nl/2025/filling/dierlijke-mest-gebruiksnorm.ts (3)
4-5: LGTM! Caching infrastructure imports.The imports support the new memoization wrapper, consistent with the pattern used in other norm calculation modules.
18-99: LGTM! Function renamed with improved formatting.The function rename to
calculateNL2025FertilizerApplicationFillingForDierlijkeMestGebruiksNormprovides better naming consistency. The refactoring improves code organization with clearer comments and better structure, while preserving the calculation logic and error handling.
101-115: LGTM! Consistent memoization pattern.The cached wrapper follows the established pattern with proper documentation and version-aware cache invalidation.
fdm-app/app/routes/farm.$b_id_farm.$calendar.norms._index.tsx (2)
164-176: LGTM! Consistent API update for norm calculations.All three norm calculation functions now correctly receive the
fdminstance as the first parameter, enabling database-backed caching while preserving parallel execution.
193-205: LGTM! Consistent API update for filling calculations.All three fertilizer filling calculation functions now correctly receive the
fdminstance as the first parameter, maintaining consistency with the norm calculations and enabling the caching layer.fdm-calculator/src/nutrient-advice/types.d.ts (3)
5-40: LGTM! Comprehensive nutrient advice type with clear documentation.The
NutrientAdvicetype is well-structured with clear JSDoc comments indicating units for each nutrient. The distinction between requirements (_req) and norms (_norm) for nitrogen and phosphate aligns with agricultural best practices.
45-71: LGTM! Well-structured API response type.The
NutrientAdviceResponsetype follows standard API response patterns with proper metadata fields. The optionalcutarray for grassland-specific per-cut advice is appropriately typed with constrained yield class literals and cut numbers.
76-85: LGTM! Appropriate input type definition.The
NutrientAdviceInputstype captures all necessary parameters for nutrient advice requests. The centroid tuple type ensures proper coordinate structure, and the optional API key allows for flexible authentication approaches.fdm-calculator/src/norms/index.ts (3)
5-12: LGTM! Imports updated to cached variants.All NL 2025 norm and filling imports now use the
get*variants (cached wrappers), ensuring all calculations benefit from the database-backed caching layer while maintaining backward compatibility.
20-22: LGTM! Transparent caching through stable public API.The function mappings now use cached
get*variants internally while preserving the public API wrapper names (calculateNormFor*), ensuring transparent caching for all consumers without breaking changes.
40-45: LGTM! Consistent caching pattern for filling calculations.The fertilizer filling function mappings follow the same pattern as norm calculations, using cached
get*variants internally while maintaining stable public wrapper names for backward compatibility.fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsx (2)
197-199: LGTM! Norm calculations updated consistently.The norm calculation functions now receive the
fdminstance as the first parameter, consistent with the changes in the norms index route and enabling database-backed caching.
212-224: LGTM! Filling calculations updated consistently.The fertilizer filling calculation functions now receive the
fdminstance as the first parameter, maintaining consistency across all norm and filling calculations in the application.fdm-core/src/db/schema-calculator.ts (1)
3-4: LGTM! Dedicated schema for calculator caching.The dedicated
fdm-calculatorschema provides clear separation of concerns, isolating calculation cache data from other application data.fdm-core/src/calculator.ts (2)
11-38: LGTM: Deterministic hash generation is well-implemented.The use of
safe-stable-stringifyensures consistent serialization, and SHA-256 provides a collision-resistant hash. The combination of function name, version, and input guarantees proper cache invalidation when any component changes.
40-66: LGTM: Cache retrieval logic is sound.The query correctly uses the calculation_hash as the lookup key and properly handles the case when no cached result exists by returning
null.
| export function calculateNL2025FertilizerApplicationFillingForManure( | ||
| input: NL2025NormsFillingInput, | ||
| export function calculateNL2025FertilizerApplicationFillingForDierlijkeMestGebruiksNorm( | ||
| input: NL2025NormsFillingInput, |
There was a problem hiding this comment.
do you by purpose swith from english (manure) to dierlijkemestgebruiksnorm?
There was a problem hiding this comment.
Yes, to be consistent with the value functions
|
|
||
| /** | ||
| * Memoized version of {@link calculateNL2025FertilizerApplicationFillingForDierlijkeMestGebruiksNorm}. | ||
| * |
There was a problem hiding this comment.
3 participants
Enhancements
fdm-calculatorschema.Summary by CodeRabbit
New Features
Improvements
Refactor