Skip to content

fix: set maxVersions default value not work#7

Merged
BlackHole1 merged 1 commit intomainfrom
fix-payload.maxVersions
Nov 3, 2025
Merged

fix: set maxVersions default value not work#7
BlackHole1 merged 1 commit intomainfrom
fix-payload.maxVersions

Conversation

@BlackHole1
Copy link
Member

No description provided.

Signed-off-by: Kevin Cui <bh@bugs.cc>
Copilot AI review requested due to automatic review settings November 3, 2025 05:03
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Fixed unintended input mutations in batch row retrieval, range queries, and row read operations. Default version handling now applies correctly without modifying input parameters while maintaining consistent request behavior.

Walkthrough

Three operator files are modified to change the location where the default maxVersions = 1 is assigned when neither maxVersions nor timeRange is provided. The changes shift the assignment from the input object (table, options) to the payload object in each respective builder. This prevents mutation of the input parameters while preserving the identical request output encoding.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • All three files exhibit the same refactoring pattern applied consistently across batch-get-row, get-range, and get-row operators
  • Changes are minimal and isolated to the default assignment location within conditional blocks
  • The homogeneous nature of these changes (identical pattern repeated) reduces per-file cognitive load

Areas requiring attention:

  • Verify that the payload object property names match the intended request field names in all three files
  • Confirm that no downstream code relies on the previous mutation of input objects
  • Ensure the condition evaluation logic (if (!maxVersions && !timeRange) or equivalent) remains functionally equivalent across all three modifications

Pre-merge checks

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description Check ❓ Inconclusive No pull request description was provided by the author. While the leniency guideline suggests the check should pass if the description is not off-topic, an empty description provides no information and therefore cannot be considered related to the changeset in any way. The criteria for passing requires the description to be related to the changeset, which an absent description does not satisfy. The author should provide a description explaining the fix. For example, they could describe the issue being fixed (default maxVersions not working correctly) and how the solution addresses it (by applying the default to the payload instead of the input object to preserve immutability and ensure the default is reflected in the encoded request).
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title follows the required format of <type>: <subject>, with "fix" as the type and no optional scope. The title is written in English as required. The title content is related to the changeset, as it references fixing the maxVersions default value behavior, which directly corresponds to the changes made across all three files that move the default assignment from the input object to the payload object.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 595fef8 and 1c2f43c.

📒 Files selected for processing (3)
  • src/operator/batch-get-row.ts (1 hunks)
  • src/operator/get-range.ts (1 hunks)
  • src/operator/get-row.ts (1 hunks)
🔇 Additional comments (3)
src/operator/get-row.ts (1)

74-76: Excellent fix! Input mutation eliminated.

The change correctly applies the default maxVersions = 1 to the payload object instead of mutating the input options object. This preserves immutability of input parameters while maintaining the same request encoding behavior.

src/operator/batch-get-row.ts (1)

85-87: Excellent fix! Input mutation eliminated.

The change correctly applies the default maxVersions = 1 to the local payload object p instead of mutating the input table object. This preserves immutability of input parameters while maintaining the same request encoding behavior.

src/operator/get-range.ts (1)

80-82: Excellent fix! Input mutation eliminated.

The change correctly applies the default maxVersions = 1 to the payload object instead of mutating the input options object. This preserves immutability of input parameters while maintaining the same request encoding behavior.


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.

❤️ Share

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

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes bugs in three operators where maxVersions was being incorrectly assigned to the input options object instead of the payload object that gets encoded and sent to the server.

  • Changed assignment from options.maxVersions to payload.maxVersions in GetRow and GetRange operators
  • Changed assignment from table.maxVersions to p.maxVersions in BatchGetRow operator

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/operator/get-row.ts Fixed incorrect assignment of maxVersions to options instead of payload
src/operator/get-range.ts Fixed incorrect assignment of maxVersions to options instead of payload
src/operator/batch-get-row.ts Fixed incorrect assignment of maxVersions to table instead of the p payload object

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@BlackHole1 BlackHole1 merged commit cace8ba into main Nov 3, 2025
8 checks passed
@BlackHole1 BlackHole1 deleted the fix-payload.maxVersions branch November 3, 2025 05:09
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.

2 participants