Auto-generated Pull Request for fix/list-plasmids#610
Merged
Conversation
Allow plasmid type filtering (Regular and GoldenBraid) in stock service query conversion. Refactor filter validation to only check id and in_stock fields, delegating type filtering to new plasmidTypeQuery function. Update plasmid type tag constants to match stock service expectations. Updates: - Add plasmidTypeQuery function to generate stock query for plasmid types - Modify buildListPlasmidFilterQuery to use TryCatchError for simpler error handling - Update tag constants: RegularPlasmidTag to "vector" and GoldenBraidPlasmidTag to "GB vector" - Add tests for plasmid type filtering functionality Decomposes complex validation pipeline into focused checks and adds type-specific query generation. 💘 Generated with Crush Assisted-by: Z.ai: GLM 4.7 Flash via Crush <crush@charm.land>
…ectations Update RegularPlasmidTag and GoldenBraidPlasmidTag constants to use "vector" and "GB vector" respectively, aligning with stock service query format. 💘 Generated with Crush Assisted-by: Z.ai: GLM 4.7 Flash via Crush <crush@charm.land>
📝 WalkthroughWalkthroughThe pull request refactors the plasmid filtering system to remove type verification checks that previously rejected unverified plasmid types. The validation pipeline is replaced with a consolidated function that directly generates query strings for plasmid type constraints. Plasmid tag constants are updated to new naming conventions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
internal/registry/registry.go (1)
42-43: Add Go doc comments for the exported plasmid tag constants.Line 42 and Line 43 update exported constants that now define stock-service filter behavior. Please add identifier-specific comments so future changes to
"vector"/"GB vector"are deliberate. As per coding guidelines, Document all exported functions, types, and constants with proper Go doc comments.📝 Proposed documentation update
- GoldenBraidPlasmidTag = "GB vector" - RegularPlasmidTag = "vector" + // GoldenBraidPlasmidTag identifies GoldenBraid plasmids in modware-annotation. + GoldenBraidPlasmidTag = "GB vector" + // RegularPlasmidTag identifies regular plasmids in modware-annotation. + RegularPlasmidTag = "vector"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/registry/registry.go` around lines 42 - 43, Add Go doc comments above the exported constants GoldenBraidPlasmidTag and RegularPlasmidTag describing their purpose and how they influence stock-service filter behavior; e.g., a short sentence for each explaining that GoldenBraidPlasmidTag ("GB vector") marks plasmids intended for GoldenBraid workflows and RegularPlasmidTag ("vector") marks standard vectors, so future changes to those string values are intentional and documented. Ensure comments follow Go doc style (start with the constant name) and are placed immediately above each constant declaration.internal/graphql/resolverutils/plasmid_filter.go (1)
54-54: Add a Go doc comment forPlasmidFilterToQuery.Line 54 exports a utility that now owns nil-filter handling and validation behavior, so the contract should be documented at the declaration. As per coding guidelines, Document all exported functions, types, and constants with proper Go doc comments.
📝 Proposed documentation update
+// PlasmidFilterToQuery converts a plasmid list filter into a stock-service filter query. +// A nil filter produces an empty query. func PlasmidFilterToQuery(filter *models.PlasmidListFilter) (string, error) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/graphql/resolverutils/plasmid_filter.go` at line 54, Add a Go doc comment above the exported function PlasmidFilterToQuery that documents its behavior and contract: state that it converts a *models.PlasmidListFilter to a SQL/query string, describe what happens when filter is nil (e.g., returns empty query or default), list validation behavior and possible errors returned (what conditions cause non-nil error), and mention any side-effects or assumptions (e.g., which fields are used). Reference the function name PlasmidFilterToQuery and keep the comment concise and in proper Go doc format starting with the function name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/graphql/resolverutils/plasmid_filter.go`:
- Around line 68-70: When plasmidTypeQuery(filter) returns an error, the current
code returns the partially built fieldQuery which may be misused; change the
error path so you return the zero-value/empty query instead of fieldQuery along
with the error. Locate the call to plasmidTypeQuery in plasmid_filter.go (the
variables typeQuery and fieldQuery) and replace the return of fieldQuery on
error with an empty query value (the zero-value for the query type) plus err so
callers won't get a partial query when type validation fails.
---
Nitpick comments:
In `@internal/graphql/resolverutils/plasmid_filter.go`:
- Line 54: Add a Go doc comment above the exported function PlasmidFilterToQuery
that documents its behavior and contract: state that it converts a
*models.PlasmidListFilter to a SQL/query string, describe what happens when
filter is nil (e.g., returns empty query or default), list validation behavior
and possible errors returned (what conditions cause non-nil error), and mention
any side-effects or assumptions (e.g., which fields are used). Reference the
function name PlasmidFilterToQuery and keep the comment concise and in proper Go
doc format starting with the function name.
In `@internal/registry/registry.go`:
- Around line 42-43: Add Go doc comments above the exported constants
GoldenBraidPlasmidTag and RegularPlasmidTag describing their purpose and how
they influence stock-service filter behavior; e.g., a short sentence for each
explaining that GoldenBraidPlasmidTag ("GB vector") marks plasmids intended for
GoldenBraid workflows and RegularPlasmidTag ("vector") marks standard vectors,
so future changes to those string values are intentional and documented. Ensure
comments follow Go doc style (start with the constant name) and are placed
immediately above each constant declaration.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e369fecf-d0b3-4018-b7a6-a139e4153b43
📒 Files selected for processing (5)
internal/graphql/resolver/stock_plasmid_fp.gointernal/graphql/resolver/stock_test.gointernal/graphql/resolverutils/plasmid_filter.gointernal/graphql/resolverutils/plasmid_filter_test.gointernal/registry/registry.go
Comment on lines
+68
to
+70
| typeQuery, err := plasmidTypeQuery(filter) | ||
| if err != nil { | ||
| return fieldQuery, err |
There was a problem hiding this comment.
Return an empty query when type validation fails.
Line 70 returns fieldQuery together with err; for a filter like {Summary: ..., PlasmidType: "INVALID"}, direct callers could accidentally use the partial summary query despite the failed full conversion. Return the zero query on error.
🐛 Proposed fix
typeQuery, err := plasmidTypeQuery(filter)
if err != nil {
- return fieldQuery, err
+ return "", err
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| typeQuery, err := plasmidTypeQuery(filter) | |
| if err != nil { | |
| return fieldQuery, err | |
| typeQuery, err := plasmidTypeQuery(filter) | |
| if err != nil { | |
| return "", err | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/graphql/resolverutils/plasmid_filter.go` around lines 68 - 70, When
plasmidTypeQuery(filter) returns an error, the current code returns the
partially built fieldQuery which may be misused; change the error path so you
return the zero-value/empty query instead of fieldQuery along with the error.
Locate the call to plasmidTypeQuery in plasmid_filter.go (the variables
typeQuery and fieldQuery) and replace the return of fieldQuery on error with an
empty query value (the zero-value for the query type) plus err so callers won't
get a partial query when type validation fails.
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.
Pulling 'fix/list-plasmids into develop. Please review and merge.
Summary by CodeRabbit
Bug Fixes
Refactor