Feat: streaming support#91
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fasthx/htmy.py (1)
245-263: Consider adding streaming support torender_component.This method always uses non-streaming rendering (
self.renderer.render) regardless of thestreamclass setting. For consistency withhx()andpage(), consider providing an optional streaming variant or astreamparameter.If the non-streaming behavior is intentional for the exception handler use case, a brief doc comment clarifying this would be helpful.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (6)
fasthx/core_decorators.py(5 hunks)fasthx/htmy.py(8 hunks)fasthx/jinja.py(4 hunks)fasthx/typing.py(2 hunks)pyproject.toml(1 hunks)tests/test_htmy.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
fasthx/jinja.py (2)
fasthx/htmy.py (4)
_make_render_function(266-272)_make_render_function(275-281)_make_render_function(283-332)hx(145-194)fasthx/core_decorators.py (3)
hx(21-27)hx(31-37)hx(40-128)
fasthx/core_decorators.py (2)
fasthx/typing.py (1)
StreamingRenderFunction(50-63)fasthx/utils.py (3)
append_to_signature(12-32)execute_maybe_sync_func(35-48)get_response(51-62)
fasthx/htmy.py (2)
fasthx/core_decorators.py (3)
hx(21-27)hx(31-37)hx(40-128)fasthx/typing.py (3)
RequestComponentSelector(67-98)StreamingRenderFunction(50-63)get_component(74-98)
tests/test_htmy.py (1)
fasthx/htmy.py (1)
HTMY(108-424)
🪛 Ruff (0.14.8)
fasthx/core_decorators.py
4-4: Import from collections.abc instead: Coroutine
Import from collections.abc
(UP035)
fasthx/htmy.py
332-332: Avoid specifying long messages outside the exception class
(TRY003)
401-401: Avoid specifying long messages outside the exception class
(TRY003)
427-427: Dynamically typed expressions (typing.Any) are disallowed in route_result
(ANN401)
tests/test_htmy.py
27-27: Unused noqa directive (non-enabled: C901)
Remove unused noqa directive
(RUF100)
153-156: Wrong values type in pytest.mark.parametrize expected list of tuple
Use list of tuple for parameter values
(PT007)
🔇 Additional comments (18)
fasthx/typing.py (2)
1-1: LGTM!Correct import of
AsyncIteratorfromcollections.abcfollowing modern Python conventions.
50-63: LGTM!The
StreamingRenderFunctionProtocol is well-designed:
- Correctly uses contravariant
Tcontrafor the input type- Returns
AsyncIterator[str]appropriate for streaming HTML chunks- Follows the same signature pattern as the existing
SyncRenderFunctionandAsyncRenderFunctiontests/test_htmy.py (2)
30-35: LGTM!Good conditional instantiation pattern for
HTMYwithStreamingRendererwhenstream=True. Theno_data_htmyinstance is also correctly configured with the same streaming behavior.
144-148: LGTM!The
clienthelper properly creates aTestClientwithraise_server_exceptions=Falseto allow testing HTTP 500 responses.fasthx/jinja.py (4)
240-242: LGTM!Good use of
KW_ONLYsentinel to enforce keyword-only arguments forno_datafield in the dataclass.
276-285: LGTM!The refactored code cleanly separates the creation of
render_funcanderror_render_func, improving readability. Thetype: ignorecomment for mypy generic resolution is documented with a TODO.
310-322: LGTM!Consistent refactoring pattern with
hx(). The separate variable assignments make the code flow clearer.
250-285: Consider adding streaming support toJinjaclass for feature parity.The
HTMYclass now supports streaming via thestreamparameter inhx()andpage()methods, butJinjadoes not expose this capability. If Jinja2 streaming is desired for consistency, consider adding thestreamparameter.Is the omission of streaming support in
Jinjaintentional? If so, documenting this difference in the class docstring would help users understand the distinction.Also applies to: 287-322
fasthx/core_decorators.py (5)
15-17: LGTM!Clear type alias definition for
HXReturnTypeimproving readability of the overloaded function signatures.
20-46: LGTM!Well-structured overloads that provide type-safe differentiation between streaming and non-streaming modes. The
Literal[True]/Literal[False]pattern enables proper type narrowing at call sites.
91-117: LGTM!The streaming implementation correctly:
- Casts
renderertoStreamingRenderFunctionwhenstream=True- Returns
StreamingResponsewith appropriatemedia_type="text/html"- Handles response status code and headers consistently with the non-streaming path
131-133: LGTM!
PageReturnTypefollows the same pattern asHXReturnTypewith the appropriate return type ofResponseinstead ofT | Response.
136-159: LGTM!Consistent overload pattern with
hx()for thepage()decorator, maintaining API symmetry.fasthx/htmy.py (5)
140-143: Verify default streaming behavior is intentional.Setting
stream: bool = Trueas the default means all existing users ofHTMYwill automatically get streaming responses if their renderer supports it. This could be a breaking change in behavior.Consider whether
stream: bool = Falsewould be a safer default to maintain backward compatibility, with users explicitly opting into streaming.
145-194: LGTM!The streaming logic is well-structured with a clean override pattern: per-decorator
streamparameter takes precedence over class-level setting, and streaming only activates when bothshould_streamis true and the renderer supports streaming.
196-243: LGTM!Implementation mirrors
hx()correctly, maintaining consistency across decorators.
334-401: LGTM - mirrors the render function pattern correctly.The error version correctly passes the exception to
get_component(request, result)for error handling, consistent with theRequestComponentSelectorprotocol. The duplication noted in_make_render_functionapplies here as well.
427-434: LGTM!The
Anytype usage is intentional here as this is a pass-through selector. The docstring clearly documents the assumption that the route result should be aComponent.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
fasthx/core_decorators.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
fasthx/core_decorators.py (2)
fasthx/typing.py (1)
StreamingRenderFunction(50-63)fasthx/utils.py (3)
append_to_signature(12-32)execute_maybe_sync_func(35-48)get_response(51-62)
🔇 Additional comments (5)
fasthx/core_decorators.py (5)
1-11: Imports are well-organized and address prior feedback.The
Coroutineimport has been moved tocollections.abcas suggested in the previous review. All new imports (StreamingResponse,StreamingRenderFunction,AsyncIterable,Literal,overload) are appropriate for the streaming functionality.
15-46: Well-designed overloads ensure type safety.The overload signatures correctly enforce that when
stream=True, bothrenderandrender_errormust beStreamingRenderFunction, and whenstream=False, they must beRenderFunction. This provides good compile-time guarantees for users of the API.
91-117: Streaming implementation is correct.The streaming path properly:
- Casts to
StreamingRenderFunction(safe due to overload enforcement)- Uses
AsyncIterable[str]which correctly acceptsAsyncIterator[str]returns- Handles the edge case where
response.status_codecould beNoneviaor 200- Preserves headers and background tasks from the response dependency
The non-streaming path correctly delegates to
execute_maybe_sync_funcfor sync/async renderer compatibility.
131-159: PageReturnType and overloads are consistent with hx pattern.The
PageReturnTypecorrectly specifiesResponseas the always-returned type (unlikehxwhich can returnT | Responsefor non-HX requests). The overload structure mirrors thehxdecorator appropriately.
176-184: Error handling correctly preserved for page decorator.The simpler error handling (only checking
render_error is None) is appropriate here sincepagealways has a request context, unlikehxwhich needs the additional__hx_request is Nonecheck.
StreamingResponse allows
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
fasthx/core_decorators.py (1)
191-203: Add type annotation for consistency withhx()decorator.Line 193 lacks the explicit type annotation that's present on line 93 in the
hxdecorator. This is a minor inconsistency.🔎 Proposed fix for consistency
if stream: renderer = cast(StreamingRenderFunction[T | Exception], renderer) - content_stream = renderer(result, context=kwargs, request=__page_request) + content_stream: ContentStream = renderer(result, context=kwargs, request=__page_request)Note: This would require importing
ContentStreamat the top:+from starlette.responses import ContentStreamOr using a
TYPE_CHECKINGguard if preferred.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
fasthx/core_decorators.py(5 hunks)fasthx/typing.py(2 hunks)
🔇 Additional comments (6)
fasthx/typing.py (1)
1-8: LGTM! Clean forward-reference setup.The use of
from __future__ import annotationscombined withTYPE_CHECKINGguards forRequestandContentStreamis the correct pattern for avoiding circular imports and runtime overhead while maintaining type safety.fasthx/core_decorators.py (5)
1-11: LGTM! Import cleanup and additions.The imports are well-organized.
Coroutineis now correctly imported fromcollections.abc(addressing the previous Ruff UP035 suggestion), and the new streaming-related imports are appropriately added.
15-46: LGTM! Well-structured overloads for type narrowing.The overload pattern correctly guides type checkers to expect
StreamingRenderFunctionwhenstream=TrueandRenderFunctionwhenstream=False. The implementation signature properly accepts the union of both types.
91-117: LGTM! Streaming and non-streaming paths are correctly implemented.The branching logic properly handles both streaming (
StreamingResponse) and non-streaming (HTMLResponse) paths. Thegetattrpattern withor 200fallback correctly handles the case where FastAPI's Response dependency hasstatus_code=None.
131-158: LGTM! Consistent overload pattern withhx().The
PageReturnTypealias and overloads mirror the structure ofhx(), maintaining consistency. The return type correctly reflects thatpage()always returns aResponse(unlikehx()which may return the raw value for non-HTMX requests).
204-228: LGTM! Non-streaming path and signature handling are correct.The non-streaming path and the signature manipulation via
append_to_signatureare implemented correctly, maintaining consistency with thehx()decorator pattern.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
fasthx/typing.py (1)
55-68: Docstring still misaligns withContentStreamtype.The docstring states the return is "An async generator that yields HTML string chunks", but
ContentStreamfrom Starlette also accepts sync iterables and byte chunks.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
examples/htmy-streaming/htmy_streaming_app.py(1 hunks)fasthx/typing.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/htmy-streaming/htmy_streaming_app.py (1)
fasthx/htmy.py (2)
HTMY(108-424)get(83-85)
🪛 Ruff (0.14.8)
examples/htmy-streaming/htmy_streaming_app.py
58-58: Unnecessary ... literal
Remove unnecessary ...
(PIE790)
🔇 Additional comments (3)
fasthx/typing.py (1)
1-8: LGTM on the TYPE_CHECKING structure.Good use of
TYPE_CHECKINGto avoid runtime import cycles while still providing type hints. The forward reference annotations withfrom __future__ import annotationswork well with this pattern.examples/htmy-streaming/htmy_streaming_app.py (2)
11-15: LGTM on the streaming component demo.Good demonstration of an async component with intentional delay to showcase streaming behavior. The 1-second sleep per item clearly illustrates the progressive rendering.
18-42: Well-structured page component.Clean composition of the HTML structure with proper DOCTYPE, meta tags, and semantic markup. The generator expression for list items efficiently creates the streaming demo content.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
README.mddocs/index.mdexamples/htmy-streaming/htmy_streaming_app.py
🧰 Additional context used
🧬 Code graph analysis (1)
examples/htmy-streaming/htmy_streaming_app.py (3)
fasthx/htmy.py (2)
HTMY(108-424)get(83-85)tests/htmy_components.py (2)
h1(92-94)p(96-98)tests/test_dependencies.py (1)
app(9-22)
🪛 LanguageTool
README.md
[style] ~30-~30: Consider using a more formal and expressive alternative to ‘awesome’.
Context: ... the holm/htmy/FastHX stack, it's awesome." (David Brochart, Technical Director a...
(AWESOME)
docs/index.md
[style] ~30-~30: Consider using a more formal and expressive alternative to ‘awesome’.
Context: ... the holm/htmy/FastHX stack, it's awesome." (David Brochart, Technical Director a...
(AWESOME)
🪛 Ruff (0.14.10)
examples/htmy-streaming/htmy_streaming_app.py
58-58: Unnecessary ... literal
Remove unnecessary ...
(PIE790)
🔇 Additional comments (7)
docs/index.md (2)
21-21: LGTM! Clear documentation of the new streaming feature.The addition accurately describes the streaming capability and its performance benefits.
30-34: LGTM! Testimonials updated appropriately.The testimonial updates preserve the authentic voice of the reviewers. The informal language flagged by static analysis is appropriate in this context as these are direct quotes.
README.md (1)
30-34: LGTM! Testimonials updated appropriately.The testimonial updates preserve the authentic voice of the reviewers.
examples/htmy-streaming/htmy_streaming_app.py (4)
1-16: LGTM! Clean async component implementation.The
slow_list_itemcomponent correctly demonstrates async streaming with proper use of the@componentdecorator, async/await syntax, and appropriate delay simulation.
18-42: LGTM! Well-structured streaming page example.The
index_pagefunction correctly demonstrates streaming HTML with async components. The generator expression on line 37 creates 32 async list items that will be streamed as they resolve, effectively showcasing the streaming capability.
45-51: LGTM! Correct streaming renderer setup.The application correctly initializes
HTMYwithStreamingRenderer(), enabling the streaming functionality demonstrated in this example.
54-58: Route implementation looks correct.The route properly uses the
htmy.pagedecorator to render the streaming index page. Note that a previous review comment has already addressed the use of...vspasson line 58.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (7)
README.mddocs/examples/htmy-streaming.mddocs/index.mdexamples/htmy-streaming/htmy_streaming_app.pyfasthx/core_decorators.pyfasthx/utils.pymkdocs.yaml
🧰 Additional context used
🧬 Code graph analysis (1)
fasthx/core_decorators.py (2)
fasthx/typing.py (1)
StreamingRenderFunction(55-68)fasthx/utils.py (3)
append_to_signature(15-46)execute_maybe_sync_func(49-62)get_response(65-76)
🪛 GitHub Actions: Linters
examples/htmy-streaming/htmy_streaming_app.py
[error] 23-23: mypy error: Argument 1 has incompatible type "Callable[[], SyncComponent | AsyncComponent | str | None | list[SyncComponent | AsyncComponent | str | None] | tuple[SyncComponent | AsyncComponent | str | None, ...]]"; expected "def (*Never, **Never) -> Response | def (*Never, **Never) -> Coroutine[Any, Any, Response]" (arg-type). Subtask 'mypy' returned non-zero exit status.
🪛 LanguageTool
docs/index.md
[style] ~30-~30: Consider using a more formal and expressive alternative to ‘awesome’.
Context: ... the holm/htmy/FastHX stack, it's awesome." (David Brochart, Technical Director a...
(AWESOME)
README.md
[style] ~30-~30: Consider using a more formal and expressive alternative to ‘awesome’.
Context: ... the holm/htmy/FastHX stack, it's awesome." (David Brochart, Technical Director a...
(AWESOME)
docs/examples/htmy-streaming.md
[style] ~11-~11: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ication The application itself will be very simple: a single page that displays a list of ...
(EN_WEAK_ADJECTIVE)
🪛 Ruff (0.14.10)
fasthx/utils.py
18-18: Dynamically typed expressions (typing.Any) are disallowed in return_annotation
(ANN401)
🔇 Additional comments (16)
fasthx/utils.py (2)
11-12: Good use of sentinel pattern for default value.Using a unique
object()instance as a sentinel is the idiomatic Python approach to distinguish between "parameter not provided" and an explicitNonevalue.
15-45: LGTM - Clean implementation of optional return annotation override.The implementation correctly:
- Preserves the original return annotation when
return_annotationis not provided- Overrides only when explicitly specified
- Uses proper conditional expression in the
signature.replace()callRegarding the Ruff ANN401 hint about
Any: this is acceptable here since return annotations can legitimately be of any type (includingNone, type objects, strings, etc.).mkdocs.yaml (1)
55-55: LGTM - Navigation entry correctly added.The new streaming example is appropriately positioned after the main htmy example in the navigation hierarchy.
docs/index.md (2)
21-21: LGTM - Feature bullet accurately describes the new streaming capability.The description clearly communicates the performance benefits (TTFB and FCP optimization) of the streaming feature.
30-34: Testimonials look good.The LanguageTool hint about "awesome" can be safely ignored - this is a direct quote from a user testimonial, and changing the wording would misrepresent what they said.
README.md (2)
21-21: LGTM - Streaming feature description is complete and consistent.The sentence now correctly includes "first contentful paint" and matches the corresponding text in
docs/index.md.
30-34: Testimonials are consistent with docs.Same content as
docs/index.md- good for maintaining consistency between README and documentation.docs/examples/htmy-streaming.md (1)
1-89: LGTM - Well-structured documentation for the streaming feature.The documentation:
- Clearly explains the benefits of HTML streaming (TTFB/FCP)
- Provides progressive code snippets with syntax highlighting
- Includes practical run instructions
- Matches the actual example code
The LanguageTool suggestion about "very simple" is a minor stylistic preference and can be safely ignored.
examples/htmy-streaming/htmy_streaming_app.py (3)
15-19: LGTM - Clean async component implementation.The
slow_list_itemcomponent correctly demonstrates async streaming by awaiting a 1-second delay before returning the list item element.
26-49: LGTM - Well-structured streaming page component.The component hierarchy is clean and demonstrates:
- Proper DOCTYPE declaration
- Complete HTML5 structure
- External CSS integration (PicoCSS)
- Generator expression for creating multiple async list items
22-24: The type annotations appear sound and do not indicate a mypy error.The
@htmy.page()decorator properly accepts functions returningComponent. The method's signature is generic—it acceptsMaybeAsyncFunc[P, T | Response]whereTresolves toComponentfor theindex()function, and returns a coroutine yieldingT | Response. Thereturn_annotation=Responsein the basepage()decorator (line 229 incore_decorators.py) only overrides FastAPI's type resolution to prevent it from attempting to parse the return type; it does not invalidate the decorator's compatibility withComponentreturn types.Additionally, mypy errors in this project do not fail the CI pipeline due to
check.ignore_fail = "return_non_zero"in the poe configuration, and the recent mypy fix (commit 0355bfb) did not modify this example file, suggesting no type issues were identified.Likely an incorrect or invalid review comment.
fasthx/core_decorators.py (5)
2-10: LGTM - Clean import organization.Imports are well-organized:
Coroutinecorrectly imported fromcollections.abc(addressing previous review feedback)- New streaming-related imports (
StreamingResponse,StreamingRenderFunction,Literal) properly added
15-46: Well-designed overload structure for streaming support.The overload design correctly:
- Uses
Literal[True]andLiteral[False]to ensure type safety at call sites- Separates
StreamingRenderFunctionfromRenderFunctionbased on thestreamparameter- Maintains backward compatibility with
stream: Literal[False] = Falseas defaultThis allows type checkers to correctly infer the renderer type based on the
streamargument.
91-117: LGTM - Streaming response construction is correct.The streaming branch:
- Properly casts the renderer to
StreamingRenderFunction- Correctly constructs
StreamingResponsewith the content stream- Sets
media_type="text/html"explicitly- Preserves response attributes (status_code, headers, background)
131-158: LGTM - Consistent overload pattern for page decorator.The
pagedecorator follows the same well-designed overload pattern ashx, ensuring consistent API design across both decorators.
191-217: LGTM - Streaming logic mirrors hx decorator.The streaming implementation in
pagecorrectly mirrors thehxdecorator's logic. The acknowledged code duplication is acceptable for now as noted by the maintainer.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
docs/examples/htmy-streaming.mdexamples/htmy-streaming/htmy_streaming_app.py
🧰 Additional context used
🧬 Code graph analysis (1)
examples/htmy-streaming/htmy_streaming_app.py (1)
fasthx/htmy.py (1)
HTMY(108-424)
🪛 LanguageTool
docs/examples/htmy-streaming.md
[style] ~11-~11: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ication The application itself will be very simple: a single page that displays a list of ...
(EN_WEAK_ADJECTIVE)
🔇 Additional comments (3)
examples/htmy-streaming/htmy_streaming_app.py (3)
1-12: LGTM! Clean setup for streaming example.The imports and initialization correctly set up FastAPI with HTMY's StreamingRenderer, demonstrating the new streaming capability.
15-19: LGTM! Component correctly demonstrates async streaming.The async component with a 1-second delay effectively demonstrates how items are streamed progressively as they become ready.
22-49: LGTM! Route correctly implements streaming page.The route structure is correct and effectively demonstrates HTML streaming. The
type: ignore[arg-type]on line 23 is acceptable given the explanation in the PR commit message that mypy cannot resolve the generic types.The use of 32 async list items (each with a 1-second delay) provides a clear demonstration of progressive streaming as items appear one by one.
1 participant
Closes #90 #92
Changes:
htmy).page(), because in those cases the returned value is always aResponse.htmydependency to>=0.10.1(that's first version that has a streaming renderer).Summary by CodeRabbit
New Features
Behavior
Tests
Examples
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.