diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml deleted file mode 100644 index f42672d..0000000 --- a/.github/workflows/ci.yml +++ /dev/null @@ -1,32 +0,0 @@ -name: CI - -on: - push: - pull_request: - -jobs: - test: - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v4 - - - uses: actions/setup-python@v5 - with: - python-version: "3.11" - - - uses: actions/setup-node@v4 - with: - node-version: "22" - - - name: Install Python dependencies - run: python -m pip install -e ".[dev]" - - - name: Check Python syntax - run: python -m py_compile stream_server.py ascii_video_player2.py - - - name: Check JavaScript syntax - run: node --check app.js - - - name: Run tests - run: python -m pytest -q diff --git a/plans/001-establish-verification-baseline.md b/plans/001-establish-verification-baseline.md deleted file mode 100644 index c75c82f..0000000 --- a/plans/001-establish-verification-baseline.md +++ /dev/null @@ -1,294 +0,0 @@ -# Plan 001: Establish Reproducible Verification Baseline - -> **Executor instructions**: Follow this plan step by step. Run every -> verification command and confirm the expected result before moving to the -> next step. If anything in the "STOP conditions" section occurs, stop and -> report. When done, update the status row for this plan in `plans/README.md` -> unless a reviewer told you they maintain the index. -> -> **Drift check (run first)**: -> `rtk git diff --stat 312d5d6..HEAD -- README.md stream_server.py ascii_video_player2.py app.js .gitignore` -> If any in-scope file changed since this plan was written, compare the -> "Current state" excerpts against the live code before proceeding; on a -> mismatch, treat it as a STOP condition. - -## Status - -- **Priority**: P1 -- **Effort**: M -- **Risk**: LOW -- **Depends on**: none -- **Category**: tests, dx -- **Planned at**: commit `312d5d6`, 2026-06-12 - -## Why This Matters - -ASCILINE currently has no tracked dependency manifest, test suite, or CI entry -point. The README gives an install command, but future agents cannot run one -stable command to prove the code still imports, the JavaScript still parses, or -core queue helpers still behave. This plan creates the smallest useful -verification baseline so later fixes can be made with confidence. - -## Current State - -- `README.md` documents manual installation only: - -````markdown -README.md:45 -### 2. Install dependencies -```bash -pip install fastapi uvicorn opencv-python numpy websockets -``` -```` - -- `stream_server.py` imports runtime dependencies directly and documents a - shorter dependency list than README: - -```python -stream_server.py:13 -import asyncio -import subprocess -import json -import numpy as np -import cv2 -from fastapi import FastAPI, WebSocket, WebSocketDisconnect -from fastapi.responses import HTMLResponse, StreamingResponse -from fastapi.staticfiles import StaticFiles -import uvicorn -``` - -```python -stream_server.py:5 -Dependencies: pip install fastapi uvicorn websockets -``` - -- `stream_server.py` has pure helpers worth testing before touching playback - internals: - -```python -stream_server.py:42 -def calc_auto_rows(cols: int, vid_w: int, vid_h: int, pixel_mode: bool) -> int: - ratio = vid_w / max(vid_h, 1) - if pixel_mode: - return max(1, round(cols / ratio)) - else: - return max(1, round(cols / ratio / 2)) -``` - -```python -stream_server.py:63 -def resolve_video_path(video: str) -> str: - candidates = [ - video, - os.path.join(BASE_DIR, video), - os.path.join(BASE_DIR, "videos", os.path.basename(video)), - ] -``` - -- Existing local verification run during audit: - - `rtk python3 -c "from pathlib import Path; [compile(Path(f).read_text(encoding='utf-8'), f, 'exec') for f in ['stream_server.py','ascii_video_player2.py']]; print('python syntax ok')"` exited 0. - - `rtk node --check app.js` exited 0. - - `rtk find . -maxdepth 2 -type f -name '*test*'` found no tests. - - No tracked `requirements.txt`, `pyproject.toml`, test config, or CI files exist. - -Repo conventions to preserve: - -- Small flat repo, not a package directory tree. -- Python files use straightforward functions/classes and argparse entry points. -- Browser client is plain JavaScript in `app.js`; do not introduce a bundler. -- Commands in this Codex environment should be prefixed with `rtk`; if `rtk` - is unavailable, run the raw command. - -## Commands You Will Need - -| Purpose | Command | Expected on success | -|---------|---------|---------------------| -| Check current Python syntax | `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` | exit 0 | -| Check current JS syntax | `rtk node --check app.js` | exit 0 | -| Install after manifest exists | `rtk python3 -m pip install -e ".[dev]"` | exit 0 | -| Test after tests exist | `rtk python3 -m pytest -q` | exit 0, all tests pass | -| no-mistakes gate | `rtk git push no-mistakes HEAD && rtk no-mistakes` | gate run opens/passes or reports scoped findings | - -## Scope - -**In scope**: - -- `pyproject.toml` (create) -- `tests/` (create) -- `.github/workflows/ci.yml` (create) -- `README.md` (only update install/verification instructions if needed) -- `plans/README.md` (status update only) - -**Out of scope**: - -- Do not change runtime behavior in `stream_server.py`, `ascii_video_player2.py`, - `app.js`, `index.html`, or `style.css`. -- Do not add a JavaScript build system. -- Do not pin exact dependency versions unless a test requires it. - -## Git Workflow - -- Branch: `codex/001-verification-baseline` -- Commit message style in this repo is mostly conventional commits, for example - `feat: smart cols resolution` and `fix: remove copied architecture notes...`. - Use `chore: add verification baseline`. -- Use the no-mistakes workflow from `plans/README.md` after local checks pass. -- Do not push to `origin` unless the operator explicitly asks. - -## Steps - -### Step 1: Add Python Project Metadata - -Create `pyproject.toml` for a flat-module project. Use setuptools with explicit -modules so editable installs work without moving files: - -```toml -[build-system] -requires = ["setuptools>=69", "wheel"] -build-backend = "setuptools.build_meta" - -[project] -name = "asciline" -version = "0.1.0" -description = "Real-time ASCII and pixel video streaming engine" -requires-python = ">=3.11" -dependencies = [ - "fastapi>=0.115,<1", - "uvicorn[standard]>=0.30,<1", - "opencv-python>=4.10,<5", - "numpy>=2,<3", - "websockets>=13,<16", -] - -[project.optional-dependencies] -dev = [ - "pytest>=8,<9", - "httpx>=0.27,<1", -] - -[tool.setuptools] -py-modules = ["stream_server", "ascii_video_player2"] - -[tool.pytest.ini_options] -testpaths = ["tests"] -``` - -**Verify**: `rtk python3 -m pip install -e ".[dev]"` -> exits 0. - -### Step 2: Add Baseline Python Tests - -Create `tests/test_stream_server_baseline.py`. - -Test these cases: - -- `calc_auto_rows(240, 1920, 1080, False)` returns `68` or the live exact value - from current code. If the exact rounding differs on the target Python version, - compute the expected value using the formula in the current state excerpt and - assert that. -- `calc_auto_rows(450, 1920, 1080, True)` returns a positive integer larger - than the ASCII-mode result for the same video. -- `load_folder(tmp_path, default_mode=3, default_vol=2)` includes only files - ending in `.mp4`, `.mkv`, `.avi`, `.mov`, or `.webm`. -- `build_queue(SimpleNamespace(...))` fills missing playlist fields with global - defaults. Use a temporary playlist file and monkeypatch - `stream_server.BASE_DIR` if needed. - -Keep tests focused on helper behavior; do not require real video decoding. - -**Verify**: `rtk python3 -m pytest -q` -> exits 0 and reports the new tests pass. - -### Step 3: Add Syntax Verification To Tests Or CI Script - -Add either: - -- a Python test file `tests/test_syntax_baseline.py` that invokes - `python -m py_compile stream_server.py ascii_video_player2.py` and - `node --check app.js` with `subprocess.run(..., check=True)`, or -- equivalent explicit CI steps in `.github/workflows/ci.yml`. - -Prefer both if cheap: CI steps are clearer in logs, and a Python test keeps -local `pytest` meaningful. - -**Verify**: - -- `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` -> exits 0. -- `rtk node --check app.js` -> exits 0. -- `rtk python3 -m pytest -q` -> exits 0. - -### Step 4: Add CI - -Create `.github/workflows/ci.yml`: - -- Trigger on `push` and `pull_request`. -- Use `actions/checkout`. -- Use `actions/setup-python` with Python 3.11. -- Use `actions/setup-node` with a current LTS node version only for - `node --check app.js`; do not introduce npm. -- Install with `python -m pip install -e ".[dev]"`. -- Run: - - `python -m py_compile stream_server.py ascii_video_player2.py` - - `node --check app.js` - - `python -m pytest -q` - -**Verify**: `rtk python3 -m pytest -q && rtk node --check app.js` -> exits 0. - -### Step 5: Update README Verification Notes - -Update the install section to prefer: - -```bash -python -m pip install -e ".[dev]" -python -m pytest -q -node --check app.js -``` - -Keep the simple manual `pip install ...` command only if you label it as a quick -runtime-only path. - -**Verify**: `rtk rg -n "pytest|pyproject|pip install -e" README.md` -> shows the -new instructions. - -### Step 6: Run no-mistakes Gate - -Use the workflow in `plans/README.md`. - -**Verify**: `rtk git push no-mistakes HEAD && rtk no-mistakes` -> the gate run -opens/passes or reports findings limited to this plan scope. - -## Test Plan - -- `tests/test_stream_server_baseline.py` covers helper behavior without real - video files. -- Optional `tests/test_syntax_baseline.py` covers Python and JS parseability. -- CI repeats the same commands. - -## Done Criteria - -- [ ] `pyproject.toml` exists and `rtk python3 -m pip install -e ".[dev]"` exits 0. -- [ ] `rtk python3 -m pytest -q` exits 0. -- [ ] `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` exits 0. -- [ ] `rtk node --check app.js` exits 0. -- [ ] `.github/workflows/ci.yml` exists and runs install, Python syntax, JS syntax, and pytest. -- [ ] README documents the verification path. -- [ ] `rtk git status --short` shows only in-scope files changed. -- [ ] no-mistakes gate has run. -- [ ] `plans/README.md` status row updated. - -## STOP Conditions - -Stop and report back if: - -- Editable install cannot work without reorganizing source files into a package - directory. -- Installing OpenCV fails in the target environment for reasons unrelated to - the repo. -- Any test requires checking in a real video file. -- CI requires secrets, external services, or anything beyond GitHub-hosted - runner capabilities. - -## Maintenance Notes - -Future plans should depend on this baseline and add regression tests beside the -helper or behavior they change. Reviewers should keep this baseline small: -avoid expanding it into broad linting/formatting until the runtime fixes have -landed. diff --git a/plans/002-validate-cli-and-playlist-inputs.md b/plans/002-validate-cli-and-playlist-inputs.md deleted file mode 100644 index d252c89..0000000 --- a/plans/002-validate-cli-and-playlist-inputs.md +++ /dev/null @@ -1,278 +0,0 @@ -# Plan 002: Validate CLI And Playlist Playback Inputs - -> **Executor instructions**: Follow this plan step by step. Run every -> verification command and confirm the expected result before moving to the -> next step. If anything in the "STOP conditions" section occurs, stop and -> report. When done, update the status row for this plan in `plans/README.md` -> unless a reviewer told you they maintain the index. -> -> **Drift check (run first)**: -> `rtk git diff --stat 312d5d6..HEAD -- stream_server.py tests plans/README.md` -> If any in-scope file changed since this plan was written, compare the -> "Current state" excerpts against the live code before proceeding; on a -> mismatch, treat it as a STOP condition. - -## Status - -- **Priority**: P1 -- **Effort**: M -- **Risk**: MED -- **Depends on**: `plans/001-establish-verification-baseline.md` -- **Category**: bug -- **Planned at**: commit `312d5d6`, 2026-06-12 - -## Why This Matters - -Playback dimensions and playlist values flow directly into OpenCV resizing, -NumPy buffer allocation, FFmpeg volume filters, and client protocol metadata. -Today, CLI `--cols`, `--rows`, and `--vol` are plain integers with no bounds, -and playlist entries can override `mode`, `pixel`, `vol`, `cols`, and `rows` -without schema validation. A typo or huge value can crash playback or allocate -very large buffers before the user sees a helpful error. - -## Current State - -- Playlist values are accepted and defaulted without validating type/range: - -```python -stream_server.py:114 -if args.playlist: - print(f"[PLAYLIST] Loading: {args.playlist}") - items = load_playlist(args.playlist) - # Fill missing fields with global defaults - for item in items: - item.setdefault("mode", args.mode) - item.setdefault("vol", args.vol) - item.setdefault("pixel", args.pixel) - - is_pixel = item.get("pixel", False) - default_cols = args.cols if args.cols is not None else (450 if is_pixel else 200) - item.setdefault("cols", default_cols) - item.setdefault("rows", args.rows) -``` - -- CLI dimensions and volume are unbounded: - -```python -stream_server.py:522 -render.add_argument("--cols", type=int, default=None, help="Grid columns (default: 200 for text, 450 for pixel)") -render.add_argument("--rows", type=int, default=0, help="Grid rows (default: auto from video aspect ratio)") -``` - -```python -stream_server.py:527 -playback.add_argument( - "--vol", - type=int, default=1, - help="Volume 0-5 (0=muted, 1=normal, 5=double)" -) -``` - -- Playback allocates based on unchecked `rows * cols`: - -```python -stream_server.py:306 -frame_buf = np.empty((rows, cols, 4), dtype=np.uint8) if render_mode > 1 else None -``` - -```python -stream_server.py:313 -if pixel_mode: - pixel_send_buf = bytearray(4 + rows * cols * 3) -elif render_mode > 1: - ascii_send_buf = bytearray(4 + rows * cols * 4) -``` - -Repo conventions to preserve: - -- Keep `build_queue(args)` as the central queue construction path. -- Keep user-facing errors as concise `[ERROR] ...` CLI output plus exit code 1. -- Do not introduce Pydantic for this small local CLI unless it remains clearly - simpler than local helper functions. - -## Commands You Will Need - -| Purpose | Command | Expected on success | -|---------|---------|---------------------| -| Install dev deps | `rtk python3 -m pip install -e ".[dev]"` | exit 0 | -| Python syntax | `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` | exit 0 | -| JS syntax | `rtk node --check app.js` | exit 0 | -| Tests | `rtk python3 -m pytest -q` | exit 0, all tests pass | -| no-mistakes gate | `rtk git push no-mistakes HEAD && rtk no-mistakes` | gate run opens/passes or reports scoped findings | - -## Scope - -**In scope**: - -- `stream_server.py` -- `tests/test_stream_server_validation.py` (create or extend tests from plan 001) -- `README.md` only if validation limits must be documented -- `plans/README.md` status update only - -**Out of scope**: - -- Do not rewrite the WebSocket frame protocol. -- Do not change rendering algorithms in `ascii_video_player2.py`. -- Do not change the static root / host binding finding; the operator deferred it. -- Do not remove playlist, folder, or single-video modes. - -## Git Workflow - -- Branch: `codex/002-validate-playback-inputs` -- Commit message: `fix: validate playback inputs` -- Use the no-mistakes workflow from `plans/README.md` after local checks pass. -- Do not push to `origin` unless the operator explicitly asks. - -## Steps - -### Step 1: Add Validation Constants And Helpers - -In `stream_server.py`, near the top-level helper functions, add named constants -and a small validation layer. Suggested starting values: - -```python -MIN_COLS = 1 -MAX_COLS = 1200 -MIN_ROWS = 0 -MAX_ROWS = 800 -MAX_CELLS = 500_000 -MIN_VOL = 0 -MAX_VOL = 5 -VALID_MODES = {1, 2, 3, 4, 5} -``` - -Add helpers with explicit names, for example: - -- `validate_int_range(name: str, value: object, min_value: int, max_value: int) -> int` -- `validate_bool(name: str, value: object) -> bool` -- `validate_queue_entry(entry: dict, index: int) -> dict` -- `validate_dimensions(cols: int, rows: int) -> None` - -Rules: - -- `mode` must be an integer in `1..5`. -- `pixel` must be boolean after defaults are applied. -- `vol` must be integer in `0..5`. -- `cols` must be integer in `1..1200`. -- `rows` must be integer in `0..800`; `0` means auto. -- If `pixel` is true, `mode` must be `2..5`. -- If `rows > 0`, `cols * rows` must be `<= MAX_CELLS`. -- Playlist `video` must exist and be a non-empty string before resolving. - -Raise `ValueError` with messages that include the playlist entry index when -applicable, for example `playlist entry 2: vol must be between 0 and 5`. - -**Verify**: `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` -> exits 0. - -### Step 2: Use Validation In Queue Construction - -Update `load_playlist` and/or `build_queue` so every returned queue item has -validated `video`, `mode`, `vol`, `pixel`, `cols`, and `rows` fields. - -Important details: - -- Preserve default behavior: text mode defaults to `cols=200`; pixel mode - defaults to `cols=450`; `rows=0` means auto. -- Validate single-video and folder-generated entries too, not just playlists. -- Keep path resolution behavior unchanged after the `video` field passes the - non-empty string check. -- Wrap `queue = build_queue(args)` in the `__main__` block with a `try/except - ValueError` that prints `[ERROR] {message}` and exits 1. - -**Verify**: - -- `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` -> exits 0. -- `rtk python3 stream_server.py --help >/tmp/asciline-help.txt` -> exits 0 and - does not start the server. - -### Step 3: Validate Auto-Calculated Rows Before Allocation - -In `websocket_endpoint`, after auto rows are calculated or explicit rows are -selected, call `validate_dimensions(cols, rows)` before `VideoDecoder(...)` and -before any NumPy/bytearray allocation. - -If validation fails at this stage: - -- Send a WebSocket text message starting with `Error:`. -- Advance to the next queue item using the same existing queue-advance behavior - used for missing files. -- Do not crash the server task. - -**Verify**: `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` -> exits 0. - -### Step 4: Add Regression Tests - -Create `tests/test_stream_server_validation.py` or extend an existing test file. -Cover: - -- valid single-video args produce one normalized queue entry; -- `vol=-1` and `vol=6` are rejected; -- `cols=0`, `cols=-1`, and `cols > MAX_COLS` are rejected; -- `rows=-1` and `rows > MAX_ROWS` are rejected; -- playlist entry missing `video` is rejected with entry index in the message; -- playlist entry with `"pixel": true, "mode": 1` is rejected; -- explicit `cols * rows > MAX_CELLS` is rejected; -- `rows=0` is accepted so auto-scaling still works. - -Use `types.SimpleNamespace` to construct `args` objects instead of invoking the -server. Use temporary playlist JSON files with `tmp_path`; do not require real -video files. - -**Verify**: `rtk python3 -m pytest -q` -> exits 0 and includes the new tests. - -### Step 5: Update README If Limits Are User-Facing - -If you added hard maximums, document them near the resolution guidance so users -know why extreme values are rejected. - -Keep wording practical: - -```markdown -ASCILINE rejects extreme grid sizes before playback to avoid accidental memory -spikes. Keep `--cols` at or below 1200 and explicit `--rows` at or below 800. -``` - -**Verify**: `rtk rg -n "1200|800|memory" README.md` -> shows the new note, if -README was updated. - -### Step 6: Run no-mistakes Gate - -Use the workflow in `plans/README.md`. - -**Verify**: `rtk git push no-mistakes HEAD && rtk no-mistakes` -> the gate run -opens/passes or reports findings limited to this plan scope. - -## Test Plan - -- Python unit tests for queue normalization and validation edge cases. -- Existing syntax checks remain green. -- No real video files or FFmpeg process required. - -## Done Criteria - -- [ ] Invalid CLI/playlist `mode`, `pixel`, `vol`, `cols`, and `rows` fail with clear errors. -- [ ] `--pixel` with mode 1 is rejected for playlist entries as well as global CLI args. -- [ ] Auto-calculated dimensions are checked before frame buffer allocation. -- [ ] `rtk python3 -m pytest -q` exits 0. -- [ ] `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` exits 0. -- [ ] `rtk node --check app.js` exits 0. -- [ ] `rtk git status --short` shows only in-scope files changed. -- [ ] no-mistakes gate has run. -- [ ] `plans/README.md` status row updated. - -## STOP Conditions - -Stop and report back if: - -- The desired validation requires changing the WebSocket protocol. -- A test cannot be written without checking in or downloading a video file. -- Existing plan 001 verification baseline is missing. -- Validation changes require touching `ascii_video_player2.py` beyond syntax or - import compatibility. - -## Maintenance Notes - -If future features add higher-quality pixel modes, revisit `MAX_COLS`, -`MAX_ROWS`, and `MAX_CELLS` together. Reviewers should scrutinize error -messages and default preservation: valid existing README commands should still -build the same queue. diff --git a/plans/003-make-audio-selection-session-scoped.md b/plans/003-make-audio-selection-session-scoped.md deleted file mode 100644 index 08166f6..0000000 --- a/plans/003-make-audio-selection-session-scoped.md +++ /dev/null @@ -1,251 +0,0 @@ -# Plan 003: Make Audio Selection Session-Scoped - -> **Executor instructions**: Follow this plan step by step. Run every -> verification command and confirm the expected result before moving to the -> next step. If anything in the "STOP conditions" section occurs, stop and -> report. When done, update the status row for this plan in `plans/README.md` -> unless a reviewer told you they maintain the index. -> -> **Drift check (run first)**: -> `rtk git diff --stat 312d5d6..HEAD -- stream_server.py app.js tests plans/README.md` -> If any in-scope file changed since this plan was written, compare the -> "Current state" excerpts against the live code before proceeding; on a -> mismatch, treat it as a STOP condition. - -## Status - -- **Priority**: P2 -- **Effort**: M -- **Risk**: MED -- **Depends on**: `plans/001-establish-verification-baseline.md` -- **Category**: bug -- **Planned at**: commit `312d5d6`, 2026-06-12 - -## Why This Matters - -The WebSocket endpoint has a local `queue_index` per connected browser, but the -audio endpoint reads a global `app.state.current_index`. With two browser -sessions, one client's video loop can update the global index right before the -other client requests `/audio`, so the second client can receive the wrong -track. Audio selection should be derived from the client/session's queue item, -not from mutable global playback state. - -## Current State - -- `/audio` reads a global index: - -```python -stream_server.py:156 -@app.get("/audio") -async def audio_stream(): - queue = getattr(app.state, "queue", []) - idx = getattr(app.state, "current_index", 0) - entry = queue[idx] if queue else {} -``` - -- The WebSocket endpoint owns a local `queue_index`, but writes it to global - state before INIT: - -```python -stream_server.py:235 -queue_index = 0 # local index; advances through the queue -``` - -```python -stream_server.py:246 -# IMPORTANT: Update current_index BEFORE sending INIT so that -# when the client reloads /audio in response to INIT, the endpoint -# already serves the correct video's audio. -app.state.current_index = queue_index -``` - -- The client always requests global `/audio` after INIT: - -```javascript -app.js:182 -if (audioEl) { - audioEl.pause(); - audioEl.src = '/audio?' + Date.now(); - audioEl.volume = volumeSlider ? volumeSlider.value : 1.0; - audioEl.load(); - audioEl.play().catch(() => {}); -``` - -- INIT currently contains six fields: - -```python -stream_server.py:302 -await websocket.send_text(f"INIT:{effective_fps}:{render_mode}:{cols}:{rows}:{int(pixel_mode)}") -``` - -Repo conventions to preserve: - -- Keep the compact colon-delimited `INIT` handshake; do not introduce a JSON - protocol unless absolutely necessary. -- Keep `/audio` no-auth and local-app simple. -- Keep server-side volume behavior: `vol <= 0` returns 204 without launching - FFmpeg. - -## Commands You Will Need - -| Purpose | Command | Expected on success | -|---------|---------|---------------------| -| Install dev deps | `rtk python3 -m pip install -e ".[dev]"` | exit 0 | -| Python syntax | `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` | exit 0 | -| JS syntax | `rtk node --check app.js` | exit 0 | -| Tests | `rtk python3 -m pytest -q` | exit 0, all tests pass | -| no-mistakes gate | `rtk git push no-mistakes HEAD && rtk no-mistakes` | gate run opens/passes or reports scoped findings | - -## Scope - -**In scope**: - -- `stream_server.py` -- `app.js` -- `tests/test_audio_selection.py` (create or extend tests from plan 001) -- `plans/README.md` status update only - -**Out of scope**: - -- Do not add authentication, session cookies, or CSRF protection. -- Do not change static file serving or host binding. -- Do not change frame binary format except adding one optional field to the INIT - text message. -- Do not modify video decoding internals. - -## Git Workflow - -- Branch: `codex/003-session-audio-selection` -- Commit message: `fix: make audio selection per client` -- Use the no-mistakes workflow from `plans/README.md` after local checks pass. -- Do not push to `origin` unless the operator explicitly asks. - -## Steps - -### Step 1: Add Indexed Audio Endpoint - -In `stream_server.py`, add a new route: - -```python -@app.get("/audio/{queue_index}") -async def audio_stream_for_index(queue_index: int): - ... -``` - -Move the existing audio implementation into a helper such as: - -```python -def get_queue_entry(queue_index: int) -> dict: - queue = getattr(app.state, "queue", []) - if queue_index < 0 or queue_index >= len(queue): - raise HTTPException(status_code=404, detail="Audio entry not found") - return queue[queue_index] -``` - -Important behavior: - -- `/audio/{queue_index}` must select from the path parameter, not - `app.state.current_index`. -- Invalid indices return 404. -- `vol <= 0` still returns 204 and never starts FFmpeg. -- For compatibility, you may keep `/audio` as a fallback route that uses - `app.state.current_index`, but new client code must not use it. - -**Verify**: `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` -> exits 0. - -### Step 2: Send Queue Index In INIT - -Update the INIT message to append the local `queue_index`: - -```python -await websocket.send_text( - f"INIT:{effective_fps}:{render_mode}:{cols}:{rows}:{int(pixel_mode)}:{queue_index}" -) -``` - -Remove the comment that says the global `current_index` is needed for `/audio`. -You may keep `app.state.current_index = queue_index` only for `/status` display -and backwards-compatible `/audio`, but it must no longer be required for normal -client playback. - -**Verify**: `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` -> exits 0. - -### Step 3: Update Client Audio URL - -In `app.js`, parse the optional seventh INIT field: - -```javascript -const audioIndex = p.length > 6 ? parseInt(p[6], 10) : 0; -``` - -Use it for audio: - -```javascript -audioEl.src = `/audio/${audioIndex}?` + Date.now(); -``` - -Keep compatibility with older INIT messages by defaulting to `0` if the field is -missing. - -**Verify**: `rtk node --check app.js` -> exits 0. - -### Step 4: Add Server Regression Tests - -Create `tests/test_audio_selection.py`. - -Use FastAPI `TestClient` after plan 001 has added `httpx`: - -- Set `stream_server.app.state.queue` to two fake entries: - - entry 0: `{"video": "missing-a.mp4", "mode": 1, "vol": 0, "pixel": False, "cols": 200, "rows": 0}` - - entry 1: `{"video": "missing-b.mp4", "mode": 1, "vol": 0, "pixel": False, "cols": 200, "rows": 0}` -- Set `app.state.current_index = 1`. -- `GET /audio/0` returns 204 because entry 0 is muted. This proves the route did - not use global `current_index`. -- `GET /audio/1` returns 204. -- `GET /audio/99` returns 404. - -Do not test unmuted audio by launching FFmpeg. - -**Verify**: `rtk python3 -m pytest -q` -> exits 0 and includes these tests. - -### Step 5: Run no-mistakes Gate - -Use the workflow in `plans/README.md`. - -**Verify**: `rtk git push no-mistakes HEAD && rtk no-mistakes` -> the gate run -opens/passes or reports findings limited to this plan scope. - -## Test Plan - -- FastAPI tests prove indexed audio does not depend on global state. -- `node --check app.js` proves the client remains parseable. -- Existing baseline tests remain green. - -## Done Criteria - -- [ ] Client requests `/audio/` after INIT. -- [ ] Normal client playback no longer depends on `app.state.current_index` for audio selection. -- [ ] Invalid audio indices return 404. -- [ ] Muted indexed audio returns 204 without FFmpeg. -- [ ] `rtk python3 -m pytest -q` exits 0. -- [ ] `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` exits 0. -- [ ] `rtk node --check app.js` exits 0. -- [ ] `rtk git status --short` shows only in-scope files changed. -- [ ] no-mistakes gate has run. -- [ ] `plans/README.md` status row updated. - -## STOP Conditions - -Stop and report back if: - -- The client has already been changed away from the colon-delimited INIT format. -- Tests require launching FFmpeg or reading a real media file. -- The fix appears to require adding authentication/session infrastructure. -- Plan 001 verification baseline is not present. - -## Maintenance Notes - -If future work adds seeking, playlists with duplicate entries, or per-client -volume, use an explicit playback/session identifier rather than returning to -global mutable state. Reviewers should focus on backwards compatibility and -whether `/audio/{queue_index}` is the only path used by current `app.js`. diff --git a/plans/004-make-browser-render-loop-idempotent.md b/plans/004-make-browser-render-loop-idempotent.md deleted file mode 100644 index 27c63a2..0000000 --- a/plans/004-make-browser-render-loop-idempotent.md +++ /dev/null @@ -1,240 +0,0 @@ -# Plan 004: Make Browser Render Loop Idempotent - -> **Executor instructions**: Follow this plan step by step. Run every -> verification command and confirm the expected result before moving to the -> next step. If anything in the "STOP conditions" section occurs, stop and -> report. When done, update the status row for this plan in `plans/README.md` -> unless a reviewer told you they maintain the index. -> -> **Drift check (run first)**: -> `rtk git diff --stat 312d5d6..HEAD -- app.js tests plans/README.md` -> If any in-scope file changed since this plan was written, compare the -> "Current state" excerpts against the live code before proceeding; on a -> mismatch, treat it as a STOP condition. - -## Status - -- **Priority**: P2 -- **Effort**: S -- **Risk**: LOW -- **Depends on**: `plans/001-establish-verification-baseline.md` -- **Category**: bug, perf -- **Planned at**: commit `312d5d6`, 2026-06-12 - -## Why This Matters - -The client starts rendering when audio is ready, but it also has a 500ms -fallback for muted or failed audio. If the fallback fires and the audio later -emits `playing`, `beginRendering()` can schedule a second `requestAnimationFrame` -loop. Multiple render loops compete for the same frame buffer, which can cause -jitter, extra CPU use, and hard-to-debug playback timing behavior. - -## Current State - -- `beginRendering()` schedules RAF unconditionally: - -```javascript -app.js:174 -const beginRendering = () => { - readyToRender = true; - streamStartTime = performance.now(); - lastRenderTime = performance.now(); - lastFpsUpdate = lastRenderTime; - requestAnimationFrame(renderFrame); -}; -``` - -- Both `playing` and the fallback can call it: - -```javascript -app.js:193 -audioEl.addEventListener('playing', beginRendering, { once: true }); -// Fallback: if audio fails to load (vol=0 / 204), start after 500ms -setTimeout(() => { - if (!readyToRender) beginRendering(); -}, 500); -``` - -- `renderFrame` also schedules the next frame unconditionally while playing: - -```javascript -app.js:248 -function renderFrame(now) { - if (state !== 'PLAYING' || !readyToRender) return; - requestAnimationFrame(renderFrame); -``` - -- Cleanup does not cancel any outstanding RAF id: - -```javascript -app.js:341 -function finishStream() { - state = 'IDLE'; - if (ws) { ws.onclose = null; ws.close(); ws = null; } - if (audioEl) { audioEl.pause(); audioEl.src = ''; } -``` - -Repo conventions to preserve: - -- Plain JavaScript; no bundler, framework, or npm dependency. -- Keep the audio ready gate behavior: wait for audio when possible, fallback for - muted/204 audio. - -## Commands You Will Need - -| Purpose | Command | Expected on success | -|---------|---------|---------------------| -| Install dev deps | `rtk python3 -m pip install -e ".[dev]"` | exit 0 | -| JS syntax | `rtk node --check app.js` | exit 0 | -| Full tests | `rtk python3 -m pytest -q` | exit 0, all tests pass | -| Python syntax | `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` | exit 0 | -| no-mistakes gate | `rtk git push no-mistakes HEAD && rtk no-mistakes` | gate run opens/passes or reports scoped findings | - -## Scope - -**In scope**: - -- `app.js` -- `tests/test_frontend_static.py` (create or extend) -- `plans/README.md` status update only - -**Out of scope**: - -- Do not redesign the player UI. -- Do not change the WebSocket protocol. -- Do not add Playwright, jsdom, npm, or a frontend build system. -- Do not modify server files except if a syntax/test command requires no - behavioral change; report first if that happens. - -## Git Workflow - -- Branch: `codex/004-idempotent-render-loop` -- Commit message: `fix: make render loop idempotent` -- Use the no-mistakes workflow from `plans/README.md` after local checks pass. -- Do not push to `origin` unless the operator explicitly asks. - -## Steps - -### Step 1: Track The Active RAF - -In the state section near the timing variables, add: - -```javascript -let renderLoopId = null; -``` - -**Verify**: `rtk node --check app.js` -> exits 0. - -### Step 2: Add A Single Render-Loop Starter - -Replace the inline INIT `beginRendering` logic with a helper function at module -scope, for example: - -```javascript -function beginRendering() { - if (renderLoopId !== null) return; - readyToRender = true; - streamStartTime = performance.now(); - lastRenderTime = performance.now(); - lastFpsUpdate = lastRenderTime; - renderLoopId = requestAnimationFrame(renderFrame); -} -``` - -Then in the INIT handler, call this shared helper from: - -- the immediate `audioEl.readyState >= 3` path; -- the `playing` event listener; -- the 500ms fallback; -- the no-audio-element branch. - -Keep the existing fallback guard `if (!readyToRender) beginRendering();`, but the -new helper must also guard on `renderLoopId !== null`. - -**Verify**: `rtk node --check app.js` -> exits 0. - -### Step 3: Keep RAF Id Updated And Clear It On Stop - -Update `renderFrame(now)` so it owns the active RAF id: - -```javascript -function renderFrame(now) { - renderLoopId = null; - if (state !== 'PLAYING' || !readyToRender) return; - renderLoopId = requestAnimationFrame(renderFrame); - ... -} -``` - -Update `finishStream()` to cancel any pending frame: - -```javascript -if (renderLoopId !== null) { - cancelAnimationFrame(renderLoopId); - renderLoopId = null; -} -``` - -Do this before clearing buffers and showing the overlay. - -**Verify**: `rtk node --check app.js` -> exits 0. - -### Step 4: Add A Lightweight Static Regression Test - -Create `tests/test_frontend_static.py`. Since this repo has no frontend test -runner and this plan must not add one, use a focused static guard: - -- read `app.js`; -- assert it contains `let renderLoopId = null`; -- assert `beginRendering` checks `renderLoopId !== null`; -- assert `renderFrame` assigns `renderLoopId = requestAnimationFrame(renderFrame)`; -- assert `finishStream` calls `cancelAnimationFrame(renderLoopId)`; -- run `node --check app.js` with `subprocess.run(..., check=True)`. - -This is not a full behavior test, but it preserves the no-dependency frontend -setup while catching regressions to the exact bug fixed here. - -**Verify**: `rtk python3 -m pytest -q` -> exits 0. - -### Step 5: Run no-mistakes Gate - -Use the workflow in `plans/README.md`. - -**Verify**: `rtk git push no-mistakes HEAD && rtk no-mistakes` -> the gate run -opens/passes or reports findings limited to this plan scope. - -## Test Plan - -- `node --check app.js` for JS parseability. -- `tests/test_frontend_static.py` for the render-loop guard and cleanup shape. -- Full pytest suite from plan 001. - -## Done Criteria - -- [ ] `beginRendering()` is idempotent. -- [ ] `renderFrame()` stores the active RAF id. -- [ ] `finishStream()` cancels and clears any active RAF id. -- [ ] No new frontend dependencies are added. -- [ ] `rtk node --check app.js` exits 0. -- [ ] `rtk python3 -m pytest -q` exits 0. -- [ ] `rtk python3 -m py_compile stream_server.py ascii_video_player2.py` exits 0. -- [ ] `rtk git status --short` shows only in-scope files changed. -- [ ] no-mistakes gate has run. -- [ ] `plans/README.md` status row updated. - -## STOP Conditions - -Stop and report back if: - -- The app has already gained a frontend test runner or module system; this plan - should be rewritten to use that instead of static tests. -- The render loop has been substantially refactored since the excerpts above. -- Fixing duplicate RAF loops requires changing server timing or the WebSocket - protocol. - -## Maintenance Notes - -If future client work adds pause/resume, seeking, or playlist controls, route -all render-loop starts through the same idempotent starter. Reviewers should -look for accidental extra `requestAnimationFrame(renderFrame)` calls outside the -helper. diff --git a/plans/README.md b/plans/README.md deleted file mode 100644 index 39142e4..0000000 --- a/plans/README.md +++ /dev/null @@ -1,60 +0,0 @@ -# Implementation Plans - -Generated by the improve skill on 2026-06-12. Execute in the order below unless -dependencies say otherwise. Each executor: read the assigned plan fully before -starting, honor its STOP conditions, run every verification command, push -through the no-mistakes gate, and update your row when done. - -## Execution Order & Status - -| Plan | Title | Priority | Effort | Depends on | Status | -|------|-------|----------|--------|------------|--------| -| 001 | Establish Reproducible Verification Baseline | P1 | M | - | DONE | -| 002 | Validate CLI And Playlist Playback Inputs | P1 | M | 001 | DONE | -| 003 | Make Audio Selection Session-Scoped | P2 | M | 001 | DONE | -| 004 | Make Browser Render Loop Idempotent | P2 | S | 001 | DONE | - -Status values: TODO | IN PROGRESS | DONE | BLOCKED (with one-line reason) | -REJECTED (with one-line rationale). - -## Dependency Notes - -- 001 must land first because the other plans rely on a dependency manifest and - a one-command test path. -- 002 can run before or after 003/004 once 001 is done. -- 003 and 004 both touch playback synchronization surfaces. If one has already - landed, the other executor must re-run drift checks and compare the app.js and - stream_server.py excerpts before editing. - -## no-mistakes Workflow For Executors - -The operator requested this gate for implementation work. Use it after local -verification and before asking for review. - -1. From repo root, run `rtk git remote get-url no-mistakes` to check whether the - gate remote already exists. -2. If the remote is missing, run `rtk no-mistakes init`. -3. Work on a `codex/NNN-short-slug` branch unless the operator assigned another - branch. -4. After local verification passes, run `rtk git push no-mistakes HEAD`. -5. Run `rtk no-mistakes` and review the active gate run. -6. Address gate findings inside the plan scope, or mark the plan BLOCKED if the - gate requires out-of-scope changes. -7. Do not push to `origin`, merge, or open a PR unless the operator explicitly - asks. - -If `rtk` is unavailable in a future executor environment, run the same commands -without the `rtk` prefix. - -## Findings Considered And Rejected Or Deferred - -- Static root exposure via `/static`: intentionally ignored for now by the - operator because this is a personal/self-hosted project. Reconsider only if - the server is exposed beyond trusted local devices or sensitive local files - are stored beside the app. -- License wording: real documentation/distribution issue, but lower leverage - than runtime correctness and verification. -- Resize selection-layer drift: real UI issue, but lower priority than playback - validation and audio/render synchronization. -- Pre-encoded frame cache, installable CLI package, and LLM-ready export path: - direction options, not selected for this plan batch.