Extend field productivity detection with name-based check#318
Conversation
🦋 Changeset detectedLatest commit: 0179337 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 |
WalkthroughRenames Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant FieldModule as field.ts
Note right of FieldModule #DDEEFF: determineIfFieldIsProductive(area, perimeter, name)
Caller->>FieldModule: determineIfFieldIsProductive(area, perimeter, name)
alt Shape-based check
FieldModule->>FieldModule: compute shapeMetric = 4π*area / perimeter²\ncheck area threshold
end
alt Name-based check
FieldModule->>FieldModule: check name.toLowerCase().includes("buffer")
end
FieldModule->>Caller: return (shapeOk && !nameHasBuffer)
Note left of Caller #F6F6F6: b_isproductive assigned from result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development #318 +/- ##
===============================================
+ Coverage 93.10% 93.11% +0.01%
===============================================
Files 81 81
Lines 13316 13322 +6
Branches 1360 1361 +1
===============================================
+ Hits 12398 12405 +7
+ Misses 916 915 -1
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:
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-core/src/field.ts (1)
644-661: Update the function description to reflect both checks.The JSDoc now includes the
@param b_namedocumentation, but the function description (lines 645-649) still only describes the shape-based heuristic. Update the description to mention that the function now also checks if the field name contains "buffer" (case-insensitive) and returnstrueonly when both the shape-based and name-based checks pass.Apply this diff to update the JSDoc:
/** - * Determines if a field is considered productive based on its area and perimeter. + * Determines if a field is considered productive based on its area, perimeter, and name. * - * This function uses a heuristic to differentiate between productive fields and non-productive areas like buffer strips. - * A field is classified as non-productive if its area is less than 2.5 hectares and the ratio of its perimeter - * to the square root of its area (in square meters) is greater than or equal to a predefined constant (20). + * This function uses two heuristics to differentiate between productive fields and non-productive areas like buffer strips: + * 1. Shape-based: A field is classified as non-productive if its area is less than 2.5 hectares and the ratio of its perimeter + * to the square root of its area (in square meters) is greater than or equal to a predefined constant (20). + * 2. Name-based: A field is classified as non-productive if its name contains "buffer" (case-insensitive). + * + * A field is considered productive only if both checks pass. * * @param b_area The area of the field in hectares. * @param b_perimeter The perimeter of the field in meters. * @param b_name The name of the field. * @returns `true` if the field is determined to be productive, `false` otherwise. * @alpha */
🧹 Nitpick comments (1)
fdm-core/src/field.test.ts (1)
955-997: LGTM with a suggestion for enhanced coverage.The test suite provides good coverage of the new combined logic, testing both shape-based and name-based productivity checks. All test cases correctly validate the expected behavior of the function.
Consider adding a test case where the shape-based check fails due to both high perimeter/area ratio AND small area (not just small area with acceptable ratio), combined with a name that contains "buffer". This would more thoroughly exercise the conjunction of the two failing conditions.
Example:
it("should determine field is not productive when both shape and name checks fail", async () => { const b_isproductive = determineIfFieldIsProductive( 1.0, // Small area < 2.5 ha 10000.0, // High perimeter creates ratio > 20 "Buffer Zone", // Name contains 'buffer' ) expect(b_isproductive).toBe(false) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/great-bars-run.md(1 hunks)fdm-core/src/cultivation.ts(2 hunks)fdm-core/src/field.test.ts(3 hunks)fdm-core/src/field.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (18)
📓 Common learnings
Learnt from: SvenVw
PR: SvenVw/fdm#279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Learnt from: SvenVw
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.
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: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).
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.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.
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.
📚 Learning: 2025-01-31T15:05:14.310Z
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.
Applied to files:
fdm-core/src/field.tsfdm-core/src/field.test.tsfdm-core/src/cultivation.ts
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
PR: SvenVw/fdm#279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
fdm-core/src/field.tsfdm-core/src/field.test.ts.changeset/great-bars-run.mdfdm-core/src/cultivation.ts
📚 Learning: 2025-01-31T15:05:14.310Z
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).
Applied to files:
fdm-core/src/field.tsfdm-core/src/field.test.tsfdm-core/src/cultivation.ts
📚 Learning: 2025-09-26T08:26:05.718Z
Learnt from: SvenVw
PR: SvenVw/fdm#279
File: fdm-core/src/field.ts:169-169
Timestamp: 2025-09-26T08:26:05.718Z
Learning: In agricultural field management systems (fdm-core), when calculating field perimeter for productivity classification heuristics, holes inside polygons should be ignored. Use ST_ExteriorRing instead of ST_Perimeter to get only the outer boundary perimeter, as interior features like ponds or buildings should not affect the buffer strip vs productive field classification.
Applied to files:
fdm-core/src/field.tsfdm-core/src/cultivation.ts
📚 Learning: 2025-01-31T15:34:20.850Z
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.
Applied to files:
fdm-core/src/field.tsfdm-core/src/field.test.tsfdm-core/src/cultivation.ts
📚 Learning: 2025-01-31T15:05:14.310Z
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.
Applied to files:
fdm-core/src/field.tsfdm-core/src/cultivation.ts
📚 Learning: 2025-01-23T15:17:23.027Z
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.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/field.tsfdm-core/src/field.test.ts
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
PR: SvenVw/fdm#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-core/src/field.tsfdm-core/src/field.test.tsfdm-core/src/cultivation.ts
📚 Learning: 2025-01-23T15:17:23.028Z
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.
Applied to files:
fdm-core/src/field.tsfdm-core/src/field.test.tsfdm-core/src/cultivation.ts
📚 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-core/src/field.test.tsfdm-core/src/cultivation.ts
📚 Learning: 2025-08-13T10:33:05.313Z
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.
Applied to files:
fdm-core/src/field.test.tsfdm-core/src/cultivation.ts
📚 Learning: 2025-01-23T15:18:57.212Z
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.
Applied to files:
fdm-core/src/field.test.ts
📚 Learning: 2025-02-13T08:35:59.306Z
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.
Applied to files:
fdm-core/src/field.test.ts
📚 Learning: 2025-08-11T12:14:28.354Z
Learnt from: SvenVw
PR: SvenVw/fdm#233
File: fdm-core/src/db/schema.ts:370-0
Timestamp: 2025-08-11T12:14:28.354Z
Learning: In the FDM database schema, the field `b_lu_rest_oravib` in the `cultivations_catalogue` table is intentionally nullable (can be undefined). When undefined, it should be treated as `false` in the UI layer. This allows distinguishing between "explicitly set to false" and "not set" at the database level while providing simplified boolean logic in the UI.
Applied to files:
fdm-core/src/field.test.ts
📚 Learning: 2024-11-27T12:15:36.425Z
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();`.
Applied to files:
fdm-core/src/cultivation.ts
📚 Learning: 2025-02-13T09:03:11.890Z
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.
Applied to files:
fdm-core/src/cultivation.ts
📚 Learning: 2024-11-27T11:27:27.797Z
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.
Applied to files:
fdm-core/src/cultivation.ts
🧬 Code graph analysis (3)
fdm-core/src/field.ts (1)
fdm-core/src/db/schema.ts (1)
fieldsTypeSelect(107-107)
fdm-core/src/field.test.ts (1)
fdm-core/src/field.ts (1)
determineIfFieldIsProductive(657-672)
fdm-core/src/cultivation.ts (1)
fdm-core/src/field.ts (1)
determineIfFieldIsProductive(657-672)
🔇 Additional comments (8)
.changeset/great-bars-run.md (1)
1-5: LGTM!The changeset is correctly formatted and clearly describes the enhancement to the
b_isproductivecheck.fdm-core/src/cultivation.ts (2)
26-26: LGTM!The import correctly reflects the updated function name and signature.
919-923: LGTM!The function call is correctly updated to pass the field name as the third argument, enabling name-based productivity detection. The field name is properly retrieved from the database query.
fdm-core/src/field.test.ts (2)
12-12: LGTM!The import correctly includes the updated function name.
768-769: LGTM!The test data additions properly include the required default date fields in the correct MM-dd format.
fdm-core/src/field.ts (3)
662-672: LGTM!The implementation correctly combines both shape-based and name-based productivity checks. The use of
toLowerCase().includes("buffer")provides case-insensitive matching, which is appropriate for field names. The conjunction of both checks ensures that a field is only marked as productive when it passes both criteria, aligning with the objective stated in issue #317.
190-194: LGTM!The call site correctly invokes the updated function with all three required parameters, including the field name retrieved from the database query.
304-308: LGTM!The call site correctly invokes the updated function with all three required parameters for each field in the loop. The field name is properly retrieved from the database query.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-core/src/field.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-01-31T15:05:14.310Z
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.
Applied to files:
fdm-core/src/field.ts
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
PR: SvenVw/fdm#279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
fdm-core/src/field.ts
📚 Learning: 2025-01-31T15:05:14.310Z
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).
Applied to files:
fdm-core/src/field.ts
📚 Learning: 2025-01-31T15:34:20.850Z
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.
Applied to files:
fdm-core/src/field.ts
📚 Learning: 2025-01-31T15:05:14.310Z
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.
Applied to files:
fdm-core/src/field.ts
📚 Learning: 2025-01-23T15:17:23.027Z
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.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/field.ts
📚 Learning: 2025-01-23T15:17:23.028Z
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.
Applied to files:
fdm-core/src/field.ts
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
PR: SvenVw/fdm#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-core/src/field.ts
📚 Learning: 2025-08-13T10:33:05.313Z
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.
Applied to files:
fdm-core/src/field.ts
📚 Learning: 2025-04-18T14:51:48.033Z
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 will be updated in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5 to include id and email fields, which are necessary for subsequent role updates and user removal operations.
Applied to files:
fdm-core/src/field.ts
🧬 Code graph analysis (1)
fdm-core/src/field.ts (1)
fdm-core/src/db/schema.ts (1)
fieldsTypeSelect(107-107)
🔇 Additional comments (3)
fdm-core/src/field.ts (3)
190-194: Call site updated correctly.The call to
determineIfFieldIsProductivenow correctly includes the field name as the third parameter.
304-308: Call site updated correctly.The call to
determineIfFieldIsProductivenow correctly includes the field name as the third parameter.
644-664: No issues found. The schema confirmsb_nameis constrained with.notNull()(fdm-core/src/db/schema.ts:93), eliminating any null safety risk. The function signature and documentation changes are appropriate and correct.
Summary by CodeRabbit
Bug Fixes
Tests
Closes #317