Skip to content

Inject SseServer instead of the static SSE facade - #45

Merged
syntaxwanderer merged 1 commit into
masterfrom
develop
Aug 19, 2026
Merged

Inject SseServer instead of the static SSE facade#45
syntaxwanderer merged 1 commit into
masterfrom
develop

Conversation

@syntaxwanderer

@syntaxwanderer syntaxwanderer commented Aug 19, 2026

Copy link
Copy Markdown
Member

What

Part of ep-kill-static-facades: DemoExecutionShowcaseService takes the container-managed SseServer via #[InjectAsReadonly]; the heartbeat console command stays on the facade (static context, allowlisted in the guard rule).

Requires the paired semitexa-ssr PR (SseServer).

Verification

  • Full workbench gate incl. 86 Playwright E2E green

🤖 Generated with Claude Code

https://claude.ai/code/session_01WfzwnJKXsFfsGVqRkHEbCD

Summary by CodeRabbit

  • Bug Fixes
    • Improved event delivery in the demo execution showcase for more reliable server-sent event handling.

Part of tk-facades-sse-server - DemoExecutionShowcaseService takes the
container-managed SseServer via #[InjectAsReadonly]; the heartbeat
console command stays on the facade (static context).

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

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 773253d0-8599-4580-9ad8-dd6ee3899715

📥 Commits

Reviewing files that changed from the base of the PR and between ba7a255 and e59cf7c.

📒 Files selected for processing (1)
  • src/Application/Service/DemoExecutionShowcaseService.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The service now injects SseServer and uses its instance method to deliver SSE payloads.

Changes

SSE delivery

Layer / File(s) Summary
Instance-backed SSE delivery
src/Application/Service/DemoExecutionShowcaseService.php
The service replaces AsyncResourceSseServer with SseServer and changes SSE delivery to use $this->sseServer->deliver().

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to e59cf

This change replaces a static SSE call with the container-managed service in one application service; no actionable merge-blocking risk remains after normal checks and review.

Poem

A rabbit hops through streams of light,
With SseServer tucked in tight.
No static call, the messages flow,
Instance-led events gently go.
Hop, hop, delivered right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: injecting SseServer instead of using the static SSE facade.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch develop

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the demo execution “showcase” SSE emission path to use a container-managed SseServer instance rather than the static AsyncResourceSseServer facade, aligning with the ep-kill-static-facades effort while keeping the console heartbeat on the facade.

Changes:

  • Replaced AsyncResourceSseServer::deliver(...) with an injected SseServer->deliver(...) call.
  • Added SseServer dependency injection to DemoExecutionShowcaseService via #[InjectAsReadonly].

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@syntaxwanderer
syntaxwanderer merged commit 0fa4b2f into master Aug 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants