Skip to content

fix: align Rust behavior with Node qs 6.16.0 - #44

Merged
techouse merged 4 commits into
mainfrom
chore/qs-js-6.16.0-compat
Sep 30, 2026
Merged

techouse merged 4 commits into
mainfrom
chore/qs-js-6.16.0-compat

Conversation

@techouse

@techouse techouse commented Sep 30, 2026 •

Copy link
Copy Markdown
Owner

Description

Fix two parity gaps against the Node qs 6.16.0 baseline:

  • Enforce strict list_limit checks for each comma group assigned through []= before splitting or invoking custom value decoders. Preserve the nested outer-list shape at the limit and existing non-throwing behavior.
  • Escape dotted root keys with primitive leaves when encode_dot_in_keys is enabled, preserving literal-key round trips and function-filter prefixes.
  • Update the parity baseline documentation and add Node-backed and Rust regression coverage.

Type of change

  • Bug fix
  • Documentation update

How Has This Been Tested?

  • cargo test --lib bracketed_comma — 4 passed.
  • cargo test --test regressions dotted_root — 2 passed.
  • cargo test --test parity_decode --test parity_encode --test comparison -- --nocapture — all 3 suites passed against installed qs 6.16.0.
  • cargo test --all-features — 301 passed across 16 suites.
  • cargo clippy --all-targets --all-features -- -D warnings — clean.
  • Direct public-API smoke verified strict rejection, at-limit nested shape, and literal dotted-key round trip.

Compatibility notes

Non-throwing comma decoding, decode_pairs, existing dot-option validation, and nested separator-dot behavior are unchanged.

Summary by CodeRabbit

  • Bug Fixes
    • Strict list limits now apply to comma-separated groups assigned through []=, with oversized groups rejected before splitting or custom value decoding when strict mode is enabled.
    • Enabling dot encoding now escapes dots in top-level keys with primitive values as well as complex values.
  • Compatibility
    • Updated the documented and tested Node qs behavior baseline to version 6.16.0.
  • Documentation
    • Clarified comma-group list-limit behavior and added examples for encoding dots in top-level keys.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 23f8faae-eae7-4f7d-86b4-f19119f56224

📥 Commits

Reviewing files that changed from the base of the PR and between 9594e05 and 6ead36a.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • tests/comparison/js/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (19)
  • .github/copilot-instructions.md
  • CHANGELOG.md
  • Cargo.toml
  • README.md
  • docs/divergences.md
  • docs/qs-6.16.0-upstream-review.md
  • src/decode/accumulate/build.rs
  • src/decode/accumulate/build/tests.rs
  • src/decode/tests/duplicates.rs
  • src/decode/tests/flat.rs
  • src/encode.rs
  • src/options/decode.rs
  • tests/comparison/js/README.md
  • tests/comparison/js/package.json
  • tests/porting_ledger.md
  • tests/regressions.rs
  • tests/support/cases/kotlin_encoder_internal.rs
  • tests/support/cases/node_parse.rs
  • tests/support/cases/node_stringify.rs
💤 Files with no reviewable changes (2)
  • tests/support/cases/kotlin_encoder_internal.rs
  • src/decode/accumulate/build/tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The update aligns decoding and encoding behavior with Node qs 6.16.0 for strict comma-group limits and dot encoding on top-level primitive-valued keys. It adds regression and parity cases, and updates the documented parity baseline and comparison harness.

Changes

Node qs 6.16.0 parity

Layer / File(s) Summary
Strict comma-group limits
src/decode/accumulate/build.rs, src/decode/accumulate/build/tests.rs, src/decode/tests/*, src/options/decode.rs, tests/support/cases/node_parse.rs, README.md, tests/porting_ledger.md, docs/qs-6.16.0-upstream-review.md
Strict mode checks each comma group assigned through []= against list_limit before splitting or custom value decoding. Tests cover over-limit, at-limit, and non-throwing cases, including cumulative outer limits.
Root-key dot encoding
src/encode.rs, tests/regressions.rs, tests/support/cases/node_stringify.rs, tests/support/cases/kotlin_encoder_internal.rs, README.md, tests/porting_ledger.md, docs/qs-6.16.0-upstream-review.md
Root object keys use the common key-component encoding path. Regression and parity cases cover primitive values, filters, encoding options, and round-trip decoding.
Parity baseline and supporting updates
.github/copilot-instructions.md, CHANGELOG.md, Cargo.toml, README.md, docs/divergences.md, docs/qs-6.16.0-upstream-review.md, tests/comparison/js/*, tests/porting_ledger.md
The documented Node qs baseline and comparison dependency move to 6.16.0. The changelog and upstream review document record the parity updates and other reviewed upstream changes.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 6ead3

The decoding and encoding changes align with the documented qs 6.16.0 behavior, and the comparison harness pins the intended version. The change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6ead3

The changes strengthen configured parser limits and preserve literal dotted keys. No introduced security bypass was identified in the inspected paths. Remaining risk is limited compatibility with downstream consumers and custom callbacks whose integration behavior has not been fully assessed.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected effects are bounded to caller-provided query data, constructed values, serialized key identity, and caller-supplied callbacks. Deployment-wide, tenant, or service exposure cannot be determined without downstream integration context.

Trust Boundaries and Controls

  • observed — Raw comma-group size is checked before custom value decoding, while escaped root paths continue through the existing filter and key-encoder interfaces. The inspected changes strengthen limit enforcement and correct key representation rather than introducing a new authority transition.

Resilience and Maintainability Implications

  • observed — A pre-existing aggregate-combine path replaces the prior local accumulator value before a fallible combination. This behavior is unchanged by the PR. External observability after a top-level error was not established, so it is not treated as an introduced security concern.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 8 files. (9 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: aligning Rust behavior with Node qs 6.16.0.
Description check ✅ Passed The description provides a clear summary, change type, motivation, compatibility notes, and detailed test results. It does not include an issue reference or the checklist sections, but the required ch…
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 47.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 8 files. (9 skipped: 9 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@codacy-production

codacy-production Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics -4 complexity · 0 duplication

Metric Results
Complexity -4
Duplication 0

View in Codacy

🟢 Coverage 100.00% diff coverage · +0.01% coverage variation

Metric Results
Coverage variation ✅ +0.01% coverage variation (-1.00%)
Diff coverage ✅ 100.00% diff coverage

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (9594e05) 5060 4938 97.59%
Head commit (6ead36a) 5046 (-14) 4925 (-13) 97.60% (+0.01%)

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 (#44) 4 4 100.00%

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

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.60%. Comparing base (9594e05) to head (6ead36a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
+ Coverage   97.58%   97.60%   +0.01%     
==========================================
  Files          37       37              
  Lines        5060     5046      -14     
==========================================
- Hits         4938     4925      -13     
+ Misses        122      121       -1     

☔ View full report in Codecov by Harness.
📢 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.

@techouse techouse added enhancement New feature or request dependencies Pull requests that update a dependency file labels Sep 30, 2026
@techouse techouse self-assigned this Sep 30, 2026
@techouse
techouse merged commit 3188677 into main Sep 30, 2026
23 checks passed
@techouse
techouse deleted the chore/qs-js-6.16.0-compat branch September 30, 2026 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant