Skip to content

chore: delete dead code (PR 1/3) — -29,458 lines - #167

Merged
EtienneLescot merged 6 commits into
release/1.8.0from
chore/ponytail-1-dead-code
Jul 26, 2026
Merged

chore: delete dead code (PR 1/3) — -29,458 lines#167
EtienneLescot merged 6 commits into
release/1.8.0from
chore/ponytail-1-dead-code

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

First of three staged PRs from an over-engineering review of release/1.8.0. This one is deletion only — every cut is code with no reachable caller, plus one real bug fix. The behaviour-changing refactors (LLM stack consolidation, the native inspector channel, SSOT collapses) are held for PR 2 and PR 3.

Verification

Gate Baseline (release/1.8.0) This branch
tsc --noEmit clean clean
vitest --run 109 files / 1338 tests 94 files / 1108 tests
cargo check -p compositor-view-napi clean clean

The test delta is exactly the test files that covered deleted code — no surviving test was removed or weakened.

Commits

refactor(exporter) — the web MP4 pipeline. ExportDialog has rendered MP4 through exportMultiNative since the D3D compositor landed, and its mp4 branch returns before reaching exportAxcutDocument. VideoExporter was reachable only from src/bench, whose every arm drove that same dead path. Removes VideoExporter, WgslFrameRenderer, audioEncoder, muxer, nativeFrameSink, planarChunkQueue, perfTimings, videoDecoder, asyncVideoFrameQueue, the RenderPlan/audioConcat layer that only fed it, and the renderer→ffmpeg IPC encoder (its own header says REFUTED; its only consumer was gated on a localStorage flag nothing sets). resolveCropAt moves to exporter/cropSchedule.ts, which GIF still needs. Also ~29 zero-importer modules across components/ui, video-editor and ai-edition.

chore (POCs)poc/ and poc-native/, both superseded by the poc-d3d compositor that ships and referenced by no build script. Plus poc-d3d/spikes (throwaway C++ probes with .bat builders hardwired to a local Visual Studio path), the S0–S8 spike reports, and the stt-eval bake-off harness whose verdict shipped as electron/native/whisper-stt (REPORT.md kept).

docsspecs/computer-use (20 files scripting RightPanelStack, Bottombar, TimelinePane, CropModal — none of which have a source file), seven root specs each headed "Roadmap status: done", two superseded STT plans, and a stale .worktrees/wt-9ce78f24/HANDOFF.md that was tracked despite .worktrees/ being in .gitignore. Dangling links in surviving docs are fixed.

chore(deps)gsap, @pixi/filter-drop-shadow, emoji-picker-react, fix-webm-duration (stale twin of @fix-webm-duration/fix), @langchain/langgraph, @langchain/langgraph-checkpoint, langsmith, and website/'s clsx — all verified zero-import. The four Cache caption assets CI blocks key on hashFiles('scripts/fetch-caption-model.mjs'); that script does not exist, so the key is constant and the path caches nothing.

fix(export) — see below.

The hardcoded path

poc-d3d/compositor-view-napi/src/lib.rs shipped fixture_dir() defaulting to an absolute path inside a personal git worktree:

C:\Users\camil\Documents\repos\openscreen\.claude\worktrees\prerelease-version-tag-ee96ae\poc-d3d\fixture

Two paths reached it. create_view fell back to it when given no screen_path, and the exportNative fallback rendered that fixture whenever the timeline was empty — reachable from ExportDialog on any release build, where the directory does not exist. create_view now requires screen_path; the fixture-export entry point is removed end to end (ExportTask, the napi binding, the IPC action and contract variant, the service method, the client). An empty timeline raises exportDialog.nothingToExport, added to all 13 locales. exportMulti — the real timeline path — is untouched.

Deliberately kept

Three docs the review flagged are kept: ai-edition-roadmap.md pins collision-analysis, openscreen-inventory and axcut-inventory as deep reference, and calls design/openscreen-editor.html the canonical UI target. Overriding that in-repo intent was not worth the lines.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 375912e7-dd34-48ec-87c8-91c3cd346f8d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/ponytail-1-dead-code

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.

ExportDialog has rendered MP4 through exportMultiNative since the D3D
compositor landed; the mp4 branch returns before ever reaching
exportAxcutDocument, so VideoExporter was reachable only from the bench.
GIF is the one format the document adapter still renders.

Removes VideoExporter, WgslFrameRenderer, audioEncoder, muxer,
nativeFrameSink, planarChunkQueue, perfTimings, videoDecoder,
asyncVideoFrameQueue, the RenderPlan/audioConcat layer that only fed
VideoExporter, and src/bench (every arm drove the web path). The
renderer->ffmpeg IPC encoder goes with them: its own header says REFUTED
and its only consumer was gated on a localStorage flag nothing sets.

resolveCropAt + CropScheduleEntry move to exporter/cropSchedule.ts, which
GIF still needs. Also drops ~29 modules with zero importers across
components/ui, video-editor and ai-edition.

tsc clean; 94 files / 1108 tests pass (baseline 109/1338 - the delta is
test files covering the deleted code).
poc/ (WebGPU) and poc-native/ (Rust+wgpu) were both replaced by poc-d3d,
which is the compositor that actually ships. Neither is referenced by any
build script; poc/ was reachable only from .claude/launch.json.

Also removes poc-d3d/spikes (throwaway C++ probes with .bat builders
hardwired to a local Visual Studio path), the S0-S8 spike reports and
HANDOFF.md for a decision already shipped, the stt-eval bake-off harness
whose verdict shipped as electron/native/whisper-stt (REPORT.md kept),
an unreferenced 428 KB sample mp4 and a duplicate rec-button.png.
Removes specs/computer-use (20 files scripting RightPanelStack, Bottombar,
TimelinePane and CropModal — none of which have a source file), seven root
specs each headed "Roadmap status: done", the two STT plans whose own
successor declares itself the replacement, an axcut-UX spec for a surface
that does not exist post-merge, and a divergence audit whose conclusion is
consumed. Dangling links in the surviving docs are fixed.

Also untracks .worktrees/wt-9ce78f24/HANDOFF.md: .worktrees/ is in
.gitignore, but this file predates the rule and pins a dead branch plus a
G:\ path from another machine.

Kept deliberately: collision-analysis, openscreen-inventory and
axcut-inventory, which ai-edition-roadmap.md pins as deep reference, and
design/openscreen-editor.html, which it calls the canonical UI target.
design/_ds_bundle.js (checked-in bundle) and _adherence.oxlintrc.json
(config for a linter absent from the repo) do go.
gsap, @pixi/filter-drop-shadow, emoji-picker-react and fix-webm-duration
have zero imports across src, electron, scripts and tests; the last is a
stale twin of @fix-webm-duration/fix, which has the real call sites.

@langchain/langgraph, @langchain/langgraph-checkpoint and langsmith have
no direct imports either — all three arrive transitively through
deepagents/langchain/@langchain/core, and the declared ranges sit below
what deepagents requires, so they pin nothing. website/ drops clsx.

The four "Cache caption assets" blocks in build.yml key on
hashFiles('scripts/fetch-caption-model.mjs'); neither that script nor
before-pack.cjs exists, so the key degenerates to a constant and the path
caches nothing. bench:export goes with src/bench.
fixture_dir() defaulted to an absolute path inside a personal git
worktree (C:\Users\camil\...\worktrees\prerelease-version-tag-ee96ae).
Two paths reached it: create_view fell back to it when no screen_path was
given, and the exportNative fallback rendered the fixture whenever the
timeline was empty — reachable from ExportDialog on any release build,
where that directory does not exist.

create_view now requires screen_path and errors without it. The
fixture-export entry point goes entirely: ExportTask, the napi `export`
binding, the compositor "export" IPC action and contract variant, the
service method and the exportNative client. An empty timeline now raises
exportDialog.nothingToExport (added to all 13 locales) instead of
silently exporting someone else's fixture.

exportMulti — the real timeline path — is untouched.
cargo check clean; tsc clean; 94 files / 1108 tests pass.
The docs reorg on release/1.8.0 landed after this branch forked, so it
documented modules this branch deletes. `docs:check` caught two dead links;
reading around them showed the prose was describing the removed renderer-side
pipeline, not the native one that ships.

- export-pipeline.md: the `RenderPlan` section documented a type that only
  ever fed `VideoExporter` — on release/1.8.0 `renderPlan.ts` was imported by
  `documentExporter`, its own test, and `videoExporter` alone, while
  `ExportDialog` already called `exportMultiNative(clips, path, sceneJson)`
  direct. The diagram claimed it fed the addon; it never did. Points at
  `buildSceneDescription` instead, which preview.md already documents.
- Drops the per-segment cursor section: `cursorSamplesForAsset` and
  `segment.cursorSamples` went with the web pipeline, and the addon's export
  `ClipInput` carries no cursor telemetry (only `createView` takes a
  cursorPath), so the section described capability the native path never had.
- Output sizing said "largest clip" via `pickReferenceDimensions`;
  `tierOutputDims` feeds the crop-aware SMALLEST clip to
  `calculateMp4ExportSettings`. Codec crosses as a plain `ExportVideoCodec`
  string now, not a WebCodecs encoder string.
- preview.md: removes the "Path B" section for `PreviewCompositor.tsx`, which
  that same section already called dead code today.
@EtienneLescot
EtienneLescot force-pushed the chore/ponytail-1-dead-code branch from 371925e to 8c9b832 Compare July 26, 2026 18:23
@EtienneLescot
EtienneLescot merged commit 6649646 into release/1.8.0 Jul 26, 2026
6 checks passed
@EtienneLescot
EtienneLescot deleted the chore/ponytail-1-dead-code branch July 26, 2026 19:00
EtienneLescot added a commit that referenced this pull request Jul 27, 2026
…nto installers

Two ways this release could ship broken, and one obligation it was missing.

CI never ran on this branch. ci.yml triggered on [main, feat/ai-edition]
only, so PRs #167, #168 and #169 — 30k+ lines of deletion and refactor —
merged into release/1.8.0 with lint, typecheck, tests, build and the
PR-title check all skipped, and CodeRabbit reporting "reviews are
disabled for this base branch". Added release/** to both triggers.

The installers had no speech-to-text. electron/native/bin/ is gitignored
and build-whisper-stt.yml publishes the binaries as artifacts of its own
run; nothing carried them into build.yml. The model is downloaded at
runtime, the binary is not, so resolveBinaryPath() would find nothing and
transcription and captions would fail with a "build it via
scripts/build-whisper-stt.sh" message shown to end users.
scripts/stage-whisper-stt.sh now fetches them in all four build legs and
fails the build if they are missing — a red build beats a silent one.
Artifact retention 30 -> 90 days, since a release build now depends on it.

Added THIRD-PARTY-NOTICES.md and shipped it (with LICENSE) into
resources/. We redistribute FFmpeg's LGPL shared libraries and
whisper.cpp; fetch-ffmpeg.mjs went to real lengths to keep the build LGPL
rather than GPL, but nothing carried the attribution or the source offer
into the installer.

Also stops shipping the static ffmpeg.exe on Windows
("!win32-*/ffmpeg.exe"): the compositor addon dlopens the av*.dll set, but
no runtime code spawns the exe — it is there for bench-export.mjs. It was
adding a large binary and an LGPL obligation to every installer for
nothing. Its licence-gate comment also still pointed at
electron/media/ffmpegCapabilities.ts, deleted with the web export path.

tsconfig.test.json makes the untypechecked half of the repo visible:
tsconfig.json includes only src + electron and excludes **/*.test.ts, so
no test file has ever been typechecked. It runs as an advisory CI job —
82 pre-existing errors, so gating on it today would just be red.
EtienneLescot added a commit that referenced this pull request Aug 1, 2026
…nto installers

Two ways this release could ship broken, and one obligation it was missing.

CI never ran on this branch. ci.yml triggered on [main, feat/ai-edition]
only, so PRs #167, #168 and #169 — 30k+ lines of deletion and refactor —
merged into release/1.8.0 with lint, typecheck, tests, build and the
PR-title check all skipped, and CodeRabbit reporting "reviews are
disabled for this base branch". Added release/** to both triggers.

The installers had no speech-to-text. electron/native/bin/ is gitignored
and build-whisper-stt.yml publishes the binaries as artifacts of its own
run; nothing carried them into build.yml. The model is downloaded at
runtime, the binary is not, so resolveBinaryPath() would find nothing and
transcription and captions would fail with a "build it via
scripts/build-whisper-stt.sh" message shown to end users.
scripts/stage-whisper-stt.sh now fetches them in all four build legs and
fails the build if they are missing — a red build beats a silent one.
Artifact retention 30 -> 90 days, since a release build now depends on it.

Added THIRD-PARTY-NOTICES.md and shipped it (with LICENSE) into
resources/. We redistribute FFmpeg's LGPL shared libraries and
whisper.cpp; fetch-ffmpeg.mjs went to real lengths to keep the build LGPL
rather than GPL, but nothing carried the attribution or the source offer
into the installer.

Also stops shipping the static ffmpeg.exe on Windows
("!win32-*/ffmpeg.exe"): the compositor addon dlopens the av*.dll set, but
no runtime code spawns the exe — it is there for bench-export.mjs. It was
adding a large binary and an LGPL obligation to every installer for
nothing. Its licence-gate comment also still pointed at
electron/media/ffmpegCapabilities.ts, deleted with the web export path.

tsconfig.test.json makes the untypechecked half of the repo visible:
tsconfig.json includes only src + electron and excludes **/*.test.ts, so
no test file has ever been typechecked. It runs as an advisory CI job —
82 pre-existing errors, so gating on it today would just be red.
EtienneLescot added a commit that referenced this pull request Aug 3, 2026
The push trigger listed main alone, which is the same gap ci.yml carries a
comment about after PRs #167-#169 merged into a release branch with every job
skipped. Pull requests were already covered everywhere — that trigger has no
branch filter — but a rebase force-push onto release/** is not a pull request,
and a release branch is the last place to let a stale hash through unnoticed.
EtienneLescot added a commit that referenced this pull request Aug 3, 2026
The push trigger listed main alone, which is the same gap ci.yml carries a
comment about after PRs #167-#169 merged into a release branch with every job
skipped. Pull requests were already covered everywhere — that trigger has no
branch filter — but a rebase force-push onto release/** is not a pull request,
and a release branch is the last place to let a stale hash through unnoticed.
EtienneLescot added a commit that referenced this pull request Aug 3, 2026
The push trigger listed main alone, which is the same gap ci.yml carries a
comment about after PRs #167-#169 merged into a release branch with every job
skipped. Pull requests were already covered everywhere — that trigger has no
branch filter — but a rebase force-push onto release/** is not a pull request,
and a release branch is the last place to let a stale hash through unnoticed.
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.

1 participant