Skip to content

Remove stale rich-markup escapes leaking backslashes into logged warnings - #6989

Merged
masenf merged 6 commits into
mainfrom
claude/rel-fix-warning-escape-leak
Aug 28, 2026
Merged

Remove stale rich-markup escapes leaking backslashes into logged warnings#6989
masenf merged 6 commits into
mainfrom
claude/rel-fix-warning-escape-leak

Conversation

@masenf

@masenf masenf commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Defect

FINDING-027 from 0.9.9a1 pre-release testing (cosmetic regression vs 0.9.8, verifier-confirmed): compile-time warnings print literal backslash-escaped brackets, e.g. the event-handler arg-mismatch warning renders as

Warning: Event handler on_submit expects (dict\[str, typing.Any]) -> () but got (dict\[str, str]) -> () ...

where 0.9.8 printed clean dict[str, ...].

Root cause: 0.9.8 built these messages with rich-markup escaping (rich.markup.escape() / .replace("[", "\\[")) and emitted them via the legacy console.* helpers, whose rich Console rendered with markup enabled and consumed the \[ escapes. The 0.9.9 migration to the stdlib logging pipeline kept the escaping, but RichConsoleHandler.emit renders records with markup=False unless a record opts in via extra={"rich": True} (these call sites do not), so the escapes now print literally — in the console sink, the JSON sink, and the full-logging file.

Fix

Drop the stale escaping at every call site that logs through the pipeline without the markup opt-in (all confirmed to use console.* + escaping on the v0.9.8 tag, i.e. all the same migration regression):

  • reflex_base/event/__init__.py — the arg-mismatch warning from the finding's repro (.replace("[", "\\[") on both type lists)
  • reflex_base/components/component.py — None-to-non-Optional Var deprecation warning
  • reflex_base/vars/base.py — computed var return-type error
  • reflex_base/event/processor/event_processor.py — event-queue processing error ([txid=...])
  • reflex/state.py — inline computed var type warning and state field type error

Bracketed text now renders verbatim. Markup injection from user-controlled values (type names, event payloads) stays impossible: the sink keeps markup=False, nothing opts into markup rendering. Call sites that do opt in via extra={"rich": True} (e.g. js_runtimes.py, prerequisites.py) correctly keep their escaping and are untouched, as is the deliberate escaping of client-supplied JS error text placed into Exception objects for user-defined frontend exception handlers.

Test plan

  • New regression test tests/units/test_event.py::test_arg_mismatch_warning_renders_brackets_verbatim: reproduces the finding's scenario (handler annotated dict[str, str] bound to the on_submit specs) through the managed pipeline and asserts the captured rendered output contains expects (dict[str, typing.Any]) -> () but got (dict[str, str]) -> () with no backslashes. Fails on unfixed main with dict\[str, typing.Any] in the output; passes with the fix.
  • New injection-safety test tests/units/reflex_base/utils/test_log.py::test_warning_with_markup_tags_stays_literal: a warning containing [red]x[/red] renders it literally — neither styled/stripped nor escaped.
  • uv run ruff check . and uv run ruff format . clean; uv run pyright reflex tests passes (0 errors); pyright over reflex_base shows only a pre-existing warning unrelated to this change.
  • Scoped suites pass: tests/units/test_event.py, tests/units/reflex_base/utils/test_log.py, tests/units/reflex_base/event/processor/test_event_processor.py, tests/units/components/test_component.py, tests/units/test_state.py, tests/units/test_var.py, tests/units/vars. (One pre-existing cross-file ordering failure, test_superseding_event_logs_debug_on_cancel when test_log.py runs before test_event_processor.py in the same invocation, reproduces identically on unmodified main and is unrelated.)

🤖 Generated with Claude Code

https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x


Generated by Claude Code

Review in cubic

The 0.9.9 migration from the legacy console helpers to stdlib logging
kept the rich-markup escaping (escape() / .replace("[", "\\[")) that the
old markup-rendering path consumed. The new RichConsoleHandler renders
records with markup=False unless a record opts in via extra={"rich":
True}, so the leftover escapes now print literally: compile-time
warnings such as the event-handler arg-mismatch warning showed
"dict\[str, typing.Any]" instead of "dict[str, typing.Any]"
(FINDING-027 from 0.9.9a1 pre-release testing, a cosmetic regression vs
0.9.8).

Drop the escaping at every call site that logs through the pipeline
without the markup opt-in: the event arg-mismatch warning, the
None-to-non-Optional Var warning, the computed-var return-type errors,
the event-queue processing error, and the state field type error.
Bracketed text now renders verbatim, and markup injection from
user-controlled values stays impossible because the sink keeps markup
disabled. Call sites that opt into markup (extra={"rich": True}) keep
their escapes, as does the escaping of client-supplied data placed into
Exception objects for user-defined handlers.

Regression tests capture the rendered console output: the arg-mismatch
warning must contain clean "dict[str, str]" with no backslashes, and a
warning containing markup-like text ("[red]x[/red]") must render it
literally without styling.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
@masenf
masenf requested a review from a team as a code owner August 28, 2026 18:22
@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes obsolete Rich-markup escaping from warnings, errors, and VarAttributeError messages now rendered by the plain stdlib logging pipeline.

  • Preserves literal brackets in event, state, component, and computed-var diagnostics.
  • Adds regression coverage for bracket rendering and markup-like text.
  • Adds release-note fragments for the root and reflex-base packages.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/event/init.py Removes stale bracket escaping from event-handler argument mismatch warnings while retaining the existing validation behavior.
packages/reflex-base/src/reflex_base/event/processor/event_processor.py Logs event-queue failures verbatim through the non-markup logging path.
packages/reflex-base/src/reflex_base/components/component.py Removes Rich escaping from the None-to-non-Optional deprecation warning.
packages/reflex-base/src/reflex_base/vars/base.py Removes obsolete escaping from computed-var diagnostics and attribute errors.
packages/reflex-base/src/reflex_base/vars/object.py Preserves generic type brackets verbatim in object-var attribute errors.
reflex/state.py Removes obsolete escaping from inline computed-var and state-field type diagnostics.
tests/units/reflex_base/utils/test_log.py Verifies that markup-like warning text remains literal when records do not opt into Rich rendering.
tests/units/test_event.py Adds end-to-end regression coverage for clean bracket rendering in argument mismatch warnings.

Reviews (3): Last reviewed commit: "Remove test asserting on the dropped mar..." | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Aug 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 32 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing claude/rel-fix-warning-escape-leak (eeba048) with main (7427617)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 8 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread tests/units/test_event.py
@masenf masenf added this to the v0.9.9 milestone Aug 28, 2026
@FarhanAliRaza FarhanAliRaza added the skip-changelog For doc/internal changes label Aug 28, 2026
FarhanAliRaza
FarhanAliRaza previously approved these changes Aug 28, 2026
The changelog gate requires a fragment in each package whose sources the
PR touches; the fix edits reflex-base modules but only carried the root
news entry. Mirror it under packages/reflex-base/news/ so
reflex-release changelog-check passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
FarhanAliRaza

This comment was marked as resolved.

FarhanAliRaza

This comment was marked as resolved.

FarhanAliRaza and others added 3 commits August 29, 2026 00:27
The messages land in plain tracebacks, never in a markup-enabled sink, so
the escaped brackets printed literally (dict\[str, int]).

Claude-Session: https://claude.ai/code/session_01JV6TsCnMzU4kyZ9ejSPyqY
Reviewer follow-up: the same stale escaping fixed for warnings also
lived in the VarAttributeError messages raised by Var.__getattr__ and
ObjectVar.__getattr__, which land in plain tracebacks — so bracketed
type names printed as dict\[str, int]. Removes the escape() calls and
now-unused imports, extends the news fragments, and adds a regression
test that fails against the unescaped-message-free sources.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x

masenf commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

@FarhanAliRaza thanks — both VarAttributeError sites are now de-escaped on this branch (the source change landed in 0b16599/d5e0344), and 2ec0993 adds your suggested regression test (tests/units/test_var.py::test_var_attribute_error_renders_brackets_verbatim), verified to fail against the pre-fix sources and pass with them. The news fragments mention the VarAttributeError messages as well.


Generated by Claude Code

@masenf
masenf requested a review from FarhanAliRaza August 28, 2026 19:29
Testing that a removed escape stays removed is not worth a test.

Claude-Session: https://claude.ai/code/session_01JV6TsCnMzU4kyZ9ejSPyqY
@masenf
masenf merged commit 6c6a398 into main Aug 28, 2026
111 checks passed
@masenf
masenf deleted the claude/rel-fix-warning-escape-leak branch August 28, 2026 19:43
masenf pushed a commit that referenced this pull request Aug 28, 2026
…-context-refactor-jv3pig

Picks up #6987, #6990, #6989, #6992 and #6993. Clean auto-merge; main's
``templates.py`` edit is again in the Vite config template, away from
``render_iterable_tag``.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog For doc/internal changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants