⚡ general improvements

[techouse]

This pull request introduces a new configurable maximum encoding depth for the encoder, improves documentation and project guidelines, and makes some technical refinements to internal utilities. The most significant change is the addition of the max_depth option, which allows users to limit how deeply the encoder will traverse nested data structures, with robust handling to avoid exceeding Python’s recursion limit. Documentation and internal instructions are updated to reflect this, and some internal utilities are simplified for performance and clarity.

Encode depth limit and safety improvements:

• Added a max_depth option to EncodeOptions, allowing users to cap how deeply the encoder traverses nested objects. The effective limit is always capped to Python’s current recursion limit minus a safety margin to avoid RecursionError (src/qs_codec/models/encode_options.py, src/qs_codec/encode.py). [1] [2] [3] [4] [5]
• Updated the encoder to use shallow copies for mappings unless a callable filter is provided, improving performance and aligning with new project guidelines (src/qs_codec/encode.py).
• The encoder now raises a clear ValueError("Maximum encoding depth exceeded") if the depth is exceeded, and validates that max_depth is a positive integer or None (src/qs_codec/models/encode_options.py).

Documentation and project guidance:

• Updated user documentation (README.rst, docs/README.rst, docs/index.rst) to describe the new max_depth option, its default behavior, and usage examples. [1] [2] [3] [4]
• Added a new AGENTS.md file and expanded .github/copilot-instructions.md to clarify project structure, coding conventions, testing strategy, and the new depth limit behavior. [1] [2] [3]

Internal utility and technical changes:

• Simplified WeakWrapper to use stable identity-based hashing rather than deep content hashing, improving performance and avoiding recursion issues (src/qs_codec/models/weak_wrapper.py). [1] [2] [3]
• Removed unused imports and clarified comments for better maintainability (src/qs_codec/utils/utils.py, src/qs_codec/encode.py). [1] [2]

These changes together improve the safety, performance, and clarity of the encoder, while ensuring that both users and contributors have clear guidance on usage and extension.

Summary by CodeRabbit

• New Features

  • Added a configurable max_depth for encoding to cap nesting and prevent recursion errors; encoder validates positive integers.

• Refactor

  • Wrapper hashing now uses identity-based semantics (stable across mutations).
  • Merge logic optimized to apply updates in-place for more efficient merges.

• Documentation

  • Docs updated with guidance, examples, and safety notes for the new max_depth option.

• Tests

  • Added tests for encode depth guards, merge-with-non-dict targets, and wrapper hashing/lookup behavior.

• Chores

  • Added repository guidelines document and minor housekeeping updates.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[chatgpt-codex-connector]

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

[techouse]

@codex review

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a configurable max_depth encode option and runtime depth checks to the encoder, switches WeakWrapper hashing to proxy-identity, refactors Utils.merge to update targets in-place, and updates tests and docs to cover these behaviors.

Changes

Cohort / File(s)	Summary
Encoder depth guard src/qs_codec/encode.py	Add _DEPTH_MARGIN, compute effective max depth (from EncodeOptions.max_depth or sys.getrecursionlimit()), thread _depth/_max_depth through _encode, and raise ValueError when exceeded.
Encode options src/qs_codec/models/encode_options.py	Add public max_depth: Optional[int] = None and validate in __post_init__ (must be positive int or None).
WeakWrapper hashing src/qs_codec/models/weak_wrapper.py	Remove deep-content hashing; __hash__ now uses proxy identity. Docstring updated; equality remains identity-based.
Merge behavior src/qs_codec/utils/utils.py	Stop deepcopy/premerge overflow path; coerce or reuse target dict in-place and apply updates via update(); typing refinements and unused import removal.
Tests — Encode tests/unit/encode_test.py	Add depth-guard tests asserting ValueError when max_depth exceeded and when interpreter recursion limit caps depth.
Tests — EncodeOptions tests/unit/encode_options_test.py	Add validation test ensuring invalid max_depth values raise ValueError.
Tests — Utils tests/unit/utils_test.py	Add test_merge_with_non_dict_mapping_target verifying merges when target is a non-dict mapping (e.g., MappingProxyType).
Tests — WeakWrapper tests/unit/weakref_test.py	Adjust tests for identity-based hashing, value access, mutation stability, and WeakKeyDictionary lookup behavior.
Docs README.rst, docs/README.rst, docs/index.rst	Document new max_depth encode option, examples showing ValueError when exceeded, and update highlights wording to reference encode max depth and decode depth.
Misc .gitignore, AGENTS.md, .github/copilot-instructions.md	Add AGENTS.md, adjust gitignore, and update copilot guidance to reference max_depth and merge behavior notes.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• 🏷️ improve types in tests #39 — Modifies src/qs_codec/encode.py internals and _encode/encode signatures; likely overlaps with depth-tracking changes.
• ✨ decode: add raise_on_limit_exceeded option #11 — Changes in src/qs_codec/utils/utils.py affecting merge/compact logic; related to in-place merge refactor.

