Skip to content

fix: make JsonSerializable type alias read-only - #2249

Merged
vdusek merged 9 commits into
masterfrom
claude/blissful-fermi-0acyp2
Sep 24, 2026
Merged

vdusek merged 9 commits into
masterfrom
claude/blissful-fermi-0acyp2

Conversation

@Pijukatel

@Pijukatel Pijukatel commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Bumping ty to 0.0.83 (#2243) fails type-check: it rejects e.g. list[str] as list[JsonSerializable] because list/dict are invariant.

Fix: JsonSerializable uses read-only Mapping/Sequence; CookieParam becomes a closed TypedDict; one documented cast where redis' JsonType requires list.

The lock bumps only ty to 0.0.83 so CI proves the fix; no new errors in apify-sdk-python either; cookie conversion output unchanged.

ty 0.0.83 rejects e.g. `list[str]` where `list[JsonSerializable]` is expected,
because `list`/`dict` are invariant. Using `Sequence`/`Mapping` makes plain
lists and dicts of JSON values acceptable again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m46SvjMFWEVxC4RDuegPN
@codecov

codecov Bot commented Sep 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.75%. Comparing base (626f334) to head (3da0d68).
⚠️ Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
src/crawlee/sessions/_cookies.py 93.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2249      +/-   ##
==========================================
+ Coverage   93.70%   93.75%   +0.05%     
==========================================
  Files         181      181              
  Lines       12878    12884       +6     
==========================================
+ Hits        12067    12080      +13     
+ Misses        811      804       -7     
Flag Coverage Δ
unit 93.75% <94.11%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

- `CookieParam` is now a closed TypedDict, so it is accepted as JSON-serializable.
  `_from_playwright` builds it from declared keys only (the dropped keys were
  already ignored by `SessionCookies.set`).
- Redis `push_data` encodes items itself instead of going through `arrappend`,
  whose `JsonType` stub rejects read-only `Sequence`/`Mapping` values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m46SvjMFWEVxC4RDuegPN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m46SvjMFWEVxC4RDuegPN
Replace the untyped `execute_command` call with `arrappend` fed by
`_to_redis_json`, which ty checks against redis' `list`-based JSON type.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m46SvjMFWEVxC4RDuegPN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m46SvjMFWEVxC4RDuegPN
redis' `JsonType` types arrays as `list` although `arrappend` only encodes
them, so a documented cast is simpler than copying every item.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m46SvjMFWEVxC4RDuegPN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m46SvjMFWEVxC4RDuegPN
@Pijukatel
Pijukatel marked this pull request as ready for review September 24, 2026 07:00
@Pijukatel
Pijukatel requested a review from vdusek September 24, 2026 07:00
@apify-service-account apify-service-account added the t-tooling Issues with this label are in the ownership of the tooling team. label Sep 24, 2026

@vdusek vdusek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@vdusek
vdusek merged commit dd7358f into master Sep 24, 2026
36 of 39 checks passed
@vdusek
vdusek deleted the claude/blissful-fermi-0acyp2 branch September 24, 2026 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants