feat(cli): implement missing output element types and handles #1191

Merged
freemo merged 1 commit from feature/m5-output-elements into master 2026-03-30 20:09:28 +00:00
Owner

Summary

Implements the LiveMaterializationStrategy — the third materialization strategy type defined in the specification's Output Rendering Framework. Updates RichMaterializer to use live rendering instead of sequential buffer delegation.

Changes

  • src/cleveragents/cli/output/materializers.py: Added _LiveMaterializationStrategy class (~130 lines) with ~15fps throttling, dirty-element tracking, per-frame composition, and thread-safe operation. Updated RichMaterializer to extend it.
  • features/output_rendering.feature: 5 new BDD scenarios for live strategy
  • features/steps/output_rendering_steps.py: Step definitions
  • vulture_whitelist.py: Added LiveMaterializationStrategy entry

Note

All 10 element types and handle classes were already fully implemented in prior work. The actual gap was only the LiveMaterializationStrategy.

Closes #903

## Summary Implements the `LiveMaterializationStrategy` — the third materialization strategy type defined in the specification's Output Rendering Framework. Updates `RichMaterializer` to use live rendering instead of sequential buffer delegation. ## Changes - `src/cleveragents/cli/output/materializers.py`: Added `_LiveMaterializationStrategy` class (~130 lines) with ~15fps throttling, dirty-element tracking, per-frame composition, and thread-safe operation. Updated `RichMaterializer` to extend it. - `features/output_rendering.feature`: 5 new BDD scenarios for live strategy - `features/steps/output_rendering_steps.py`: Step definitions - `vulture_whitelist.py`: Added `LiveMaterializationStrategy` entry ## Note All 10 element types and handle classes were already fully implemented in prior work. The actual gap was only the `LiveMaterializationStrategy`. Closes #903
freemo added this to the v3.5.0 milestone 2026-03-29 06:07:33 +00:00
freemo left a comment

Review: Looks Good (self-authored — posted as comment)

Well-designed concurrency model with proper frame coalescing. Thread-safe _lock protecting mutable state. _maybe_render_frame correctly uses time.monotonic() for frame timing. Deviation docs updated. Minor: add changelog entry and consider Robot integration test.

## Review: Looks Good (self-authored — posted as comment) Well-designed concurrency model with proper frame coalescing. Thread-safe `_lock` protecting mutable state. `_maybe_render_frame` correctly uses `time.monotonic()` for frame timing. Deviation docs updated. Minor: add changelog entry and consider Robot integration test.
freemo left a comment

Updated Review (Deep Pass): Changes Required

My initial review looked good. The deep review reveals issues.

New Finding: Missing get_error_output() method — potential runtime error

_LiveMaterializationStrategy does NOT implement get_error_output(), but other strategies (_BaseBufferStrategy, _AccumulateStrategy) do. If get_error_output() is called on a RichMaterializer (which extends _LiveMaterializationStrategy), it will raise AttributeError. This is a potential runtime error in error paths.

New Finding: materializers.py pushed past 500 lines (now 677 lines)

This PR adds ~176 lines, pushing the file from ~501 to 677 lines — well over the 500-line limit. The _LiveMaterializationStrategy class (~170 lines) should be extracted to its own module.

New Finding: Dead field _ended

self._ended: bool = False is set on on_session_end() but never read anywhere. Either remove it or use it.

New Finding: import time as _time inside hot-path methods

Both _maybe_render_frame and _render_frame import time inside the method body. While Python caches module imports, this is unnecessary overhead on every call (~15 fps) and violates the import guidelines.

Previous finding still applies: Thread safety model is well-designed.

## Updated Review (Deep Pass): Changes Required My initial review looked good. The deep review reveals issues. ### New Finding: Missing `get_error_output()` method — potential runtime error `_LiveMaterializationStrategy` does NOT implement `get_error_output()`, but other strategies (`_BaseBufferStrategy`, `_AccumulateStrategy`) do. If `get_error_output()` is called on a `RichMaterializer` (which extends `_LiveMaterializationStrategy`), it will raise `AttributeError`. This is a potential **runtime error in error paths**. ### New Finding: `materializers.py` pushed past 500 lines (now 677 lines) This PR adds ~176 lines, pushing the file from ~501 to 677 lines — well over the 500-line limit. The `_LiveMaterializationStrategy` class (~170 lines) should be extracted to its own module. ### New Finding: Dead field `_ended` `self._ended: bool = False` is set on `on_session_end()` but never read anywhere. Either remove it or use it. ### New Finding: `import time as _time` inside hot-path methods Both `_maybe_render_frame` and `_render_frame` import `time` inside the method body. While Python caches module imports, this is unnecessary overhead on every call (~15 fps) and violates the import guidelines. ### Previous finding still applies: Thread safety model is well-designed.
freemo scheduled this pull request to auto merge when all checks succeed 2026-03-30 19:52:46 +00:00
freemo force-pushed feature/m5-output-elements from 540cb7fc1b
All checks were successful
CI / lint (pull_request) Successful in 3m25s
CI / quality (pull_request) Successful in 3m40s
CI / typecheck (pull_request) Successful in 3m58s
CI / security (pull_request) Successful in 4m10s
CI / build (pull_request) Successful in 25s
CI / helm (pull_request) Successful in 35s
CI / unit_tests (pull_request) Successful in 4m33s
CI / docker (pull_request) Successful in 1m21s
CI / integration_tests (pull_request) Successful in 7m4s
CI / e2e_tests (pull_request) Successful in 9m27s
CI / coverage (pull_request) Successful in 12m29s
CI / status-check (pull_request) Successful in 2s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 58m42s
to afe6b849fe
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 19s
CI / helm (pull_request) Successful in 21s
CI / lint (pull_request) Successful in 3m19s
CI / quality (pull_request) Successful in 3m52s
CI / typecheck (pull_request) Successful in 3m56s
CI / security (pull_request) Successful in 4m5s
CI / integration_tests (pull_request) Successful in 7m2s
CI / unit_tests (pull_request) Successful in 11m51s
CI / docker (pull_request) Successful in 1m31s
CI / coverage (pull_request) Successful in 11m50s
CI / e2e_tests (pull_request) Successful in 16m31s
CI / status-check (pull_request) Successful in 1s
CI / build (push) Successful in 17s
CI / helm (push) Successful in 21s
CI / lint (push) Successful in 24s
CI / quality (push) Successful in 3m41s
CI / integration_tests (push) Successful in 3m46s
CI / typecheck (push) Successful in 3m55s
CI / security (push) Successful in 4m6s
CI / unit_tests (push) Successful in 7m14s
CI / docker (push) Successful in 1m32s
CI / benchmark-regression (push) Has been skipped
CI / coverage (push) Successful in 11m46s
CI / e2e_tests (push) Successful in 18m1s
CI / status-check (push) Successful in 1s
CI / benchmark-publish (push) Successful in 28m19s
CI / benchmark-regression (pull_request) Successful in 54m43s
2026-03-30 19:52:53 +00:00
Compare
freemo merged commit afe6b849fe into master 2026-03-30 20:09:28 +00:00
freemo deleted branch feature/m5-output-elements 2026-03-30 20:09:28 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core!1191
No description provided.