Skip to content

chore: align with upstream changes from Node qs v6.15.2#10

Merged
techouse merged 8 commits into
mainfrom
fix/upstream-changes
May 20, 2026
Merged

chore: align with upstream changes from Node qs v6.15.2#10
techouse merged 8 commits into
mainfrom
fix/upstream-changes

Conversation

@techouse
Copy link
Copy Markdown
Owner

@techouse techouse commented May 19, 2026

This pull request updates the Node qs semantic baseline from version 6.15.1 to 6.15.2 and refines the handling of key parsing and comma array encoding for improved standards compliance and bug fixes. The most important changes are as follows:

Semantic Baseline Update:

  • Updated documentation and code comments to reference Node qs version 6.15.2 as the new semantic baseline, replacing 6.15.1. This affects README.md and docs/divergences.md. [1] [2] [3] [4]

Key Parsing and Divergence Handling:

  • Changed the normalization of top-level dotted keys to occur before depth = 0 preservation when allow_dots = true, and updated the divergence matrix and related documentation to reflect this behavior.
  • Refactored the key parsing logic in split_key_into_segments to simplify and improve handling of unterminated bracket groups, removing the find_recoverable_balanced_open recovery logic and updating tests accordingly. [1] [2] [3] [4] [5] [6]

Comma Array Encoding Fixes:

  • Reworked comma array encoding to better handle empty and null-like values, ensuring correct application of skip_nulls, strict_null_handling, and related options, and fixing edge cases for empty arrays and nulls. [1] [2] [3] [4] [5] [6] [7]

Test and Code Cleanup:

  • Removed unused or redundant code and test helpers, including find_recoverable_balanced_open and related test cases, to streamline the codebase. [1] [2] [3] [4]

Other Bug Fixes:

  • Fixed the use of the delimiter when appending the charset sentinel in the encoder, ensuring correct output formatting.
  • Simplified RFC1738 handling in encode_key_only_fragment to avoid unnecessary replacements.

Summary by CodeRabbit

  • Bug Fixes

    • Charset sentinel now respects the configured delimiter instead of always using &.
    • Top-level dotted keys normalize before depth=0 preservation when allow_dots is enabled.
    • RFC1738 space encoding in key-only fragments now uses + instead of %20.
    • Refined null-handling behavior for comma-separated arrays.
  • Documentation

    • Semantic baseline updated to Node qs 6.15.2.
    • Divergence docs clarified for bracket and dotted-key parsing.
    • README and comparison instructions now use pnpm install --frozen-lockfile.

Review Change Stack

@techouse techouse requested a review from Copilot May 19, 2026 13:46
@techouse techouse self-assigned this May 19, 2026
@techouse techouse added the enhancement New feature or request label May 19, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

📝 Walkthrough

Walkthrough

Updates baseline to Node qs 6.15.2 and pnpm bootstrap docs; refactors decode key segmentation to normalize dots before depth=0 and removes bracket-recovery helper; refines comma-array null handling and charset-sentinel delimiter; expands parity tests for these behaviors.

Changes

Baseline upgrade and behavior alignment

