refactor: split three oversized files (display_list, paint, tv_controller)

[AndrewAltimit]

Summary

Pure code-motion refactor to bring three large files under the 2500-line target. No behavior change.

• paint/display_list.rs (4022 lines) -> display_list/ directory

  • mod.rs (667): DisplayItem enum + DisplayList struct + public replay/replay_dirty dispatchers
  • mask.rs (869): MaskParams + apply_mask + linear/radial/url mask rasterizers
  • optimize.rs (787): compact() + optimize() + merge_vertical_strips + eliminate_occluded
  • replay.rs (1753): replay_inner + replay_dirty_inner + ActiveLayer + helpers (clip intersection, sticky offset, border-edge styling)
  • Tests live alongside the code they cover (each submodule has its own #[cfg(test)] mod tests)

• paint/mod.rs (2584 -> 1022): extracted the inline mod tests block to a sibling tests.rs (1265 lines). The integration tests can still reach private helpers via super::*.

• tv_controller/mod.rs (1946 -> 56): same treatment -- extracted the inline mod tests block to tests.rs (1887 lines). The per-concern submodules (streaming_buffer, cdn_failover, seek, download, player, tune, catalog) were already split out in earlier PRs; mod.rs was dominated by tests for those submodules.

Test plan

• cargo build --workspace
• cargo clippy --workspace -- -D warnings (matches CI)
• cargo fmt --all -- --check
• cargo test --workspace -- 1235 tests pass, 0 fail

Generated with Claude Code

[github-actions]

Claude AI Architecture & Quality Review

Issues

• [WARNING] crates/oasis-browser/src/paint/display_list/mod.rs — items and recording_scroll_y are declared pub(super) on DisplayList, but this is architecturally unnecessary and actively harmful. In Rust, child modules (optimize.rs, replay.rs, mask.rs) already have direct access to private fields of types defined in their parent module (display_list/mod.rs) — that's inherent to Rust's privacy model. pub(super) doesn't help the children; it widens access upward to paint/mod.rs, allowing the paint compositor to bypass DisplayList's controlled public API (push(), clear(), items() method, etc.). If paint/mod.rs genuinely needs direct field access, that's a design decision worth making explicitly; if it doesn't, remove pub(super) and restore private visibility.

Previous Issues

None on record for this branch.

Suggestions

• crates/oasis-app/src/tv_controller/tests.rs — the original mod tests was gated with both #[cfg(test)] and #[cfg(feature = "_video")] on the declaration in mod.rs, so the module didn't exist at all without the feature. The new split declares mod tests; unconditionally (only #[cfg(test)]) and moves the feature gate inside the file as #![cfg(feature = "_video")]. This compiles an empty module in test builds without _video, which is harmless but differs from the original. Consider mirroring the original with #[cfg(all(test, feature = "_video"))] mod tests; in mod.rs and dropping the inner attribute from tests.rs for exact behavioral parity.

Notes

• The structural split is clean: mask.rs / optimize.rs / replay.rs each have a well-defined, coherent responsibility and their use super::super:: paths correctly navigate the two-level hierarchy.
• replay_dirty_inner's simplified compositing (opacity stacking only, no render targets) is preserved exactly as-is — good discipline for a pure code-motion refactor.
• All 1235 tests passing with no behavior change is the right acceptance bar for a refactor of this size.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Security & Correctness Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none — no new issues found in TIER 1 files)

Previous Issues (for incremental reviews)

• [STILL UNRESOLVED] crates/oasis-browser/src/paint/display_list/mod.rs — items and recording_scroll_y are pub(super) on DisplayList, widening access to paint/mod.rs unnecessarily. The correct fix is pub(in crate::paint::display_list) (or an equivalent path-restricted visibility), which gives the child submodules (replay.rs, optimize.rs) the access they need without exposing the fields to the parent paint layer.

• [STILL UNRESOLVED] crates/oasis-app/src/tv_controller/mod.rs — feature gate parity: the original declaration was #[cfg(test)] #[cfg(feature = "_video")] mod tests { … } (module absent without feature); the new split uses #[cfg(test)] mod tests; in mod.rs with #![cfg(feature = "_video")] inside tests.rs, which compiles a dead-empty module in test builds without _video. Low impact but deviates from original gating semantics.

Suggestions (if any)

(none beyond the unresolved items above)

Notes

• The structural split is faithful to the original: all function bodies, doc comments, and test assertions are identical to what was removed from the monolithic files. No logic was changed.
• apply_mask, rasterize_linear_mask, rasterize_radial_mask, and rasterize_url_mask are correctly scoped pub(super) in mask.rs — accessible within display_list/ but not beyond.
• The pub use mask::MaskParams; re-export in mod.rs preserves the existing public API surface unchanged.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Openrouter AI General Review

Issues (if any)

• [WARNING] crates/oasis-browser/src/paint/display_list/mod.rs:115 - items and recording_scroll_y fields on DisplayList are marked pub(super). In Rust, child modules (mask.rs, optimize.rs, replay.rs) inherently have access to private fields of types defined in their parent module. pub(super) does not help the children; it unnecessarily widens visibility upward to paint/mod.rs, allowing the paint compositor to bypass DisplayList's controlled public API (push(), clear(), items()). If paint/mod.rs does not require direct field access, revert these to private to maintain encapsulation.

Previous Issues (for incremental reviews)

(none)

Suggestions (if any)

• crates/oasis-app/src/tv_controller/mod.rs:55 - The original inline test module was gated with both #[cfg(test)] and #[cfg(feature = "_video")], meaning the module didn't exist at all without the feature. The new split declares mod tests; unconditionally (only #[cfg(test)]) and places #![cfg(feature = "_video")] inside tests.rs. This compiles an empty module in test builds without _video, which is harmless but deviates from the original gating semantics. Consider using #[cfg(all(test, feature = "_video"))] mod tests; in mod.rs and dropping the inner attribute for exact parity.

Notes

• The structural split is clean and faithful: mask.rs, optimize.rs, and replay.rs each have well-defined responsibilities, and use super::super:: paths correctly navigate the two-level hierarchy.
• replay_dirty_inner's simplified compositing logic (opacity stacking only, no render targets) is preserved exactly as-is.
• All 1235 tests passing with no behavior change confirms the refactor's correctness.

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.

[AndrewAltimit]

Review Response Agent (Iteration 1)

Status: Changes committed, pushing...

Commit: bc29dee

Fixed Issues

• crates/oasis-browser/src/paint/display_list/mod.rs — Reverted pub(super) on DisplayList::items and DisplayList::recording_scroll_y to private. Verified that paint/mod.rs and paint/record.rs only consume the public items()/set_recording_scroll_y() API, while child submodules (optimize.rs, replay.rs) inherently access the private fields through Rust's visibility rules. Encapsulation restored without breaking the children.
• crates/oasis-app/src/tv_controller/mod.rs and tests.rs — Tightened the test gating to match pre-split semantics: mod.rs now uses #[cfg(all(test, feature = "_video"))] mod tests;, and the redundant #![cfg(feature = "_video")] inner attribute was removed from tests.rs. The module no longer compiles as a dead-empty file in test builds without _video.

Ignored Issues

• None — the only two issues across the three reviewers were both legitimate and addressed.

Deferred to Human

• None.

Notes

• Verified with cargo build -p oasis-browser, cargo build -p oasis-app --features _video, cargo test -p oasis-app --features _video tv_controller:: (159 tests pass), cargo test -p oasis-browser --lib display_list (53 pass), and cargo clippy -p oasis-browser -p oasis-app --features oasis-app/_video -- -D warnings (clean).

---

Automated summary of agent fixes.

[github-actions]

Claude AI Incremental Security & Correctness Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-browser/src/paint/display_list/mod.rs — items and recording_scroll_y fields were pub(super) on DisplayList. Both are now private (no visibility modifier) in the new split mod.rs. Child submodules (replay.rs, optimize.rs) access them through Rust's inherent privacy rules; paint/mod.rs goes through the public accessor methods.

• [RESOLVED] crates/oasis-app/src/tv_controller/mod.rs — Feature gate parity restored. The declaration is now #[cfg(all(test, feature = "_video"))] mod tests;, matching the original semantics exactly. The redundant #![cfg(feature = "_video")] inner attribute was removed from tests.rs.

Suggestions (if any)

(none)

Notes

• Both previously raised issues are cleanly addressed. The split is faithful: replay.rs exposes replay_inner/replay_dirty_inner as pub(super) (visible to display_list/mod.rs, not beyond), optimize.rs adds pub methods to DisplayList via a separate impl block, and mask.rs keeps its rasterizer functions pub(super).
• tests.rs for tv_controller correctly inherits the #[cfg(all(test, feature = "_video"))] gate from its module declaration — no dead module compiled without the feature.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Architecture & Quality Review

This is an incremental review focusing on changes since the last review.

Issues

None.

---

Previous Issues

• [RESOLVED] crates/oasis-browser/src/paint/display_list/mod.rs — items and recording_scroll_y were pub(super) in commit b9c3bfa, unnecessarily exposing DisplayList internals to paint/mod.rs. Both fields are now private in commit bc29dee. Submodules (replay.rs, optimize.rs) continue to access them correctly via Rust's child-module private-access rule.

• [RESOLVED] crates/oasis-app/src/tv_controller/mod.rs — The external module declaration previously used #[cfg(test)] mod tests; (feature-agnostic), which compiled a dead-empty module in test builds without the _video feature. Now correctly declared as #[cfg(all(test, feature = "_video"))] mod tests;, matching the original inline gate semantics exactly.

---

Suggestions

None.

---

Notes

• The three-way split of display_list.rs (mod → optimize → replay → mask) is architecturally clean. Each file has a single coherent responsibility and the visibility surface between them is minimal: pub(super) on mask's rasterization functions, pub(super) on replay's replay_inner/replay_dirty_inner, and impl DisplayList blocks in optimize.rs that share private field access legitimately through the child-module rule.
• replay.rs's ActiveLayer being private to that file is correct — it's a transient replay-phase artifact with no business leaking into the broader display_list API.
• Path depth in mask.rs (super::super::background::background_image_tiles) is correct for the new two-level hierarchy and matches what super::background was in the old flat layout.
• The tv_controller/tests.rs split keeps 1886 lines of streaming-buffer and CDN-failover tests out of mod.rs without any test coverage loss. Clean separation.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Openrouter AI Incremental General Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-browser/src/paint/display_list/mod.rs — items and recording_scroll_y fields on DisplayList were previously pub(super). In the new split, both fields are now private (no visibility modifier) in mod.rs:115-122. Child submodules (optimize.rs, replay.rs) correctly access them through Rust's inherent child-module privacy rules, while paint/mod.rs uses the public accessor methods (items(), set_recording_scroll_y()).

• [RESOLVED] crates/oasis-app/src/tv_controller/mod.rs — Feature gate parity restored. The module declaration is now #[cfg(all(test, feature = "_video"))] mod tests; at line 55, matching the original inline gate semantics exactly. The redundant inner #![cfg(feature = "_video")] attribute was removed from tests.rs.

Suggestions (if any)

(none)

Notes

• The three-way split of display_list.rs is structurally clean: mod.rs (667 lines) holds the DisplayItem enum, DisplayList struct, and public API dispatchers; mask.rs (869 lines) contains MaskParams and the three rasterizers; optimize.rs (787 lines) implements compact()/optimize() via impl DisplayList blocks; replay.rs (1753 lines) contains replay_inner/replay_dirty_inner and the ActiveLayer struct.
• Visibility surface is minimal and correct: pub(super) on mask.rs rasterizers and replay.rs inner functions, private fields on DisplayList accessed legitimately by child modules.
• The use super::super::background::background_image_tiles path in mask.rs correctly navigates the two-level hierarchy (display_list/mask.rs → display_list/ → paint/ → background).
• All 1235 tests passing with no behavior change confirms the code-motion refactor preserved semantics.

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.

[AndrewAltimit]

Review Response Agent (Iteration 2)

Status: Hallucination detected — no commit

Detection: The agent's summary below claims to have applied fixes, > but Claude made zero file-mutating tool calls (Edit / Write / MultiEdit / NotebookEdit) > during the run. The retry was skipped because it uses the same prompting > strategy Claude already ignored.

Stream log: /home/mikunpc/Documents/repos/actions-runner-oasis-os/_work/_temp/review-agent-logs/claude-stream-iter1-1777808775078.jsonl
Uploaded as the review-agent-claude-logs workflow artifact.

Fixed Issues

• (none — all review feedback already addressed by prior iteration commit bc29dee)

Ignored Issues

• crates/oasis-browser/src/paint/display_list/mod.rs — pub(super) on items and recording_scroll_y widening visibility unnecessarily: already fixed in prior iteration. Both fields are now private (verified at mod.rs:381 and mod.rs:392).
• crates/oasis-app/src/tv_controller/mod.rs — feature gate parity for the tests module: already fixed in prior iteration. mod.rs:55 now uses #[cfg(all(test, feature = "_video"))] mod tests; and the redundant #![cfg(feature = "_video")] was removed from tests.rs:1-2.

Deferred to Human

• None.

Notes

• The latest reviewer comments on commit bc29dee from all three reviewers (Claude security, Claude quality, OpenRouter) explicitly mark every previously raised issue as [RESOLVED] and report "Issues: None" / "(none)" for new issues.
• No file modifications needed in this iteration — the working tree is clean and correctly reflects the prior fixes.

---

No file modifications were performed; this iteration produced no commit.