Repository navigation
[chore] Replace tomli-w with in-house writer - #17342
Conversation
Credentials and config show only need a small TOML subset.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ PR preview is ready!
|
|
There was a problem hiding this comment.
Summary
All three expected reviews finished: gpt-5.6-terra-xhigh, claude-opus-5-5-medium, and grok-4.7-xhigh. None failed to complete review. All three approved.
This PR removes the tomli-w runtime dependency and replaces it with lib/streamlit/toml_writer.py, a private serializer for the two TOML documents Streamlit writes: ~/.streamlit/credentials.toml and the assignments printed by streamlit config show. config_util._dump_toml_value and Credentials.save call toml_writer.dumps. The dependency is also removed from lib/pyproject.toml, uv.lock, scripts/assets/min-constraints-gen.txt, and the lazy-loaded-modules e2e list.
The reviewers agreed that the writer is limited to the types those paths use, that the credentials file text stays the same, and that two config show changes are fixes: font faces stay inline under [theme], and Unicode line separators are escaped. They differed on whether rejecting dates and nested arrays preserves old error behavior. It does not. That difference is intentional and is not merge-blocking (see Backwards Compatibility).
Product Alignment
This has limited user-facing impact. streamlit config show is an existing CLI surface, and Streamlit stops installing tomli-w. The PR adds no public API, configuration option, default, or command. All three reviewers agreed a merged product spec is not required. Nothing in specs/ covers credentials files or config show, and the PR body does not claim a separate product approval. That fits a dependency-maintenance change plus a correction to CLI text.
The reviewers agreed on the user-visible results:
- Streamlit no longer installs
tomli-w. - A
theme.fontFacesvalue stays one inline-table assignment under the[theme]header thatconfig showalready prints.tomli-wswitched a long face to[[fontFaces]]once the inline form passed 100 characters, which closes[theme]when that text is pasted under the header. - U+0085, U+2028, and U+2029 are escaped, so a commented-out value stays on one physical line. Parsed strings are unchanged.
The implementation matches the font-face decision in the PR body, and it matches the decision to raise TypeError for values outside the supported subset. The common credentials and config-show cases stay the same, with no new parameters or defaults.
Code Quality
All three reviewers found the writer proportionate: one module, no extra types, and no attempt to be a general TOML library. That matches the dependency guidance in lib/AGENTS.md. Private helpers are underscored, annotations are present, and dumps is the only public function. Both call sites pass a plain dict.
Details all three reviews treated as correct, and that match tomli-w 1.2.0 for the shapes Streamlit writes:
boolis handled beforeint.- Basic strings escape quotes, backslashes, C0 controls, and DEL. Tab stays raw.
- Scalar assignments come before child tables. Non-bare keys, including the empty key, are quoted. A dot inside a key is quoted, so it does not become another table path.
- Dicts inside arrays are inline tables. A multiline value inside an inline table raises
TypeError.
Claude noted a pre-existing quirk the other reviews did not mention. A dict-valued option such as server.trustedUserHeaders is still emitted as a [trustedUserHeaders] header, which closes the [server] section config show already printed. tomli-w did the same for mappings. This PR leaves that behavior as it was. Fixing it is a separate follow-up.
Performance
All three reviewers agreed there is no meaningful hot-path impact. Credential save and streamlit config show run outside script reruns, element creation, message transport, and frontend rendering. There is no frontend change.
Grok noted that toml_writer is imported with config_util and credentials, while tomli_w was imported only when those commands ran. Claude and GPT treated that cost as negligible. That assessment holds: the new module imports re and typing and compiles one key regex. Normal script runs no longer load tomli-w. Performance-specific coverage is not needed.
Test Coverage
All three reviewers agreed the changed behavior is covered well enough to merge.
lib/tests/streamlit/toml_writer_test.pycovers scalar spelling, escapes, Unicode line separators, the credentials document, empty containers, array layout, short and long font faces (including atomllibparse under a[theme]header), quoted keys, tuples, and rejected types.test_dump_toml_value_round_trips_config_optionsparses every current config option's default and value back throughtomlliband checks type and equality, including that booleans stay booleans.- Existing
Credentials.savestill asserts the exact credentials file text. This PR does not modify that test file. e2e_playwright/lazy_loaded_modules_test.pyexpects 14 markdown rows aftertomli_wleaves the lazy-import list.
New tests follow lib/tests/AGENTS.md. The e2e edit updates the existing count.
Claude wanted one more integration assertion through show_config for a set theme.fontFaces value. GPT and Grok treated the writer test as enough. The writer test already parses a long font face under a [theme] header, which is the regression this PR fixes. That difference is optional coverage, recorded as an inline comment.
Backwards Compatibility
Supported documents stay compatible. All three reviewers agreed on these points:
- A normal credentials file is still
[general]withemail = "...". Emails with quotes or backslashes still parse back to the same string.test_Credentials_savelocks the exact text. - Config-show lists, booleans, numbers, and ordinary strings keep the spelling the current tests lock in.
- A long font face changes from
[[fontFaces]]to one inline table. Both forms describe the same data. The array-of-tables form closed the[theme]section in this output. - Escaping U+0085, U+2028, and U+2029 changes the bytes of a string that contains those characters.
tomllibreads the same string back. - A project that imported
tomli_wwithout declaring it, and relied on Streamlit to install it, needs its own dependency.tomli-wwas not a Streamlit API.
The reviews disagreed on dates and nested arrays. Claude described unsupported values as still raising TypeError. GPT and Grok described a behavior change. tomli-w 1.2.0 confirms GPT and Grok: format_literal writes date and datetime with str(), and an array of tables whose inline form contains a newline or exceeds 100 characters becomes [[fontFaces]]. The new writer raises TypeError for a date and for a multiline value inside an inline table. No current config option uses date, datetime, time, or Decimal, and theme font faces are flat tables. config show only dumps config values, so this is not a practical break. The inaccurate part is the PR body's "still raises" wording (see Readability).
Security & Risk
All three reviewers agreed the risk is low. The writer serializes local config and credential values. It does no I/O and no evaluation. Basic strings escape ", \, and the controls a TOML basic string cannot contain raw, plus the Unicode line separators that would split a config-show comment. A quote, newline, or ] in an email or header name stays inside a quoted string or quoted key. File path handling and who may call Credentials.save are unchanged. No WebSocket, auth, cookie, upload, HTML, or eval changes. Removing a runtime dependency shrinks the supply-chain surface.
External test recommendation
- Recommend external_test: No
- Triggered categories: None
- Evidence:
lib/streamlit/toml_writer.py: in-process TOML text only.lib/streamlit/config_util.pyandlib/streamlit/runtime/credentials.py: serializer swap for CLI text and the local credentials file. No routes, cookies, WebSockets, embedding, storage, cross-origin behavior, or browser policies.lib/pyproject.toml,uv.lock,scripts/assets/min-constraints-gen.txt: dependency removal.e2e_playwright/lazy_loaded_modules.py: startup import list. No embed, CORS, or asset-serving change.
- Suggested external_test focus areas:
- None. Local unit tests cover document spelling and round-trips.
- Confidence: High
- Assumptions and gaps: Credentials stay a file under the user's home directory, and config show stays terminal text. No externally hosted or embedded behavior depends on
tomli-w. Tests were not executed in this review.
All three reviewers reached this recommendation.
Accessibility
Not applicable. There are no frontend or UI changes. All three reviewers agreed.
Readability
Code comments explain the non-obvious constraints: bool before int, a raw tab, deferred dicts, inline font faces, and Unicode line separators. Test names say what they lock in. All three reviewers found the title clear: [chore] Replace tomli-w with in-house writer.
PR description
- "still raises
TypeError" describes dates and nested font-face arrays as iftomli-walready rejected them.tomli-w1.2.0 writes dates withstr()and emits[[fontFaces]]when an inline table would be multiline. GPT and Grok flagged this; Claude's review repeated the "still" wording. Proposed bullet: "A value outside that subset, such as a date or a list inside a font face, raisesTypeErrorsoconfig showfails visibly." - The Testing Plan lists
lib/tests/streamlit/runtime/credentials_test.py — credentials file contents, and that file is not in the diff. Claude flagged this. Proposed line: "lib/tests/streamlit/runtime/credentials_test.py— existingtest_Credentials_savestill asserts the unchanged credentials file contents." - "Made with Cursor" is meta-commentary. GPT and Grok flagged it. Remove the line.
lib/streamlit/toml_writer.py
- Line 54: "
numbers" is broader than the supported numeric types. GPT flagged this. Proposed sentence: "Supports strings, booleans, integers, floats, lists, tuples, and dicts." - Line 127: the docstring limits inline tables to dictionaries nested in arrays. The formatter also writes a dictionary nested inside an inline table. GPT flagged this. Proposed docstring: "Serialize a dict as a TOML inline table."
- Lines 15–21: the module docstring is already accurate. Grok proposed naming the writer as the actor in the last sentence. Optional closing sentence: "The writer raises
TypeErrorfor dates and any other unsupported type."
These are wording cleanups.
Recommendations
- In the PR body, replace the "still raises
TypeError" bullet with the wording in Readability, point the credentials testing-plan line at the existing assertion, and remove "Made with Cursor". - Optionally, apply the
toml_writer.pydocstring wording in Readability. - Optionally, track the existing
config showbehavior where a dict-valued option such as[trustedUserHeaders]closes the surrounding section. That behavior predates this PR.
Verdict
APPROVED: All three reviews approve, and the verified differences from tomli-w are a font-face paste fix, tighter string escaping, and an intentional TypeError for types no current config option uses.
This is an automated consolidated AI review 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.
Leaked "_test" options made the full-suite round-trip fail, and config show now asserts a set font face stays under [theme].
There was a problem hiding this comment.
Summary
All four expected reviews completed (gpt-5.6-sol-high, claude-opus-5-5-high, gemini-3.8-flash-high, grok-4.7-xhigh). None failed.
This PR removes the tomli-w runtime dependency and replaces it with lib/streamlit/toml_writer.py, used for the two TOML documents Streamlit writes: ~/.streamlit/credentials.toml and each assignment printed by streamlit config show. lib/pyproject.toml, uv.lock, the minimum-constraints file, and the lazy-loaded-modules e2e expectation are updated with it.
All four reviews approve. They agree the writer is limited to the types Streamlit actually writes, credentials output stays the same, and a long theme.fontFaces value stays an inline table under the [theme] header that config show prints. Differences are about how to describe that CLI change and about optional wording. No reviewer found a merge-blocking defect.
Product Alignment
This has limited user-facing impact. streamlit config show changes for long font-face arrays, and tomli-w is no longer installed with Streamlit. There is no new public API, config option, default, or UI.
gemini-3.8-flash-high marked product alignment not applicable and described CLI formats as preserved. The other three reviews, and the implementation, show a real output change: font faces are always inline tables, so a long face stays under the [theme] section config show already printed. tomli-w turned that array into [[fontFaces]], which closed the section. Gemini's own summary already describes that font-face behavior. The resolved assessment is the one shared by Claude, GPT, and Grok.
credentials.toml stays [general] with email = "...". U+0085, U+2028, and U+2029 are escaped so a commented config-show line stays one physical line; the parsed string is unchanged. A value outside this subset, such as a date or a list nested inside a font face, raises TypeError. Current options are strings, bools, ints, floats, lists, and dicts of those.
No product spec on develop covers this change, and none is required. specs/AGENTS.md excludes bug fixes and small non-controversial maintenance. The PR body does not claim a separate product approval, which is expected here. The common path — copying a config-show block into config.toml — stays valid TOML, and the change adds no permanent st.* surface.
Code Quality
All four reviews find the module proportionate to the dependency it removes, in line with lib/AGENTS.md (add a dependency only when the behavior cannot easily be replicated in-house). The file is one dumps entry point plus private helpers, with future annotations, _-prefixed internals, and a Numpydoc dumps docstring.
Spelling follows tomli-w for the documents Streamlit writes: 4-space multiline arrays with a trailing comma, the same compact escapes, bare keys limited to ASCII letters, digits, _, and -, and str() for ints and floats so a whole-number float stays a float after parsing. The intentional divergence is that a dict inside an array is always an inline table, including faces longer than tomli-w's 100-character heuristic. That is the config-show fix, and the tests lock it in. Unsupported types raise TypeError instead of guessing.
Optional notes on error text, a pre-existing dict-option layout, and a missing _format_string docstring are inline comments. No reviewer asked for a structural change.
Performance
All four reviews agree there is no meaningful hot-path impact. dumps runs only when saving credentials or printing config, on small documents. Element enqueue, ForwardMsg, reruns, caches, Arrow, and the frontend are untouched.
gemini-3.8-flash-high also said dropping the package reduces cold-start import overhead. The other three are more precise, and the diff matches them: tomli-w was imported lazily, only on write, and the e2e list already required it to stay unloaded at startup. config_util and credentials now import toml_writer at import time. That module only uses re, which config_util already imports, so script startup still avoids a third-party TOML import. The practical win is one fewer installed dependency. Performance coverage is not warranted.
Test Coverage
Coverage is sufficient, and all four reviews agree:
lib/tests/streamlit/toml_writer_test.pychecks scalar spelling, escapes (including Unicode line separators), the credentials document, quoted keys, empty containers, multiline arrays, font faces staying inline under[theme], andTypeErrorfor unsupported values, non-dict documents, non-string keys, and a list inside an inline table.lib/tests/streamlit/config_util_test.pyround-trips every real config option's default and current value throughtomllib, and parses a set font face out of the printed[theme]section.test_Credentials_savestill asserts the exact credentials file body.e2e_playwright/lazy_loaded_modules_test.pydropstomli_wand expects 14 markdown rows, matching the updated app script.
A CLI serializer does not need a new browser scenario. One overlapping unit test is called out inline; the credentials document and the dict-as-table behavior remain covered by other tests.
Backwards Compatibility
Reading config.toml and credentials.toml is unchanged (tomllib). Credentials written for a normal email, including quotes and backslashes, match the previous file body. Current config values keep their parsed types. The visible config-show difference is the long font-face case, which previously produced a section break and now stays valid TOML.
tomli-w is no longer installed as a Streamlit dependency. An environment that imported tomli_w without declaring it will need its own dependency. Streamlit does not treat that transitive import as a public API. Claude, GPT, and Grok all note this. Gemini's "fully backwards-compatible" leaves it out. It is ordinary dependency churn, and Streamlit's public API is unchanged. A release-note line is optional.
Security & Risk
All four reviews rate this low risk, and the writer matches that assessment. Credential emails are TOML basic strings. Quotes, backslashes, newlines, other C0 controls, DEL, and the Unicode line separators are escaped, so an email cannot break out of the string and add keys. The credentials path, permissions, and write path are unchanged. Dropping a runtime dependency shrinks the install-time supply chain. There is no new subprocess, eval, network call, authentication change, or path handling.
streamlit config show raises TypeError for a nested list inside a font face instead of emitting a [[fontFaces]] block. That aborts the command before it prints, which is the behavior the PR describes, and current theme options do not use that shape.
External test recommendation
- Recommend external_test: No
- Triggered categories: None
- Evidence: All four reviews agree, and the external-test checklist matches the diff.
lib/streamlit/toml_writer.pyserializes in-process values for credentials andstreamlit config show.lib/streamlit/config_util.pyandlib/streamlit/runtime/credentials.pyonly replace that local writer.lib/pyproject.tomlanduv.lockdrop thetomli-winstall dependency.e2e_playwright/lazy_loaded_modules.pyupdates the startup import list.- No routing, auth, WebSocket, embedding, asset serving, cross-origin, client storage, SiS/Snowflake, or security-header behavior changes. Test mentions of
server.trustedUserHeadersare config-show formatting of an existing option, not identity binding.
- Suggested external_test focus areas: None. Embedded hosting, auth handshakes, asset URLs, and SiS/Snowflake runtimes are unaffected.
- Confidence: High
- Assumptions and gaps: Call sites are credentials save and
streamlit config show. No remainingtomli_wimport is part of this change.
Accessibility
Not applicable. There are no frontend, rendered UI, or interaction changes. All four reviews agree.
Readability
Module and helper docs are in good shape: the module docstring states the supported subset and the font-face rule, _iter_table explains why dict values are deferred, and the bool-before-int check and the Unicode line-separator comment say why they exist. The credentials comment ("TOML has no null, so a missing email is stored as an empty string.") is clear. Line-level wording on _format_string, one error message, and one test name is in inline comments.
PR title. [chore] Replace tomli-w with in-house writer matches [type] Description and stands alone. Claude, GPT, and Gemini leave it as is. Grok would name the format explicitly. tomli-w already identifies TOML, so the current title is enough.
Optional title if the PR is edited again:
[chore] Replace tomli-w with a built-in TOML writer
PR description. GPT finds the current text sufficient: it names both documents, the font-face constraint, the TypeError behavior, and the tests, and the template sections are present. Claude, Gemini, and Grok want the motivation in the first sentences: one fewer runtime dependency, and tomli-w's long-value heuristic emitting [[fontFaces]]. That context is present, but it sits in the second bullet after the mechanism. The empty issue-link heading is the template default when there is no issue.
Optional opening if the description is edited again:
Streamlit writes `credentials.toml` and `streamlit config show` with a built-in TOML writer and no longer depends on `tomli-w`.
`tomli-w` turns a long font-face array into `[[fontFaces]]`, which closes the `[theme]` section that config show prints itself. The built-in writer keeps that array as one inline-table assignment under `[theme]`.Recommendations
- No change is required before merge. All four reviews approve, and the remaining disagreements are optional wording and follow-ups.
- If release notes are updated for this change, one line that
tomli-wis no longer installed with Streamlit would help users who imported it without declaring their own dependency.
Verdict
APPROVED: Four reviews agree this is a well-tested replacement of tomli-w that preserves credentials output, corrects long font-face config-show text, and introduces no merge-blocking product, performance, security, or compatibility issue.
This is an automated consolidated AI review by grok-4.7-xhigh. Please verify the feedback and use your judgment.
This review also includes 5 inline comment(s) on specific code lines.
There was a problem hiding this comment.
Auto-approved by AI final review (workflow run).
QA Report: Replace tomli-w with in-house TOML writerSummary
Overall statusPASS Test resultsPassed tests
Failed testsNone. Blocked testsNone. Issues foundIssue 1: Live server rewrites font-face
|
Unsupported config-show values now include the option key, and an inline table that does not fit on one line names the field.
|
The |
Describe your changes
Replaces the
tomli-wruntime dependency with a small writer for the two documents Streamlit produces:credentials.tomlandstreamlit config showassignments.[general]withemail = "...".[theme]header that config show prints. A separate[[fontFaces]]table would close that section.TypeErrorsoconfig showfails visibly.GitHub Issue Link (if applicable)
Testing Plan
Unit Tests (JS and/or Python)
E2E Tests
Any manual testing needed?
lib/tests/streamlit/toml_writer_test.py— writer spelling, escapes, the credentials document, and font faces staying under[theme]lib/tests/streamlit/config_util_test.py— config-show values round-trip, and a set font face stays under the printed[theme]headerlib/tests/streamlit/runtime/credentials_test.py— existingtest_Credentials_savestill asserts the unchanged credentials file contentse2e_playwright/lazy_loaded_modules_test.py—tomli_wis no longer imported at startup