Layer / File(s) Summary
Version baseline and documentation
README.md, docs/divergences.md, docs/release_checklist.md, tests/comparison/js/README.md, tests/comparison/js/package.json, tests/porting_ledger.md, tests/support/mod.rs
Update Node qs baseline to 6.15.2, switch JS bootstrap instructions to pnpm install --frozen-lockfile, and reflect dotted-key normalization-before-depth=0 in docs and ledger.
Decode key parsing simplification
src/decode.rs, src/decode/keys.rs, src/decode/tests/keys.rs, src/decode/tests/mod.rs, src/decode/tests/scanner.rs, tests/divergences.rs
Apply top-level dot-to-bracket conversion prior to depth==0 early-return; remove unterminated-bracket recovery and find_recoverable_balanced_open; simplify parse_object root cleaning; update tests and re-exports accordingly.
Encode: comma-array and null handling
src/encode/comma.rs, src/encode/tests/helpers.rs
Restrict null compaction to comma_compact_nulls, centralize empty-result handling to honor skip_nulls/strict_null_handling/key=, make null-like items yield empty strings early, and update tests covering these cases.
Encode: charset-sentinel and RFC1738 key-only
src/encode.rs, src/encode/scalar.rs, tests/support/cases/*
Use options.delimiter after charset sentinel instead of &; stop post-processing +%20 in key-only RFC1738 helper; update case metadata and stringify tests asserting these behaviors.
Parity test expansion
tests/support/cases/node_parse.rs, tests/support/cases/node_stringify.rs, tests/support/cases/csharp_encode.rs, tests/support/cases/dart_encode.rs, tests/divergences.rs
Add decode cases for dot normalization at depth=0, bracket parsing and unterminated-group handling; add stringify cases for comma-array null semantics, strict-null RFC1738 formatting, and charset-sentinel delimiter; update test descriptions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • techouse/qs_rust#4: Updates key parsing imports and adds bracket/dot/list-limit edge-case coverage; overlaps with this PR's decode helper removal and tests.

Suggested labels

test

Poem

"I am a rabbit in the query-tree glen,
Dots become brackets again and again,
Nulls in commas now know their place,
Sentinels follow the delimiter's trace,
Hooray for 6.15.2 — hop, hop, amen!" 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The PR description is comprehensive, detailing all major changes (semantic baseline update, key parsing refactor, comma array encoding fixes, code cleanup, and other bug fixes) with relevant context and references.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title 'chore: align with upstream changes from Node qs v6.15.2' accurately summarizes the main objective of this PR: updating the semantic baseline to Node qs v6.15.2 and implementing related behavioral changes throughout the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/upstream-changes

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.

@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 19, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics -24 complexity · 0 duplication

Metric Results
Complexity -24
Duplication 0

View in Codacy

🟢 Coverage 85.29% diff coverage · -0.04% coverage variation

Metric Results
Coverage variation -0.04% coverage variation (-1.00%)
Diff coverage 85.29% diff coverage

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (d09b12f) 5048 4940 97.86%
Head commit (9342b4d) 4997 (-51) 4888 (-52) 97.82% (-0.04%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#10) 34 29 85.29%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

❌ Patch coverage is 85.29412% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.81%. Comparing base (d09b12f) to head (9342b4d).

Files with missing lines Patch % Lines
src/encode/comma.rs 80.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #10      +/-   ##
==========================================
- Coverage   97.86%   97.81%   -0.05%     
==========================================
  Files          37       37              
  Lines        5048     4997      -51     
==========================================
- Hits         4940     4888      -52     
- Misses        108      109       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@tests/comparison/js/README.md`:
- Around line 1-4: The README currently instructs "npm ci" but the fixture uses
pnpm (pnpm-lock.yaml and package.json), so update the bootstrap instruction by
replacing the "npm ci" command with the pnpm equivalent (e.g., "pnpm install
--frozen-lockfile" or "pnpm i --frozen-lockfile") so the locked pnpm
dependencies are installed deterministically; update the line containing the
"npm ci" command in the tests/comparison/js README (the sentence referencing
bootstrapping the Node comparison tests) to use the chosen pnpm command.
🪄 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: 6ae8de3c-764d-4b3b-b70a-dd92d76147db

📥 Commits

Reviewing files that changed from the base of the PR and between d09b12f and 17b4728.

⛔ Files ignored due to path filters (1)
  • tests/comparison/js/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (19)
  • README.md
  • docs/divergences.md
  • src/decode.rs
  • src/decode/keys.rs
  • src/decode/tests/keys.rs
  • src/decode/tests/mod.rs
  • src/decode/tests/scanner.rs
  • src/encode.rs
  • src/encode/comma.rs
  • src/encode/scalar.rs
  • src/encode/tests/helpers.rs
  • tests/comparison/js/README.md
  • tests/comparison/js/package.json
  • tests/divergences.rs
  • tests/porting_ledger.md
  • tests/support/cases/csharp_encode.rs
  • tests/support/cases/dart_encode.rs
  • tests/support/cases/node_parse.rs
  • tests/support/cases/node_stringify.rs

Comment thread tests/comparison/js/README.md Outdated
Copy link
Copy Markdown

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 updates the repository’s Node qs semantic baseline to 6.15.2 and aligns Rust encode/decode edge-case behavior (notably structured key parsing and comma-array null handling) to match upstream semantics more closely.

Changes:

  • Bumped Node comparison fixture from qs@6.15.1 to qs@6.15.2 and updated baseline references in docs/tests.
  • Refined decode key segmentation to better handle dotted keys at depth=0 and treat unterminated/nested bracket groups as literal segments (matching upstream behavior).
  • Reworked comma list encoding around null-like values to match Node’s “empty slot” joining behavior, and fixed charset-sentinel delimiter handling.

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/support/cases/node_stringify.rs Adds new Node parity encode cases for strict-null RFC1738 key formatting, comma-array null slot behavior, and delimiter-aware charset sentinels.
tests/support/cases/node_parse.rs Adds Node parity decode cases for dot normalization with depth=0 and bracket edge cases (literal empty brackets, unterminated groups, trailing text).
tests/support/cases/dart_encode.rs Updates test case description to reflect RFC1738 key-only formatting behavior.
tests/support/cases/csharp_encode.rs Updates test case description to reflect delimiter-aware charset sentinel behavior.
tests/porting_ledger.md Updates ledger entries to reflect new Node-consistent behavior for dotted keys at depth=0 and related C# divergence notes.
tests/divergences.rs Updates divergence test name/expectations for dot normalization before depth=0 preservation.
tests/comparison/js/README.md Updates stated pinned qs version (but still incorrectly instructs npm ci).
tests/comparison/js/pnpm-lock.yaml Updates Node fixture lockfile to qs@6.15.2.
tests/comparison/js/package.json Updates dependency constraint to ^6.15.2.
src/encode/tests/helpers.rs Updates/extends unit tests for comma-null joining and RFC1738 key-only formatting.
src/encode/scalar.rs Simplifies encode_key_only_fragment to use the same formatter logic (fixing RFC1738 space handling in key-only output).
src/encode/comma.rs Adjusts comma-array encoding to preserve null “slots” (empty strings) vs compacting, and updates strict/skip-null edge handling.
src/encode.rs Ensures charset sentinel separator uses the configured delimiter instead of hardcoding &.
src/decode/tests/scanner.rs Updates tests for new split-key behavior (balanced bracket scanning, dotted keys at depth=0).
src/decode/tests/mod.rs Removes re-exports of deleted key-recovery helper.
src/decode/tests/keys.rs Updates structured-key helper tests to new literal-bracket/trailing-text behavior.
src/decode/keys.rs Reworks split_key_into_segments to scan balanced brackets, normalize dots before depth=0, and handle unterminated bracket groups as a single literal segment.
src/decode.rs Removes references to deleted key-recovery helper.
README.md Updates Node fixture bootstrap instructions (pnpm) and baseline version reference to 6.15.2.
docs/divergences.md Updates divergence matrix baseline/version and the dotted-key depth=0 entry to match new behavior.
Files not reviewed (1)
  • tests/comparison/js/pnpm-lock.yaml: Language not supported

Comment thread tests/comparison/js/README.md Outdated
Comment thread src/encode/comma.rs Outdated
@techouse techouse requested a review from Copilot May 19, 2026 16:12
@techouse
Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

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

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • tests/comparison/js/pnpm-lock.yaml: Language not supported

@techouse techouse changed the title chore: align with upstream changes chore: align with upstream changes from qs.js v6.15.2 May 19, 2026
@techouse techouse changed the title chore: align with upstream changes from qs.js v6.15.2 chore: align with upstream changes from qs v6.15.2 May 19, 2026
@techouse techouse changed the title chore: align with upstream changes from qs v6.15.2 chore: align with upstream changes from Node qs v6.15.2 May 19, 2026
@techouse techouse added bug Something isn't working and removed enhancement New feature or request labels May 19, 2026
@techouse techouse merged commit 9ccd566 into main May 20, 2026
31 of 35 checks passed
@techouse techouse deleted the fix/upstream-changes branch May 20, 2026 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants