Skip to content

fix: address review issues from plugin standardization PR#259

Merged
datlechin merged 2 commits intomainfrom
fix/plugin-standardization-review-issues
Mar 10, 2026
Merged

fix: address review issues from plugin standardization PR#259
datlechin merged 2 commits intomainfrom
fix/plugin-standardization-review-issues

Conversation

@datlechin
Copy link
Copy Markdown
Collaborator

@datlechin datlechin commented Mar 10, 2026

Summary

Fixes 5 issues found during review of #258:

  • Overload ambiguity: Renamed cancellation-aware variant to pluginDispatchAsyncCancellable() to prevent Swift from silently resolving all pluginDispatchAsync callers to the overload with Task.checkCancellation()
  • RedisParseError prefix lost: Restored "Invalid argument: " / "Missing argument: " prefix in pluginErrorMessage for .invalidArgument and .missingArgument cases
  • Error code [0] noise: Skip [code] prefix in default errorDescription when code is 0 (used by all static error cases like .notConnected, .connectionFailed)
  • PostgreSQL detail label: Restored "Detail: " prefix before pluginErrorDetail in default errorDescription
  • MongoDB truncation false positive: Changed find()/aggregate() to return (docs:isTruncated:) tuple from iterateCursor, detecting actual cursor truncation instead of using docs.count >= limit which false-positives when exactly 100K docs exist

Test plan

  • xcodebuild build succeeds
  • xcodebuild test passes
  • Verify Redis parse errors show "Invalid argument:" prefix
  • Verify PostgreSQL errors with detail show "Detail:" label
  • Verify MongoDB find with exactly N=limit docs doesn't show truncation warning

Summary by CodeRabbit

Release Notes

  • New Features

    • Query operations now indicate when results are truncated
    • Async operations improved with cancellation support
  • Bug Fixes

    • Error messages now display with improved formatting and localization
    • Better error detail visibility in UI
  • Refactor

    • Updated query result handling structure
    • Repositioned SQL review interface element in toolbar

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin datlechin merged commit 74c3bfa into main Mar 10, 2026
1 of 2 checks passed
@datlechin datlechin deleted the fix/plugin-standardization-review-issues branch March 10, 2026 17:41
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 10, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 34e833be-e4c9-44a0-800f-2c66a1dd2ed9

📥 Commits

Reviewing files that changed from the base of the PR and between 0085ec7 and 6ca6395.

📒 Files selected for processing (6)
  • Plugins/MongoDBDriverPlugin/MongoDBConnection.swift
  • Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift
  • Plugins/RedisDriverPlugin/RedisCommandParser.swift
  • Plugins/TableProPluginKit/PluginConcurrencySupport.swift
  • Plugins/TableProPluginKit/PluginDriverError.swift
  • TablePro/Views/Toolbar/TableProToolbarView.swift

📝 Walkthrough

Walkthrough

The pull request updates the MongoDB plugin to return tuples containing documents and a truncation flag, adds cancellation support to the plugin dispatch mechanism, localizes Redis error messages, adjusts error description formatting, and repositions a UI popover element.

Changes

Cohort / File(s) Summary
MongoDB Truncation Tracking
Plugins/MongoDBDriverPlugin/MongoDBConnection.swift, Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift
Updated find() and aggregate() methods to return tuples (docs: [[String: Any]], isTruncated: Bool) instead of raw document arrays. Internal cursor iteration now tracks truncation state and propagates it through result-building logic in the driver.
Cancellation-Aware Dispatch
Plugins/TableProPluginKit/PluginConcurrencySupport.swift
Renamed pluginDispatchAsync() to pluginDispatchAsyncCancellable() and added optional cancellationCheck parameter. Introduces task cancellation checks and handler wrapping to support cooperative cancellation.
Error Handling & Localization
Plugins/TableProPluginKit/PluginDriverError.swift, Plugins/RedisDriverPlugin/RedisCommandParser.swift
Minor formatting adjustments: error description now prefixes details with "Detail: " and only includes error codes when non-zero; Redis argument parser now returns localized error messages with consistent prefixes.
UI Refinement
TablePro/Views/Toolbar/TableProToolbarView.swift
Repositioned SQL review popover attachment from toolbar item group to specific Preview SQL button element.

Sequence Diagram(s)

sequenceDiagram
    participant Driver as MongoDBPluginDriver
    participant Connection as MongoDBConnection
    participant Cursor as Cursor/iterateCursor
    participant Limit as PluginRowLimits
    
    Driver->>Connection: find(...) or aggregate(...)
    activate Connection
    Connection->>Cursor: iterateCursor(cursor)
    activate Cursor
    Cursor->>Limit: check defaultMax threshold
    alt Results exceed limit
        Cursor->>Cursor: isTruncated = true
    else Results within limit
        Cursor->>Cursor: isTruncated = false
    end
    Cursor-->>Connection: (docs: [[...]], isTruncated: Bool)
    deactivate Cursor
    Connection-->>Driver: (docs: [[...]], isTruncated: Bool)
    deactivate Connection
    
    Driver->>Driver: extract result.docs & result.isTruncated
    Driver->>Driver: buildPluginResult with isTruncated flag
    Driver-->>Client: PluginResult with truncation metadata
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • feat: implement plugin system #256: Introduces PluginRowLimits infrastructure and plugin row limit handling; this PR builds upon that foundation by using the limit threshold to set the isTruncated flag in MongoDB query results.

Poem

🐰 A MongoDB tuple hop,
With truncation flags that never stop,
Cancellations checked with care,
Errors dressed in details fair,
The plugin dance spins on!

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/plugin-standardization-review-issues

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant