🌐 US-Proxy
class="logged-out env-production page-responsive" style="word-wrap: break-word;" >
Skip to content

[fix] Type iterated session state keys as str - #17466

Merged
lukasmasuch merged 3 commits into
developfrom
lukasmasuch/typing-tests-implementation
Oct 13, 2026
Merged

lukasmasuch merged 3 commits into
developfrom
lukasmasuch/typing-tests-implementation

Conversation

@lukasmasuch

@lukasmasuch lukasmasuch commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Describe your changes

Iterating st.session_state now infers str. Stored keys are strings; the previous annotation was Any.

keys(), items(), and popitem() stay str | int so integer widget keys still type-check on lookup.

GitHub Issue Link (if applicable)

N/A

Testing Plan

  • Unit Tests (Python)
    • lib/tests/streamlit/typing/session_state_types.py — mypy and ty cover reads, writes, mapping methods, and invalid keys
    • lib/tests/streamlit/runtime/state/session_state_proxy_test.py — existing proxy behavior
  • No E2E or manual testing. Runtime behavior is unchanged.
Agent metrics
Type Name Count
skill checking-changes 1
skill finalizing-pr 1
skill implementing-typing-tests 1

Stored keys are strings. Typing tests cover the st.session_state
mapping surface.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 16:37
@lukasmasuch lukasmasuch added change:bugfix PR contains bug fix implementation impact:users PR changes affect end users labels Oct 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@snyk-io

snyk-io Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-17466/streamlit-1.65.0-py3-none-any.whl
📦 @streamlit/component-v2-lib Download from artifacts

@lukasmasuch lukasmasuch added the ai-review If applied to PR or issue will run AI review workflow label Oct 13, 2026
@greptile-apps

greptile-apps Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Low impact] The PR appears safe to merge.

Summary

Iterating st.session_state now infers str, matching its stored keys.

  • Session state loops now infer string keys.

Reviews (3) · Last reviewed commit: "Clarify inherited session state view typ..." · Reviewed by Greptile

@github-actions github-actions Bot removed the ai-review If applied to PR or issue will run AI review workflow label Oct 13, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

All three expected reviews completed (gpt-5.6-terra-xhigh, claude-opus-5-5-medium, and grok-4.7-xhigh). None failed. All three approve.

This PR narrows SessionStateProxy.__iter__ from Iterator[Any] to Iterator[str], so for key in st.session_state infers str. It also adds lib/tests/streamlit/typing/session_state_types.py, a mypy/ty test of the session-state mapping surface. Runtime behavior is unchanged: __iter__ still returns iter(get_session_state().filtered_state), and filtered_state is dict[str, Any]. Integer widget keys are still coerced with str() before they are stored.

The reviewers agree on the annotation, the test, compatibility, security, performance, and that no external e2e coverage is needed. They differ only on wording: the PR sentence that explains why keys(), items(), and popitem() stay str | int, plus a docstring and a typing-test comment. Those are optional. Checked against MutableMapping[Key, Any] (Key is str | int) and the lookup methods, the wider view types are inherited and do not control whether integer keys type-check on lookup.

Product Alignment

This changes a public static type and no runtime API, configuration, CLI, UI, or default. It is a small typing bug fix, so it does not need a new product spec. The merged spec specs/2026-07-21-public-typing/product-spec.md asks for deliberate public typing contracts and does not define session-state iteration. The PR body does not claim a separate product approval, and none is required.

The tighter iterator matches principle 12 (precise annotations without a new API) and principle 13 (iteration yields the same string keys to_dict() already exposes). Bracket access, get, pop, setdefault, and update still accept str | int, which is the existing coercion contract. Key stays an internal alias and is not exported. This matches st.query_params, whose iter() is already Iterator[str].

Code Quality

The production change is the __iter__ return annotation and docstring in lib/streamlit/runtime/state/session_state_proxy.py. Iterator is covariant, so Iterator[str] is a valid override of the inherited Iterator[str | int]. The method body is unchanged.

keys(), items(), and popitem() remain str | int because they are inherited from MutableMapping[str | int, Any]. That static type is wider than the strings stored in filtered_state. The imprecision already existed; this PR documents it in the typing test and leaves overrides for a follow-up. gpt-5.6-terra-xhigh treated the PR body's reason for that split as accurate. claude-opus-5-5-medium and grok-4.7-xhigh did not. The latter two are right: integer widget keys type-check because __getitem__, __setitem__, __delitem__, and the mapping helpers accept Key, independent of the view return types.

The new typing test follows lib/streamlit/AGENTS.md: from __future__ import annotations, assertions inside TYPE_CHECKING, import streamlit as st for a module-level object, paired attribute and bracket access, and checker-specific suppressions only where that checker reports. The get(None) lines correctly omit a ty suppression, with a comment that ty types Mapping.get keys as object.