Suggested labels

documentation, test

Poem

🐰 I hopped through nests with careful code,
I counted depths so stacks won't go tooad,
I bound the hash to the proxy's name,
I merged in-place — no needless claim,
A tiny hop, and all is stowed.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title ':zap: general improvements' is vague and non-descriptive, using generic language that does not convey the main changes (max_depth encoding limit, WeakWrapper refactoring, documentation updates).	Consider a more specific title such as ':zap: Add configurable max_depth encoding limit and refactor WeakWrapper' or ':zap: Introduce encoding depth guard with safety improvements' to clarify the primary changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The pull request description is comprehensive and well-organized, covering all major changes, the new max_depth feature, documentation updates, and internal improvements. However, it does not follow the provided description template (missing checklist sections, type of change declaration, and testing details).

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore/general-improvements

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

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

[Copilot]

Pull request overview

This PR introduces general improvements focused on performance, safety, and correctness. The changes replace deep hashing with identity-based hashing in WeakWrapper, optimize copying strategies in encoding and merging, and add recursion depth guards to prevent stack overflows.

Changes:

• Switched WeakWrapper from deep content hashing to identity-based hashing for stability with mutable objects
• Optimized encoding by replacing deep copies with shallow copies (except when callable filters are used)
• Added configurable recursion depth guards in encoding to prevent stack overflows
• Updated Utils.merge to avoid unnecessary deep copies and handle non-dict mappings

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/qs_codec/models/weak_wrapper.py	Replaced deep content hashing with identity-based hashing using proxy identity
src/qs_codec/encode.py	Added recursion depth guard and switched from deep to shallow copying for mappings
src/qs_codec/utils/utils.py	Optimized merge to avoid deep copies and handle non-dict mapping targets
tests/unit/weakref_test.py	Updated tests to verify identity-based hashing and removed deep hash tests
tests/unit/utils_test.py	Added test for merging with MappingProxyType targets
tests/unit/encode_test.py	Added test for encoding depth guard functionality

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ +0.00% (target: -1.00%)	✅ 100.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (d86d201)	1267	1267	100.00%
Head commit (1f6936f)	1247 (-20)	1247 (-20)	100.00% (+0.00%)

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 (#40)	27	27	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%

See your quality gate settings    Change summary preferences

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (d86d201) to head (1f6936f).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main       #40   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         1267      1247   -20     
=========================================
- Hits          1267      1247   -20     

☔ 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.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/qs_codec/utils/utils.py (1)

220-240: ⚠️ Potential issue | 🟠 Major

In-place mutation of target when it's a dict may violate caller-input immutability.

When target is already a dict, line 223 assigns merge_target = target (same reference), and line 239 mutates it in place via update(). This means callers passing a dict directly to Utils.merge will have their original mutated.

While the internal encoder protects inputs with a shallow copy before recursion, Utils.merge is a public utility method. Direct callers could experience unexpected mutation.

Consider always creating a shallow copy for dict targets:

🛡️ Proposed fix to avoid mutating caller input

         # Prepare a mutable target we can merge into (avoid deepcopy for performance).
         merge_target: t.Dict[str, t.Any]
-        if isinstance(target, dict):
-            merge_target = target
-        else:
-            merge_target = dict(target)
+        merge_target = dict(target)  # shallow copy preserves caller immutability

Based on learnings: "DO NOT mutate caller inputs—copy/normalize (deepcopy for mappings, index-projection for sequences) before traversal".

🧹 Nitpick comments (1)

src/qs_codec/encode.py (1)

233-234: Consider extracting the error message to a constant (optional).

The static analysis tool flags TRY003 for the inline message. While this is a minor style issue, extracting to a module-level constant could improve consistency:

💅 Optional: Extract error message constant

+_ERR_MAX_DEPTH = "Maximum encoding depth exceeded"
+
 # ...
 
     if _depth > _get_max_encode_depth():
-        raise ValueError("Maximum encoding depth exceeded")
+        raise ValueError(_ERR_MAX_DEPTH)

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.