Repository navigation
Fix CodeFactor generated marker handling - #381
Conversation
Single-source repeated SDK list/search filter contracts and tool option contracts, share list/search filter echo handling, share create/update runtime unset resolution, and reuse the canonical config key type. Refresh generated package runtime loaders and plugin MCP launchers with explicit auto-generated markers so CodeFactor can classify generated files correctly; keep the local CodeFactor settings note aligned with hosted ignore behavior. PM evidence: pm-fmjy records the duplication remediation, full coverage, generator checks, static gates, security scan, changelog check, and isolated PM_PATH/PM_GLOBAL_PATH smoke. pm-arzz remains open for staged complexity refactors.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Reviewer's GuideCentralizes shared item filter echo and tool option contracts, unifies runtime unset field resolution across create/update, exports the canonical config key type for reuse, and refreshes generated loaders and MCP wrappers with explicit auto-generated markers and updated CodeFactor docs/evidence. Sequence diagram for shared item filter echo in list/search commandssequenceDiagram
participant ListCommand as list_command
participant SearchCommand as search_command
participant CommonEcho as applyCommonItemFilterEcho
ListCommand->>CommonEcho: applyCommonItemFilterEcho(filters, options)
CommonEcho-->>ListCommand: filters updated with common item filters
SearchCommand->>CommonEcho: applyCommonItemFilterEcho(filters, options)
CommonEcho-->>SearchCommand: filters updated with common item filters
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThe PR centralizes runtime unset-field resolution, reuses shared filter contract groups and compact filter echo logic, updates generated header text in loaders and MCP wrappers, and refreshes CodeFactor and PM tracking metadata. ChangesCLI unset and config typing
Shared filter contracts and echoes
Generated header wording sync
CodeFactor and PM metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
|
@codex review /gemini review @gemini-code-assist review @coderabbitai full review |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
applyCommonItemFilterEchohelper and its callers all work onRecord<string, unknown>; consider tightening these to specific option/filter types (or at least keyed string union types) so that future additions/removals toCOMMON_ITEM_FILTER_ECHO_ENTRIESare checked by the type system rather than only at runtime. - Now that
RuntimeUnsetFieldDefinitionandresolveRuntimeUnsetFieldDefinitionare shared between create/update, you may want to colocate or expose the commonUPDATE_UNSET_ALIAS_MAP/CREATE_UNSET_ALIAS_MAPlogic as well so that future changes to runtime unset behavior are less likely to diverge between commands.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `applyCommonItemFilterEcho` helper and its callers all work on `Record<string, unknown>`; consider tightening these to specific option/filter types (or at least keyed string union types) so that future additions/removals to `COMMON_ITEM_FILTER_ECHO_ENTRIES` are checked by the type system rather than only at runtime.
- Now that `RuntimeUnsetFieldDefinition` and `resolveRuntimeUnsetFieldDefinition` are shared between create/update, you may want to colocate or expose the common `UPDATE_UNSET_ALIAS_MAP`/`CREATE_UNSET_ALIAS_MAP` logic as well so that future changes to runtime unset behavior are less likely to diverge between commands.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@coderabbitai review |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
✅ Action performedReview finished.
|
Keep the PR focused on CodeFactor generated-file classification after the PR detail page reported 16 complexity findings on large command and SDK files touched by the initial dedupe refactor. Preserve the generator/template marker changes for package runtime loaders and plugin MCP wrappers, update PM evidence, and leave the command complexity work tracked by pm-arzz for a dedicated staged refactor. Validation: generator drift checks, focused runtime/generator tests, pnpm build, typecheck, contracts:check, quality:static, security:scan, audit, changelog:pm:check, full 100% coverage, isolated temp smoke, PM health, and Sentry/telemetry gate.
|
@codex review /gemini review @gemini-code-assist review @coderabbitai full review @coderabbitai review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
5 issues found across 18 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/cli/commands/update.ts">
<violation number="1">
P2: New local runtime-unset resolver duplicates existing create-command logic instead of sharing one primitive. Keep one shared resolver to avoid drift between create/update unset behavior.</violation>
</file>
<file name="src/sdk/cli-contracts/tool-option-contracts.ts">
<violation number="1">
P3: This reintroduces duplicated option-contract blocks across list and search arrays. Keep these options in shared constants and spread them into both arrays to avoid drift bugs.</violation>
</file>
<file name="src/cli/commands/search.ts">
<violation number="1">
P3: This block duplicates existing compact filter-echo mapping logic from list command. Duplicated mappings raise maintenance drift risk when filter keys/options change.</violation>
</file>
<file name="src/cli/commands/config.ts">
<violation number="1">
P2: `ConfigKey` is duplicated as a hard-coded literal union instead of being derived, adding another key list to maintain. Drift here can desync config command typing from positional routing key coverage.</violation>
</file>
<file name="src/sdk/cli-contracts.ts">
<violation number="1">
P3: SEARCH_FLAG_CONTRACTS duplicates filter-contract entries already declared in LIST_FILTER_FLAG_CONTRACTS. Keep these flags in a shared constant to avoid list/search contract drift.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
Record claim release for pm-fmjy and pm-arzz after the CodeFactor PR update was validated.
|
@codex review /gemini review @gemini-code-assist review @coderabbitai full review @coderabbitai review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedFull review finished. |
Summary
Validation
Review status