Performance

No runtime impact. The session-state hot path still iterates the existing filtered-state dictionary. The change adds no scan, copy, lock, or allocation. The new file is static-only. Benchmark coverage is not warranted.

Test Coverage

Coverage matches the change. session_state_types.py checks reads, writes, deletion, get / setdefault / pop, views, iteration (both iter() and a for loop), to_dict, clear, update (including integer keys and keyword arguments), and invalid keys. Existing SessionStateProxyTests.test_iter already checks that iteration yields stored keys. E2E tests are unnecessary because observable runtime behavior is unchanged.

claude-opus-5-5-medium could not confirm type-check CI. grok-4.7-xhigh reported that the type-checking unit-test jobs passed, including max. The jobs py-unit-tests for min (3.11), 3.12, 3.13, 3.14, and max (3.15) have since passed. make python-types runs in that workflow for every version except 3.15, so min through 3.14 type-checked this diff. The max job skips type checking by design.

Backwards Compatibility

Runtime behavior is unchanged, including st.session_state[1] storing and reading "1".

Type checkers now infer str for for key in st.session_state where they previously inferred Any. Code that treated those loop variables as a non-string type can start failing static checks. That code did not match the stored-key contract. Lookup signatures and the str | int annotations on keys(), items(), and popitem() are unchanged.

Security & Risk

No security-sensitive surface is touched. The diff does not change session handling, key validation, serialization, authentication, WebSockets, file serving, rendering, or dependencies. The only risk is the intentional static narrowing above.

External test recommendation

  • Recommend external_test: No
  • Triggered categories: None
  • Evidence:
    • lib/streamlit/runtime/state/session_state_proxy.py: return annotation and docstring of __iter__ only. The body is still iter(get_session_state().filtered_state).
    • lib/tests/streamlit/typing/session_state_types.py: assert_type checks that mypy and ty read and that pytest does not run.
  • Suggested external_test focus areas:
    • None. No routing, auth, websocket, embedding, asset, cross-origin, storage, header, or SiS behavior changed.
  • Confidence: High
  • Assumptions and gaps: Assessment uses the two-file diff against develop and the current PR body. An embedded host's type-check of the old Iterator[Any] annotation would be a static-analysis concern, not a hosted runtime divergence.

Accessibility

No frontend or rendered UI changes. Not applicable.

Readability

Title

All three reviewers agree. [fix] Type iterated session state keys as str stands alone, uses the [type] Description form, and states the change. No rewrite.

Description

The opening sentence states the change and why. The reviewers disagree on the next sentence. gpt-5.6-terra-xhigh found the description sufficient. claude-opus-5-5-medium and grok-4.7-xhigh flagged this line:

keys(), items(), and popitem() stay str | int so integer widget keys still type-check on lookup.

That clause gives the wrong cause. Those methods stay str | int because they are inherited from MutableMapping[str | int, Any]. Integer keys type-check because the lookup and mutation methods accept Key.

Proposed description:

## Describe your changes

Iterating `st.session_state` now infers `str`. Stored keys are strings; the previous annotation was `Any`.

`keys()`, `items()`, and `popitem()` stay `str | int` because they are inherited from `MutableMapping[str | int, Any]`. Bracket lookup still accepts integer widget keys.

## Testing Plan

- Typing tests cover reads, writes, mapping methods, and invalid keys.
- No E2E or manual testing. Runtime behavior is unchanged.

Code comments

Optional docstring and views-comment wording is on the diff as inline comments. One further optional clarification, from grok-4.7-xhigh only: the note at lib/tests/streamlit/typing/session_state_types.py lines 89–90 sits between the get assertions and the setdefault assertions and does not name setdefault. get("count", None) is asserted as Any, while the note describes the Any | None overload.

Proposed rewrite:

# setdefault: omitting the default, or passing None, selects the overload
# that returns Any | None. A non-None default is Any.

Recommendations

  1. Optionally correct the PR description so the str | int view types are explained as inherited from MutableMapping[str | int, Any].
  2. Optionally, in a follow-up, override keys(), items(), and popitem() so their key type is str, matching __iter__ and to_dict().

No functional changes are required.

Verdict

APPROVED: The iterator annotation matches stored string keys, runtime behavior is unchanged, and the typing tests cover the public mapping surface. The remaining notes are optional wording and a possible follow-up for the inherited view types.


This is an automated consolidated AI review by grok-4.7-xhigh. Please verify the feedback and use your judgment.

This review also includes 2 inline comment(s) on specific code lines.

# popitem() follow the mapping key type str | int.
# =====================================================================

