Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces configurable "pick first" disambiguation behavior for WorkBench matching. A new DisambiguationBehavior type ("ask" | "pickFirst") is added to backend and frontend types and JSON schema, threaded through parsing and ParseResult, applied in upload-table match resolution to return the first match when configured, and exposed in the frontend mapping UI and reducer. Defaults remain "ask"; tests and fixtures updated. ChangesWorkBench Disambiguation Behavior
Suggested reviewers
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
specifyweb/backend/workbench/upload/column_options.py (1)
21-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
disambiguationBehaviorin shorthand serialization.At Line [21], the shorthand return (
return self.column) dropsdisambiguationBehavior. ApickFirstsetting with otherwise default options is lost on save/load and reverts to"ask".Proposed fix
def to_json(self) -> dict | str: - if self.matchBehavior == "ignoreNever" and self.nullAllowed and self.default is None: + if ( + self.matchBehavior == "ignoreNever" + and self.nullAllowed + and self.default is None + and self.disambiguationBehavior == "ask" + ): return self.column🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/backend/workbench/upload/column_options.py` around lines 21 - 24, The shorthand branch in the serializer returns self.column and thereby drops self.disambiguationBehavior (losing a pickFirst setting); update the conditional that returns self.column in the method using self.matchBehavior/self.nullAllowed/self.default so that the returned dict includes disambiguationBehavior when it is set (or when it differs from the default "ask") — e.g., construct a copy of self.column and inject {"disambiguationBehavior": self.disambiguationBehavior} (or always include it) before returning, keeping the rest of the shorthand logic intact.
🧹 Nitpick comments (1)
specifyweb/backend/workbench/upload/parsing.py (1)
35-47: ⚡ Quick winPrefer keyword construction for
ParseResult.This PR had to update every positional
ParseResult(...)/from_parse_success(...)call just to add one field. Switching these to keyword args, or routing the blank-value case through a helper, would make the next shape change much harder to miss.♻️ Suggested cleanup
`@classmethod` -def from_parse_success(cls, ps: ParseSucess, filter_on: Filter, add_to_picklist: PicklistAddition | None, column: str, - missing_required: str | None, disambiguationBehavior: DisambiguationBehavior): - return cls(filter_on=filter_on, upload=ps.payload, add_to_picklist=add_to_picklist, column=column, - missing_required=missing_required, disambiguationBehavior=disambiguationBehavior) +def from_parse_success( + cls, + ps: ParseSucess, + filter_on: Filter, + add_to_picklist: PicklistAddition | None, + column: str, + missing_required: str | None, + disambiguationBehavior: DisambiguationBehavior, +): + return cls( + filter_on=filter_on, + upload=ps.payload, + add_to_picklist=add_to_picklist, + column=column, + missing_required=missing_required, + disambiguationBehavior=disambiguationBehavior, + ) def filter_and_upload(f: Filter, column: str, disambiguation_behavior: DisambiguationBehavior = 'ask') -> ParseResult: - return ParseResult(f, f, None, column, None, disambiguation_behavior) + return ParseResult( + filter_on=f, + upload=f, + add_to_picklist=None, + column=column, + missing_required=None, + disambiguationBehavior=disambiguation_behavior, + )Also applies to: 53-54, 81-82
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/backend/workbench/upload/parsing.py` around lines 35 - 47, Update all constructions of ParseResult and calls to ParseResult.from_parse_success to use explicit keyword arguments for every field (e.g., filter_on=..., upload=..., add_to_picklist=..., column=..., missing_required=..., disambiguationBehavior=...) instead of relying on positional ordering; this includes the other uses of ParseResult/from_parse_success in this module. If there are places that pass a placeholder/empty value for a field, route that through the existing from_parse_success helper (or add a small helper) so callers don’t rely on positional defaults. Aim to make every ParseResult(...) invocation use named parameters so future shape changes won’t break silencely.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@specifyweb/backend/workbench/upload/tests/testparsing.py`:
- Around line 886-900: The test is asserting the top-level CollectionObject
result instead of the nested Agent disambiguation; change the first assertion to
expect results[0].record_result to be Uploaded (since CO '123' is new) and add
an assertion that the nested agent outcome is Matched by checking
results[0].toOne['cataloger'].record_result is an instance of Matched; keep the
existing assertion that results[1].record_result is Uploaded so the second row's
CO behavior is still validated (refer to do_upload, results, record_result,
toOne['cataloger'], Matched, Uploaded).
In `@specifyweb/backend/workbench/upload/upload_plan_schema.py`:
- Around line 277-281: The JSON schema for the property disambiguationBehavior
has an invalid default "ignoreNever" that isn't listed in its enum; update the
schema for disambiguationBehavior (property name) so its "default" is one of the
allowed enum values ("ask" or "pickFirst") or remove the "default" key entirely
— e.g., change the default to "pickFirst" or "ask" to restore internal
consistency with the enum.
In `@specifyweb/frontend/js_src/lib/components/WbPlanView/mappingReducer.ts`:
- Around line 405-421: ChangeDisambiguationBehaviorAction updates matched lines
but incorrectly uses the triggering line's columnOptions as the base
(state.lines[action.line].columnOptions), which overwrites other lines'
matchBehavior/nullAllowed/default; change the spread to use the current
iteration's columnOptions (line.columnOptions) when constructing the new
columnOptions for each matched line, keeping disambiguationBehavior from action
and preserving each line's existing options; locate
ChangeDisambiguationBehaviorAction and replace the base spread accordingly while
still using isSameMappingPath(mappingPath, ...) to select lines and setting
changesMade: true.
---
Outside diff comments:
In `@specifyweb/backend/workbench/upload/column_options.py`:
- Around line 21-24: The shorthand branch in the serializer returns self.column
and thereby drops self.disambiguationBehavior (losing a pickFirst setting);
update the conditional that returns self.column in the method using
self.matchBehavior/self.nullAllowed/self.default so that the returned dict
includes disambiguationBehavior when it is set (or when it differs from the
default "ask") — e.g., construct a copy of self.column and inject
{"disambiguationBehavior": self.disambiguationBehavior} (or always include it)
before returning, keeping the rest of the shorthand logic intact.
---
Nitpick comments:
In `@specifyweb/backend/workbench/upload/parsing.py`:
- Around line 35-47: Update all constructions of ParseResult and calls to
ParseResult.from_parse_success to use explicit keyword arguments for every field
(e.g., filter_on=..., upload=..., add_to_picklist=..., column=...,
missing_required=..., disambiguationBehavior=...) instead of relying on
positional ordering; this includes the other uses of
ParseResult/from_parse_success in this module. If there are places that pass a
placeholder/empty value for a field, route that through the existing
from_parse_success helper (or add a small helper) so callers don’t rely on
positional defaults. Aim to make every ParseResult(...) invocation use named
parameters so future shape changes won’t break silencely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2bf69460-911b-4e80-aace-d8b058412006
📒 Files selected for processing (17)
specifyweb/backend/workbench/upload/column_options.pyspecifyweb/backend/workbench/upload/parsing.pyspecifyweb/backend/workbench/upload/scoping.pyspecifyweb/backend/workbench/upload/tests/testparsing.pyspecifyweb/backend/workbench/upload/upload_attachments.pyspecifyweb/backend/workbench/upload/upload_plan_schema.pyspecifyweb/backend/workbench/upload/upload_table.pyspecifyweb/frontend/js_src/lib/components/WbPlanView/Mapper.tsxspecifyweb/frontend/js_src/lib/components/WbPlanView/MapperComponents.tsxspecifyweb/frontend/js_src/lib/components/WbPlanView/__tests__/linesGetter.test.tsspecifyweb/frontend/js_src/lib/components/WbPlanView/linesGetter.tsspecifyweb/frontend/js_src/lib/components/WbPlanView/mappingReducer.tsspecifyweb/frontend/js_src/lib/components/WbPlanView/uploadPlanParser.tsspecifyweb/frontend/js_src/lib/localization/wbPlan.tsspecifyweb/frontend/js_src/lib/tests/fixtures/mappinglines.1.jsonspecifyweb/frontend/js_src/lib/tests/fixtures/uploadplan.1.jsonspecifyweb/frontend/js_src/lib/tests/fixtures/wbplanviewlines.1.json
github-project-automation
Bot
moved this from 📋Back Log
to Dev Attention Needed
in General Tester Board
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3922
Checklist
self-explanatory (or properly documented)
Testing instructions
Summary by CodeRabbit
New Features
UI
Tests