assert_type(st.session_state.popitem(), tuple[str | int, Any])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: At runtime popitem(), keys(), and items() yield str keys, because they iterate filtered_state: dict[str, Any], same as __iter__. str | int is the inherited MutableMapping[Key, Any] key. Integer widget keys type-check on lookup through __getitem__, get, pop, and setdefault, which accept Key. Overriding the views can wait. If they stay wide, say in the comment above that this is an inherited limitation.

def __iter__(self) -> Iterator[Any]:
"""Iterator over user state and keyed widget values."""
def __iter__(self) -> Iterator[str]:
"""Iterator over user state and keyed widget keys."""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick: "keyed widget keys" repeats "key". Sibling docstrings are noun phrases. Suggest """Keys of user state and keyed widgets.""".

Co-authored-by: Cursor <cursoragent@cursor.com>
@lukasmasuch lukasmasuch added the ai-final-review If applied to PR or issue will run AI review workflow with the latest generation of AI models. label Oct 13, 2026
@github-actions github-actions Bot removed the ai-final-review If applied to PR or issue will run AI review workflow with the latest generation of AI models. label Oct 13, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

SessionStateProxy.__iter__ now returns Iterator[str], so type checkers treat keys from for key in st.session_state and iter(st.session_state) as str. The method body is unchanged: it returns iter(get_session_state().filtered_state), and filtered_state is dict[str, Any]. The docstring now describes keys. A new typing test, lib/tests/streamlit/typing/session_state_types.py, pins the public mapping surface for mypy and ty. Runtime behavior is unchanged.

All four expected reviews finished (gpt-5.6-sol-high, claude-opus-5-5-high, gemini-3.8-flash-high, and grok-4.7-xhigh) and approved. They agree the annotation matches stored string keys, lookup still accepts str | int, and this fix needs no product spec, e2e test, or external test. They differ on the PR sentence that explains why keys(), items(), and popitem() stay str | int, and on optional wording in the typing test. Those notes are non-blocking.

Product Alignment

This is a public static-type correction. Type checkers infer str for iterated session-state keys. There is no new command, parameter, default, or runtime behavior.

This bug fix does not need a new product spec. specs/AGENTS.md exempts bug fixes and small non-controversial enhancements. The merged specs/2026-07-21-public-typing/product-spec.md sets the public typing approach and does not prescribe session-state iteration. The PR body does not claim a separate product approval, and this fix does not need one.

The change matches principle 12 (Type Safety Without Burden): iteration reports the key type the proxy already stores, with no new API. SessionStateProxy remains MutableMapping[Key, Any] (Key is str | int in lib/streamlit/elements/lib/utils.py), so integer widget keys type-check on __getitem__, __setitem__, __delitem__, get, pop, setdefault, and update. keys(), items(), and popitem() keep the inherited str | int key type. QueryParamsProxy.__iter__ already returns Iterator[str]. That proxy is MutableMapping[str, str], so its views are str through the class parameter.

Code Quality

The production edit is the return annotation and docstring on SessionStateProxy.__iter__ (lib/streamlit/runtime/state/session_state_proxy.py, lines 123–124). Iterator is covariant, so Iterator[str] is a valid override of the inherited Iterator[str | int]. __setitem__ stringifies keys, update goes through that path, and to_key() stringifies widget keys. The complexity matches the change.

The typing test follows lib/streamlit/AGENTS.md and lib/tests/AGENTS.md: from __future__ import annotations, assertions inside if TYPE_CHECKING, import streamlit as st for the module-level object, assert_type for both notations, and checker-specific ignores on invalid calls. Its size matches neighboring mapping tests such as query_params_types.py and secrets_types.py.

Performance

There is no runtime or frontend performance impact. __iter__ still returns iter(get_session_state().filtered_state) with no extra scan, copy, or lock. The session-state hot path in lib/streamlit/AGENTS.md is unchanged. The new file is evaluated only by mypy and ty. A benchmark is not warranted.

Test Coverage

Coverage matches an annotation-only change. lib/tests/streamlit/typing/session_state_types.py asserts that iter(st.session_state) and for key in st.session_state are str, and it pins reads, writes, get / setdefault / pop, views, update, and invalid key types. Runtime iteration is already covered by test_iter in lib/tests/streamlit/runtime/state/session_state_proxy_test.py, which expects the stored string key. Runtime and UI behavior are unchanged, so that unit test is the right runtime coverage.

One review said widget_key: str | int = 1 is narrowed to int by both checkers, so later uses miss the union. mypy's rule for this form is that the annotation sets the variable's type: x: int | str = 1 has type int | str. The same file passes str | int keys through update(pairs) and update(mixed). No test change is required.

Backwards Compatibility

Existing apps keep the same runtime behavior. Integer widget keys are accepted on lookup and stored as strings. An integer widget key 1 is iterated as "1".

Type checkers that treated iterated keys as Any now treat them as str. Code that used non-string operations on those keys can start failing type checking. That matches the runtime contract. keys(), items(), and popitem() keep their existing str | int annotations.

Security & Risk

The diff is a type annotation, a docstring, and a static typing test. It does not touch WebSocket or session transport, auth, file serving, cookies, CORS, CSP, dependencies, dynamic code execution, HTML rendering, or secrets. The annotation leaves the set of keys that are read or written unchanged.

External test recommendation

  • Recommend external_test: No
  • Triggered categories: None
  • Evidence:
    • lib/streamlit/runtime/state/session_state_proxy.py: return type and docstring of __iter__ only; the method still iterates filtered_state
    • lib/tests/streamlit/typing/session_state_types.py: static assert_type coverage, never executed at runtime
  • Suggested external_test focus areas:
    • None. No route, auth, websocket, embed, asset, storage, cross-origin, SiS, or security-header behavior changed.
  • Confidence: High
  • Assumptions and gaps: The full two-file diff against develop was reviewed.

Accessibility

There are no frontend or rendered UI changes, so there is no accessibility impact.

Readability

Title. [fix] Type iterated session state keys as str stands alone, uses the [type] Description form, and fits the length limit. No rewrite.

Description. The first sentence states the change. Three reviews flagged the second sentence, which says the views stay str | int so integer widget keys still type-check on lookup. Those views report str | int because they inherit that key type from MutableMapping[Key, Any]. Lookup methods accept integer widget keys because their parameter type is Key.

Proposed replacement:

Type checkers now infer `str` for keys yielded by `st.session_state`. Stored keys are strings; the previous annotation was `Any`.

`keys()`, `items()`, and `popitem()` keep the inherited `str | int` key type. Lookup methods still accept integer widget keys because their parameter type is `Key`.

The testing plan matches the template. The template's Contribution License Agreement footer is absent. Restoring it is optional.

lib/streamlit/runtime/state/session_state_proxy.py. The __iter__ docstring ("Keys of user state and keyed widgets.") matches the noun-phrase style of __len__ and __str__ and describes keys. No rewrite.

lib/tests/streamlit/typing/session_state_types.py

  • Lines 89–90, next to the get assertions at lines 81–87. get("count", None) is asserted as Any, while setdefault with None is asserted as Any | None. The comment sits on setdefault, and "A non-None default is Any" can be read as the default's type. secrets_types.py already documents the get overload.

    Add above get("count", None):

    # An explicit None default selects the overload that returns Any.
    assert_type(st.session_state.get("count", None), Any)

    Proposed setdefault comment:

    # Omitting the default, or passing None, selects the overload that
    # returns Any | None. A non-None default makes the return type Any.

Recommendations

  1. Update the PR description's second sentence so the inherited view types and the Key lookup parameter are described separately. See Readability.
  2. Optionally add the get overload comment from Readability, matching secrets_types.py.
  3. Optional follow-up: override keys(), items(), and popitem() so their key type is str, matching __iter__ and to_dict(). Those return types are covariant in the key, and each override needs a cast around super() because KeysView[str | int] is not assignable to KeysView[str]. Keep __getitem__ and the other mutators on Key so integer widget keys type-check. The current wider view types are safe.

Verdict

APPROVED: The iterator annotation matches stored string keys, runtime behavior is unchanged, and the remaining notes are optional description and comment wording.


This is an automated AI review consolidated by grok-4.7-xhigh. Please verify the feedback and use your judgment.

This review also includes 1 inline comment(s) on specific code lines.

#
# Stored keys are strings, so iteration yields str. keys(), items(), and
# popitem() keep the inherited MutableMapping key type str | int. Lookup
# already accepts integer widget keys; narrowing those views can wait.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: These views stay str | int because they inherit that key type from MutableMapping[Key, Any]. The last sentence can be read as a reason to defer narrowing. Lookup methods accept Key on their own, so narrowing keys(), items(), and popitem() would leave integer-key lookup as it is.

# Stored keys are strings, so iteration yields str. keys(), items(), and
# popitem() still report str | int because they inherit that key type from
# MutableMapping[Key, Any].

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Auto-approved by AI final review (workflow run).

Co-authored-by: Cursor <cursoragent@cursor.com>
@lukasmasuch
lukasmasuch merged commit 50f85fd into develop Oct 13, 2026
38 checks passed
@lukasmasuch
lukasmasuch deleted the lukasmasuch/typing-tests-implementation branch October 13, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:bugfix PR contains bug fix implementation impact:users PR changes affect end users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants