diff --git a/.github/workflows/build-push-images.yml b/.github/workflows/build-push-images.yml index e20a2c513..ac7f577cf 100644 --- a/.github/workflows/build-push-images.yml +++ b/.github/workflows/build-push-images.yml @@ -50,7 +50,7 @@ jobs: SURGE_VERSION=$(curl -s https://api.github.com/repos/SurgeDM/Surge/releases/latest | jq -r .tag_name | sed 's/^v//') echo "Latest Surge version: $SURGE_VERSION" fi - echo "version=$SURGE_VERSION" >> $GITHUB_OUTPUT + echo "version=$SURGE_VERSION" >> "$GITHUB_OUTPUT" - name: Wait for Release Assets run: | @@ -68,7 +68,7 @@ jobs: exit 0 fi echo "Attempt $i/$MAX_RETRIES: Release not ready yet, waiting ${RETRY_INTERVAL}s..." - sleep $RETRY_INTERVAL + sleep "$RETRY_INTERVAL" done echo "Timeout waiting for release assets after $((MAX_RETRIES * RETRY_INTERVAL / 60)) minutes." @@ -85,7 +85,7 @@ jobs: elif [[ "${{ inputs.tag_as_latest }}" == "true" ]]; then ENABLE_LATEST=true fi - echo "enable=$ENABLE_LATEST" >> $GITHUB_OUTPUT + echo "enable=$ENABLE_LATEST" >> "$GITHUB_OUTPUT" - name: Set up QEMU uses: docker/setup-qemu-action@v3 @@ -125,18 +125,20 @@ jobs: - name: Summary run: | - echo "## Docker Image Built Successfully! ๐Ÿš€" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "**Surge Version:** ${{ steps.surge-version.outputs.version }}" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "**Images pushed:**" >> $GITHUB_STEP_SUMMARY - echo '```' >> $GITHUB_STEP_SUMMARY - echo "${{ steps.meta.outputs.tags }}" >> $GITHUB_STEP_SUMMARY - echo '```' >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "**Platforms:** linux/amd64, linux/arm64" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "### Pull the image:" >> $GITHUB_STEP_SUMMARY - echo '```bash' >> $GITHUB_STEP_SUMMARY - echo "docker pull ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.surge-version.outputs.version }}" >> $GITHUB_STEP_SUMMARY - echo '```' >> $GITHUB_STEP_SUMMARY + { + echo "## Docker Image Built Successfully! ๐Ÿš€" + echo "" + echo "**Surge Version:** ${{ steps.surge-version.outputs.version }}" + echo "" + echo "**Images pushed:**" + echo '```' + echo "${{ steps.meta.outputs.tags }}" + echo '```' + echo "" + echo "**Platforms:** linux/amd64, linux/arm64" + echo "" + echo "### Pull the image:" + echo '```bash' + echo "docker pull ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.surge-version.outputs.version }}" + echo '```' + } >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/core-build.yml b/.github/workflows/core-build.yml index 36628edc6..24218d8e5 100644 --- a/.github/workflows/core-build.yml +++ b/.github/workflows/core-build.yml @@ -12,6 +12,7 @@ on: - main permissions: + actions: read contents: write pull-requests: write @@ -24,6 +25,7 @@ jobs: runs-on: ubuntu-latest outputs: core: ${{ steps.filter.outputs.core }} + tui: ${{ steps.filter.outputs.tui }} steps: - uses: actions/checkout@v4 - uses: dorny/paths-filter@v3 @@ -33,6 +35,10 @@ jobs: core: - '!extension/**' - '**' + tui: + - 'internal/tui/**' + - 'go.mod' + - 'go.sum' test: name: Test and Check (${{ matrix.os }}) @@ -122,6 +128,157 @@ jobs: with: token: ${{ secrets.CODECOV_TOKEN }} + tui-perf: + name: TUI Performance Budget + needs: changes + if: needs.changes.outputs.tui == 'true' + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v4 + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: "1.25.0" + check-latest: false + - name: Enforce TUI frame budgets + env: + GOMAXPROCS: "2" + SURGE_PERF_BUDGET: "1" + run: | + set -o pipefail + go test ./internal/tui -run '^TestTUI.*RenderPerfBudget$' -count=3 -v | tee tui-perf.txt + - name: Report TUI performance values + if: always() + run: | + { + echo "### TUI performance budget" + echo + echo '```text' + if [ -f tui-perf.txt ]; then + cat tui-perf.txt + else + echo "No performance output was produced." + fi + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + - name: Record TUI performance metadata + if: always() + shell: bash + run: | + { + echo "sha=$GITHUB_SHA" + echo "run_id=$GITHUB_RUN_ID" + echo "run_number=$GITHUB_RUN_NUMBER" + echo "runner_os=$RUNNER_OS" + echo "runner_arch=$RUNNER_ARCH" + echo "gomaxprocs=${GOMAXPROCS:-2}" + date -u +"timestamp=%Y-%m-%dT%H:%M:%SZ" + } > tui-perf-metadata.txt + - name: Upload TUI performance history + if: always() + uses: actions/upload-artifact@v4 + with: + name: tui-perf-${{ github.run_number }} + path: | + tui-perf.txt + tui-perf-metadata.txt + if-no-files-found: ignore + retention-days: 90 + + tui-perf-regression: + name: TUI Performance Regression + needs: [changes, tui-perf] + if: always() && needs.changes.outputs.tui == 'true' + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v4 + - name: Download current performance report + uses: actions/download-artifact@v4 + with: + name: tui-perf-${{ github.run_number }} + path: current-perf + + - name: Compare with previous successful baseline + env: + GH_TOKEN: ${{ github.token }} + BASE_BRANCH: ${{ github.event.pull_request.base.ref || github.ref_name }} + shell: bash + run: | + set -euo pipefail + + successful_runs="$(gh api "repos/${GITHUB_REPOSITORY}/actions/workflows/core-build.yml/runs" \ + --method GET \ + --field "branch=${BASE_BRANCH}" \ + --field status=success \ + --field per_page=100 \ + --jq ".workflow_runs[] | select(.id != ${GITHUB_RUN_ID}) | [.id, .run_number] | @tsv" || true)" + + if [ -z "$successful_runs" ]; then + printf '%s\n' "No previous successful TUI performance baseline found for ${BASE_BRANCH}; comparison skipped." > perf-comparison.txt + cat perf-comparison.txt + exit 0 + fi + + artifact_id="" + while IFS=$'\t' read -r candidate_run_id candidate_run_number; do + [ -n "$candidate_run_id" ] || continue + candidate_artifact_id="$(gh api "repos/${GITHUB_REPOSITORY}/actions/runs/${candidate_run_id}/artifacts" \ + --jq ".artifacts[] | select(.expired == false and .name == \"tui-perf-${candidate_run_number}\") | .id" | head -n 1)" + if [ -n "$candidate_artifact_id" ]; then + artifact_id="$candidate_artifact_id" + break + fi + done <<< "$successful_runs" + + if [ -z "$artifact_id" ]; then + printf '%s\n' "Previous successful runs have no unexpired TUI performance artifact; comparison skipped." > perf-comparison.txt + cat perf-comparison.txt + exit 0 + fi + + mkdir -p previous-perf + gh api "repos/${GITHUB_REPOSITORY}/actions/artifacts/${artifact_id}/zip" > previous-perf.zip + unzip -q previous-perf.zip -d previous-perf + + CURRENT_REPORT="$(find current-perf -name tui-perf.txt -type f -print -quit)" + PREVIOUS_REPORT="$(find previous-perf -name tui-perf.txt -type f -print -quit)" + if [ -z "$CURRENT_REPORT" ] || [ -z "$PREVIOUS_REPORT" ]; then + printf '%s\n' "Performance report missing from current or previous artifact; comparison skipped." > perf-comparison.txt + cat perf-comparison.txt + exit 0 + fi + + python3 scripts/compare_tui_perf.py \ + --current "$CURRENT_REPORT" \ + --previous "$PREVIOUS_REPORT" \ + --output perf-comparison.txt + + - name: Report performance comparison + if: always() + run: | + { + echo "### TUI performance regression" + echo + echo '```text' + if [ -f perf-comparison.txt ]; then + cat perf-comparison.txt + else + echo "No comparison output was produced." + fi + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload performance comparison + if: always() + uses: actions/upload-artifact@v4 + with: + name: tui-perf-comparison-${{ github.run_number }} + path: perf-comparison.txt + if-no-files-found: ignore + retention-days: 90 + release: name: Release runs-on: ubuntu-latest diff --git a/.github/workflows/core-lint.yml b/.github/workflows/core-lint.yml index 20bf4020b..46180ad8a 100644 --- a/.github/workflows/core-lint.yml +++ b/.github/workflows/core-lint.yml @@ -30,3 +30,12 @@ jobs: run: go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest run - name: unicode lint run: go test ./internal/lint/... + - name: TUI performance comparison tests + run: python3 -m unittest discover -s scripts -p 'test_*.py' + - name: Install ShellCheck + run: sudo apt-get update && sudo apt-get install --no-install-recommends -y shellcheck + - name: ShellCheck scripts + shell: bash + run: find scripts -type f -name '*.sh' -print0 | xargs -0 -r shellcheck + - name: GitHub Actions lint + run: go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.7 diff --git a/.github/workflows/extension.yml b/.github/workflows/extension.yml index 3b1201e15..0b48734c7 100644 --- a/.github/workflows/extension.yml +++ b/.github/workflows/extension.yml @@ -49,7 +49,7 @@ jobs: run: | VERSION=${GITHUB_REF_NAME#ext-v} echo "Setting version to $VERSION" - npm version $VERSION --no-git-tag-version + npm version "$VERSION" --no-git-tag-version - name: Build and Package run: | npm run zip -- -b ${{ matrix.browser }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cce4b0a94..4b1a45364 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,49 +1,76 @@ # Contributing -Thanks for checking out Surge. We are very open to contributions and happy to review PRs. +Thanks for contributing to Surge. Start with the [Development Guide](docs/DEVELOPMENT.md) for prerequisites and the complete local workflow. -This is intentionally short. If you see something that can be better, open a PR. +## Quick start -## Quick Codebase Map +From the repository root: -- `cmd/`: CLI commands and startup wiring (`surge get`, `surge server`, etc.). -- `internal/core/`: service layer (`LocalDownloadService`) that orchestrates add/pause/resume/delete/list. -- `internal/download/`: high-level download flow (`RunDownload`) and worker-pool lifecycle. -- `internal/engine/`: low-level engine code. -- `internal/engine/probe.go`: probe logic (range support, metadata, mirror probing). -- `internal/engine/concurrent/`: concurrent HTTP downloader and worker/retry/failover logic. -- `internal/engine/single/`: single-connection HTTP downloader fallback. -- `internal/engine/state/`: Gob-backed file persistence for paused/history downloads. -- `internal/tui/`: terminal UI models, update loop, views. -- `internal/testutil/`: mock HTTP servers and test helpers. +```bash +go mod download +go test ./... +go test -race ./internal/... +``` + +The core Go workflow uses the dependencies pinned in `go.mod` and `go.sum`. The Python performance scripts require Python 3.8+ but only use the standard library, so no pip install is needed. Browser-extension work additionally requires Node.js 22+ and `npm ci` in `extension/`; ShellCheck and actionlint are optional locally and are enforced by CI. -If you are looking for networking behavior, start here: +For local configuration isolation while running the TUI or server: + +```bash +XDG_CONFIG_HOME="$(mktemp -d)" XDG_CACHE_HOME="$(mktemp -d)" go run . +``` -1. `internal/engine/probe.go` -2. `internal/engine/concurrent/` -3. `internal/engine/single/` +## Codebase map -## Run Tests +- `cmd/`: Cobra commands, startup wiring, and CLI/server entry points. +- `internal/orchestrator/`: lifecycle management, enqueueing, pause/resume, and event coordination. +- `internal/scheduler/`: queued/active download scheduling, rate-limit pools, and shutdown behavior. +- `internal/strategy/concurrent/`: ranged, mirrored, retried, hedged, and health-monitored downloads. +- `internal/strategy/single/`: single-connection fallback downloads and throttled streaming. +- `internal/probe/`: server capability and metadata probing. +- `internal/transport/`: network pools, host penalties, and byte rate limiters. +- `internal/progress/`: live download state, chunk maps, and progress aggregation. +- `internal/store/`: persisted download and resume state. +- `internal/tui/`: Bubble Tea model/update loop, dashboard panes, modal components, and render tests. +- `internal/testutil/`: mock servers, temporary directories, and test helpers. +- `extension/`: WXT/Solid browser extension source and tests. -From repo root: +## Focused checks + +Use focused package tests while iterating: ```bash -go test ./... +go test ./internal/tui ./internal/tui/components -count=1 +go test ./internal/strategy/concurrent -count=1 +go test ./internal/strategy/single -count=1 +go test ./internal/transport -run 'RateLimiter|HostRateLimiter' -count=1 +``` + +For TUI rendering changes, also run the opt-in budgets and rendering benchmarks: + +```bash +GOMAXPROCS=2 SURGE_PERF_BUDGET=1 \ + go test ./internal/tui -run '^TestTUI.*RenderPerfBudget$' -count=3 -v +go test ./internal/tui -run '^$' \ + -bench '^BenchmarkCPU_(FullView_Old|FullView_New|DashboardPanes_Cached)$' \ + -benchmem -count=3 -benchtime=500ms ``` -Useful focused runs: +For workflow or shell changes: ```bash -go test ./internal/engine/concurrent -run TestConcurrentDownloader_SwitchOn429 -count=1 -go test ./internal/download -run TestIntegration_PauseResume -count=1 -go test ./internal/tui -count=1 +find scripts -type f -name '*.sh' -print0 | xargs -0 -r shellcheck +go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.7 ``` -## PR Expectations +## Pull requests -- Keep PRs focused and readable. -- Add or update tests for behavior changes. -- Run `go test ./...` before opening/updating the PR. -- If behavior or CLI usage changes, update docs (`README.md` or `docs/`). +- Keep each PR focused and explain the user-visible motivation. +- Add regression tests for behavior changes and golden tests for terminal rendering changes. +- Preserve existing platform behavior; avoid assuming Linux unless the code path is platform-specific. +- Run `gofmt` on changed Go files. +- Run `go test ./...` and, for core changes, `go test -race ./internal/...`. +- Update `README.md`, `docs/`, or CLI help when setup, behavior, or user-facing commands change. +- Do not commit generated binaries, profiles, perf reports, temporary config directories, or downloaded artifacts. -That is it. If you are unsure about approach, open a draft PR early and we can iterate on it together. +CI runs the full race-tested Go suite across the supported operating systems, TUI performance budgets when relevant files change, ShellCheck, actionlint, and extension checks when extension files change. diff --git a/README.md b/README.md index 39b1a548a..4deb42939 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ [![BuyMeACoffee](https://raw.githubusercontent.com/pachadotdev/buymeacoffee-badges/main/bmc-violet.svg)](https://www.buymeacoffee.com/surge.downloader) [![Stars](https://img.shields.io/github/stars/SurgeDM/Surge?style=social)](https://github.com/SurgeDM/Surge/stargazers) -[Installation](#installation) โ€ข [Usage](#usage) โ€ข [Themes](docs/THEMES.md) โ€ข [Fonts](docs/FONTS.md) โ€ข [Benchmarks](#benchmarks) โ€ข [Extension](#browser-extension) โ€ข [Settings](docs/SETTINGS.md) โ€ข [CLI Reference](docs/USAGE.md) +[Installation](#installation) โ€ข [Usage](#usage) โ€ข [Development](docs/DEVELOPMENT.md) โ€ข [Themes](docs/THEMES.md) โ€ข [Fonts](docs/FONTS.md) โ€ข [Benchmarks](#benchmarks) โ€ข [Extension](#browser-extension) โ€ข [Settings](docs/SETTINGS.md) โ€ข [CLI Reference](docs/USAGE.md) @@ -66,7 +66,7 @@ Surge is available on multiple platforms. Choose the method that works best for | **macOS / Linux (Homebrew)** | `brew install SurgeDM/tap/surge` | Recommended for Mac/Linux users. | | **Nix / NixOS** | `nix run github:SurgeDM/Surge` | Via Nix flake. NixOS config: `inputs.surge.packages.${pkgs.system}.default` | | **Windows** | `winget install surge-downloader.surge`
or
`scoop install surge` | Recommended for Windows users. | -| **Dockerfile** | [See instructions](#4-server-mode-with-docker-compose) | Run Surge in server mode with Docker Compose | +| **Dockerfile** | [See instructions](#7-server-mode-with-docker-compose) | Run Surge in server mode with Docker Compose | | **Go Install** | `go install github.com/SurgeDM/Surge@latest` | Requires Go 1.25+ | --- @@ -133,7 +133,7 @@ surge service uninstall > [!NOTE] > On Linux, these commands may require `sudo`. On Windows, they should be run in an elevated (Administrator) terminal. -### 4. Remote TUI +### 4. Server Networking and Authentication `surge` and `surge server` bind the HTTP API to `0.0.0.0` (all interfaces) by default. This means the server is accessible via `localhost` (127.0.0.1) as well as your local network IP. @@ -150,7 +150,7 @@ surge service token Alternatively, you can find it in the TUI under **Settings > Extension**. -### 3. Remote TUI +### 5. Remote TUI Connect to a running Surge daemon (local or remote). @@ -170,7 +170,7 @@ By default, `surge connect` uses: - `http://` for loopback and private IP targets - `https://` for public/hostname targets -### 4. Global Connection Flags (CLI + TUI) +### 6. Global Connection Flags (CLI + TUI) These global flags are available on all commands: @@ -182,7 +182,7 @@ Environment variable fallbacks: - `SURGE_HOST` - `SURGE_TOKEN` -### 5. Server Mode with Docker Compose +### 7. Server Mode with Docker Compose Download the compose file and start the container: @@ -289,10 +289,11 @@ Huge thanks to the teams and sponsors helping us build and ship Surge: --- -## Community & Contributing +## Development & Contributing -We love community contributions! Whether it's a bug fix, a new feature, or just cleaning up typos. -PRs are always welcome. For a quick guide, see [CONTRIBUTING.md](CONTRIBUTING.md). +For project setup, the current repository map, focused tests, race checks, TUI performance budgets, extension development, and CI conventions, see the [Development Guide](docs/DEVELOPMENT.md). + +We love community contributions! Whether it's a bug fix, a new feature, or just cleaning up typos, PRs are always welcome. See [CONTRIBUTING.md](CONTRIBUTING.md) for the review checklist. You can check out the [Discussions](https://github.com/SurgeDM/Surge/discussions) for any questions or ideas, or follow us on [X (Twitter)](https://x.com/SurgeDownloader)! diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md new file mode 100644 index 000000000..d6ae7dd39 --- /dev/null +++ b/docs/DEVELOPMENT.md @@ -0,0 +1,227 @@ +# Development Guide + +This guide covers the normal local workflow for contributing to Surge. Run commands from the repository root unless noted otherwise. + +## Prerequisites + +- Git +- Go **1.25 or newer** (the module declares `go 1.25.0`) +- A POSIX shell for the commands below; Windows contributors can use Git Bash or adapt the commands to PowerShell + +Optional tools: + +- Node.js 22+ and npm for the browser extension +- ShellCheck for shell scripts +- `actionlint` for GitHub Actions files +- `strace` and `perf` for Linux profiling +- Nix, if you use the flake-based build + +## Development dependencies + +The core application has no separate vendored development-dependency file. Use the tools below only for the workflows that need them: + +- **Go modules:** `go mod download` fetches the versions pinned by `go.mod` and `go.sum`; no global Go packages are required for normal builds and tests. +- **Python 3.8+:** the performance-history helper and its tests use only the Python standard library. There is no `pip install`, virtual environment, or `requirements.txt` step. +- **Go lint tools:** the lint workflow runs `golangci-lint` and `actionlint` through `go run`, so they do not need to be installed globally. +- **ShellCheck:** optional locally and installed by CI on Ubuntu; use it for shell-script changes when available. +- **GitHub CLI (`gh`):** optional, useful for downloading TUI performance-history artifacts. +- **Node.js 22+ and npm:** required only for browser-extension development; `npm ci` installs the locked extension development dependencies. +- **Nix:** optional; the flake supplies a reproducible build environment if preferred. + +The profiling tools `strace`, `perf`, and `go tool pprof` are optional Linux/development tools and are not needed for normal tests. + +## First-time setup + +```bash +git clone https://github.com/SurgeDM/Surge.git +cd Surge +go mod download +go build -o ./surge . +``` + +This setup is sufficient for the Go application, unit tests, and race tests. For the optional Python perf checks, verify the standard-library interpreter: + +```bash +python3 --version +python3 -m unittest discover -s scripts -p 'test_*.py' +``` + +For extension development, install the locked Node dependencies separately: + +```bash +cd extension +npm ci +``` + +Run the CLI without installing it globally: + +```bash +go run . --help +go run . version +``` + +When running local commands that may write configuration or resume state, use an isolated configuration directory so development data does not mix with your normal Surge installation: + +```bash +XDG_CONFIG_HOME="$(mktemp -d)" XDG_CACHE_HOME="$(mktemp -d)" go run . +``` + +On Windows, set equivalent temporary `XDG_CONFIG_HOME` and `XDG_CACHE_HOME` directories in your shell before running tests or the TUI. + +## Development loop + +The main Go packages are organized as follows: + +- `cmd/`: CLI commands and process startup +- `internal/orchestrator/`: lifecycle, enqueue, pause/resume, and event coordination +- `internal/scheduler/`: queued and active download scheduling, rate limits, and shutdown +- `internal/strategy/concurrent/`: ranged, mirrored, retried, and hedged downloads +- `internal/strategy/single/`: single-connection fallback downloads and throttling +- `internal/probe/` and `internal/transport/`: server probing and network/rate-limit primitives +- `internal/progress/` and `internal/store/`: progress state and persistence +- `internal/tui/`: Bubble Tea model, update loop, views, components, and rendering tests +- `internal/testutil/`: reusable HTTP servers and test fixtures +- `extension/`: WXT/Solid browser extension + +After a change, format only the Go files you touched or format the package files directly: + +```bash +gofmt -w path/to/changed.go path/to/changed_test.go +``` + +## Go tests and checks + +Run the complete unit test suite: + +```bash +go test ./... +``` + +Run focused tests while iterating: + +```bash +go test ./internal/tui ./internal/tui/components -count=1 +go test ./internal/strategy/concurrent -count=1 +go test ./internal/transport -run 'RateLimiter|HostRateLimiter' -count=1 +``` + +Run race coverage for all internal packages, matching the core CI coverage: + +```bash +go test -race ./internal/... +``` + +Useful additional checks: + +```bash +go test -cover ./... +go vet ./... +go build ./... +``` + +The Nix package redirects `HOME` during its checks because tests write configuration files. If you run tests through Nix, use the flake/package definitions rather than a normal home directory. + +## Linting and workflow checks + +The core lint workflow runs Go linting, Unicode checks, ShellCheck, and actionlint. The corresponding local commands are: + +```bash +go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest run +go test ./internal/lint/... +python3 -m unittest discover -s scripts -p 'test_*.py' +find scripts -type f -name '*.sh' -print0 | xargs -0 -r shellcheck +go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.7 +``` + +The Go lint commands download their tools through the Go tool cache; actionlint uses the version shown above, while golangci-lint follows the workflowโ€™s `@latest` policy. The Python command uses only the standard library. ShellCheck is the only command in this list that is not available through the repository or Go toolchain; CI installs it explicitly. + +If ShellCheck is not installed locally, install it using your platform package manager or rely on the Ubuntu CI lint job, which installs it before running the check. + +## TUI performance checks + +The TUI budget tests are opt-in so ordinary test runs are not affected by shared-host timing variance: + +```bash +GOMAXPROCS=2 SURGE_PERF_BUDGET=1 \ + go test ./internal/tui -run '^TestTUI.*RenderPerfBudget$' -count=3 -v +``` + +Run the rendering benchmarks and compare cached versus invalidated frames: + +```bash +go test ./internal/tui -run '^$' \ + -bench '^BenchmarkCPU_(FullView_Old|FullView_New|DashboardPanes_Cached)$' \ + -benchmem -count=3 -benchtime=500ms +``` + +Capture and inspect profiles when changing rendering code: + +```bash +go test ./internal/tui -run '^$' \ + -bench '^BenchmarkCPU_DashboardPanes_Cached$' -benchtime=5s \ + -cpuprofile=.tui.cpu.pprof -memprofile=.tui.mem.pprof + +go tool pprof -top -cum .tui.cpu.pprof +go tool pprof -top -sample_index=alloc_space .tui.mem.pprof +rm -f .tui.cpu.pprof .tui.mem.pprof +``` + +On Linux, `strace -f -c` helps identify syscall overhead and `perf stat -d` helps compare CPU/cache behavior. Keep generated profiles and reports out of commits. + +To compare two saved CI performance reports locally: + +```bash +python3 scripts/compare_tui_perf.py \ + --current current-perf/tui-perf.txt \ + --previous previous-perf/tui-perf.txt \ + --output perf-comparison.txt +``` + +The comparison uses medians and fails when latency grows by more than 25% or allocations by more than 10%. Missing or malformed samples are treated as failures rather than silently producing a misleading comparison. + +## Browser extension development + +```bash +cd extension +npm ci +npm run check +npm run lint +npm test +npm run dev +``` + +Build extension artifacts with: + +```bash +npm run build +npm run build:firefox +npm run zip +npm run zip:firefox +``` + +## CI behavior + +- `Core Build and Release` runs build and race-tested Go tests on Linux, macOS, and Windows. +- The TUI performance job runs only when TUI or Go dependency files change. It reports cached and invalidated measurements in the job summary and uploads a 90-day artifact containing the raw report and metadata. +- The TUI regression job compares the current report with the previous successful baseline on the target branch. It allows normal hosted-runner variance but fails on a material latency or allocation regression. +- `Core Lint` runs Go linting, ShellCheck, actionlint, and the perf-comparison unit tests. + +### Inspecting performance history + +The TUI performance job uploads two artifacts for each run: + +- `tui-perf-`: raw test output plus runner/commit metadata +- `tui-perf-comparison-`: the baseline comparison report + +Find recent successful core-build runs and download an artifact with the GitHub CLI: + +```bash +gh run list --workflow core-build.yml --status success --limit 10 +gh run download -n tui-perf- -D .tmp/tui-perf +cat .tmp/tui-perf/tui-perf.txt +cat .tmp/tui-perf/tui-perf-metadata.txt +``` + +The regression job compares medians against the previous successful run on the target branch. It tolerates up to 25% latency growth and 10% allocation growth to avoid failing on ordinary hosted-runner noise; larger changes fail the job and are included in the step summary. + +`act` is not required for local workflow validation. Running actionlint plus the commands above catches YAML, expression, workflow, shell, and test issues without starting a local Actions runner. `gh` is only needed when downloading historical artifacts; CI provides its own authenticated token for baseline lookup. diff --git a/internal/progress/bitmap.go b/internal/progress/bitmap.go index 57d5370a1..f2326c338 100644 --- a/internal/progress/bitmap.go +++ b/internal/progress/bitmap.go @@ -8,12 +8,31 @@ import ( "github.com/SurgeDM/Surge/internal/utils" ) +// bitmapSnap is an immutable snapshot of the tracker, cached until the next +// mutation so callers (TUI render, progress aggregator, pause state) don't +// re-scan and re-allocate the full width on every access. +type bitmapSnap struct { + version uint64 + totalSize int64 + packed []byte + progress []int64 +} + type BitmapTracker struct { mu sync.RWMutex chunkStatus []atomic.Int32 chunkProgress []atomic.Int64 actualChunkSize int64 width int + + // snapVersion bumps on every mutation; snap holds the last consistent view. + snapVersion atomic.Uint64 + snap atomic.Pointer[bitmapSnap] +} + +// bumpVersion invalidates the cached snapshot after any state mutation. +func (b *BitmapTracker) bumpVersion() { + b.snapVersion.Add(1) } // bitmapLayout returns the number of tracked chunks and backing bytes for a @@ -39,6 +58,7 @@ func (b *BitmapTracker) Reset() { if b.width > 0 { b.chunkStatus = make([]atomic.Int32, b.width) b.chunkProgress = make([]atomic.Int64, b.width) + b.bumpVersion() } } @@ -67,6 +87,7 @@ func (b *BitmapTracker) InitBitmap(totalSize int64, chunkSize int64) { b.width = numChunks b.chunkStatus = make([]atomic.Int32, numChunks) b.chunkProgress = make([]atomic.Int64, numChunks) + b.bumpVersion() } // RestoreBitmap restores the chunk bitmap from saved state. @@ -100,6 +121,7 @@ func (b *BitmapTracker) RestoreBitmap(totalSize int64, bitmap []byte, actualChun if len(b.chunkProgress) != numChunks { b.chunkProgress = make([]atomic.Int64, numChunks) } + b.bumpVersion() } // SetChunkProgress updates chunk progress array from external sources. @@ -116,6 +138,7 @@ func (b *BitmapTracker) SetChunkProgress(progress []int64) { for i, v := range progress { b.chunkProgress[i].Store(v) } + b.bumpVersion() } // SetChunkState sets the state for a specific chunk index. @@ -131,6 +154,7 @@ func (b *BitmapTracker) setChunkState(index int, status types.ChunkStatus) { return } b.chunkStatus[index].Store(int32(status)) + b.bumpVersion() } // GetChunkState gets the state for a specific chunk index. @@ -167,6 +191,7 @@ func (b *BitmapTracker) UpdateChunkStatus(totalSize, offset, length int64, statu } var totalIncrement int64 + var changed bool for i := startIdx; i <= endIdx; i++ { chunkStart := int64(i) * b.actualChunkSize @@ -192,6 +217,18 @@ func (b *BitmapTracker) UpdateChunkStatus(totalSize, offset, length int64, statu switch status { case types.ChunkCompleted: + markCompleted := func() bool { + for { + currentStatus := b.chunkStatus[i].Load() + if currentStatus == int32(types.ChunkCompleted) { + return false + } + if b.chunkStatus[i].CompareAndSwap(currentStatus, int32(types.ChunkCompleted)) { + return true + } + } + } + // Lock-free CAS loop to avoid overcounting under concurrent updates for { currentProg := b.chunkProgress[i].Load() @@ -204,16 +241,19 @@ func (b *BitmapTracker) UpdateChunkStatus(totalSize, offset, length int64, statu if inc <= 0 { // We might have already reached the end or overlap is zero. - if currentProg >= (chunkEnd - chunkStart) { - b.chunkStatus[i].Store(int32(types.ChunkCompleted)) + if currentProg >= (chunkEnd-chunkStart) && markCompleted() { + changed = true } break } if b.chunkProgress[i].CompareAndSwap(currentProg, currentProg+inc) { totalIncrement += inc + changed = true if currentProg+inc >= (chunkEnd - chunkStart) { - b.chunkStatus[i].Store(int32(types.ChunkCompleted)) + if markCompleted() { + changed = true + } } else { b.chunkStatus[i].CompareAndSwap(int32(types.ChunkPending), int32(types.ChunkDownloading)) } @@ -221,10 +261,15 @@ func (b *BitmapTracker) UpdateChunkStatus(totalSize, offset, length int64, statu } } case types.ChunkDownloading: - b.chunkStatus[i].CompareAndSwap(int32(types.ChunkPending), int32(types.ChunkDownloading)) + if b.chunkStatus[i].CompareAndSwap(int32(types.ChunkPending), int32(types.ChunkDownloading)) { + changed = true + } } } + if changed { + b.bumpVersion() + } return totalIncrement } @@ -313,6 +358,7 @@ func (b *BitmapTracker) RecalculateProgress(totalSize int64, remainingTasks []ty b.chunkStatus[i].Store(int32(types.ChunkPending)) } } + b.bumpVersion() return total } @@ -325,6 +371,18 @@ func (b *BitmapTracker) GetBitmapSnapshot(totalSize int64, includeProgress bool) return nil, 0, 0, 0, nil } + // Fast path: return the previously-built snapshot when nothing has + // changed since it was cached. Snapshot reads are read-only by contract, + // so callers may safely share the cached slices. + if s := b.snap.Load(); s != nil && s.version == b.snapVersion.Load() && s.totalSize == totalSize { + if includeProgress { + return s.packed, b.width, totalSize, b.actualChunkSize, s.progress + } + return s.packed, b.width, totalSize, b.actualChunkSize, nil + } + + v := b.snapVersion.Load() + _, bytesNeeded, _ := bitmapLayout(totalSize, b.actualChunkSize) result := make([]byte, bytesNeeded) @@ -336,13 +394,20 @@ func (b *BitmapTracker) GetBitmapSnapshot(totalSize int64, includeProgress bool) result[byteIndex] |= val } - var progressResult []int64 - if includeProgress { - progressResult = make([]int64, len(b.chunkProgress)) - for i := 0; i < len(b.chunkProgress); i++ { - progressResult[i] = b.chunkProgress[i].Load() - } + progressResult := make([]int64, len(b.chunkProgress)) + for i := 0; i < len(b.chunkProgress); i++ { + progressResult[i] = b.chunkProgress[i].Load() + } + + // Only cache a view consistent with the version we scanned. If a + // mutation raced in mid-scan the version moved; drop the cache write and + // let the next call rebuild against the current state. + if b.snapVersion.Load() == v { + b.snap.Store(&bitmapSnap{version: v, totalSize: totalSize, packed: result, progress: progressResult}) } - return result, b.width, totalSize, b.actualChunkSize, progressResult + if includeProgress { + return result, b.width, totalSize, b.actualChunkSize, progressResult + } + return result, b.width, totalSize, b.actualChunkSize, nil } diff --git a/internal/progress/bitmap_test.go b/internal/progress/bitmap_test.go index 87e451763..f952be2b4 100644 --- a/internal/progress/bitmap_test.go +++ b/internal/progress/bitmap_test.go @@ -63,11 +63,16 @@ func TestBitmapTracker_UpdateChunkStatus(t *testing.T) { t.Errorf("expected state ChunkCompleted, got %v", state) } - // Try to update chunk 0 again, should return 0 increment + // Try to update chunk 0 again, should return 0 increment without + // invalidating the cached snapshot a second time. + version := bt.snapVersion.Load() inc = bt.UpdateChunkStatus(totalSize, 0, 250, types.ChunkCompleted) if inc != 0 { t.Errorf("expected increment 0, got %d", inc) } + if got := bt.snapVersion.Load(); got != version { + t.Fatalf("repeated completion changed snapshot version from %d to %d", version, got) + } } func TestBitmapTracker_ConcurrentUpdates(t *testing.T) { diff --git a/internal/progress/progress.go b/internal/progress/progress.go index aa7ca5a7a..e905e301b 100644 --- a/internal/progress/progress.go +++ b/internal/progress/progress.go @@ -300,3 +300,13 @@ func (ps *DownloadProgress) GetBitmap() ([]byte, int, int64, int64, []int64) { func (ps *DownloadProgress) GetBitmapSnapshot(includeProgress bool) ([]byte, int, int64, int64, []int64) { return ps.Bitmap.GetBitmapSnapshot(ps.Bytes.TotalSize.Load(), includeProgress) } + +// GetBitmapVersion returns the snapshot version of the bitmap. It changes +// whenever chunk state is mutated, so callers can cheaply detect that a +// previously computed bitmap snapshot is still current without re-scanning. +func (ps *DownloadProgress) GetBitmapVersion() uint64 { + if ps == nil { + return 0 + } + return ps.Bitmap.snapVersion.Load() +} diff --git a/internal/transport/rate_limiter.go b/internal/transport/rate_limiter.go index ecb918224..7f36cc02f 100644 --- a/internal/transport/rate_limiter.go +++ b/internal/transport/rate_limiter.go @@ -174,11 +174,18 @@ func (r *RateLimiter) SetRate(rate int64, bucketSize int64) { } } + oldRate := r.rate r.rate = rate r.bucketSize = bucketSize - if rate == 0 { + if rate <= 0 { r.tokens = 0 + } else if oldRate <= 0 { + // Disabling a limiter intentionally drains its bucket. Re-seed the + // bucket when it is enabled again; otherwise the first read after + // removing a throttle waits for a full refill despite having a fresh + // burst budget. + r.tokens = bucketSize } else if r.tokens > bucketSize { r.tokens = bucketSize } diff --git a/internal/transport/rate_limiter_test.go b/internal/transport/rate_limiter_test.go index 687ed34e8..6391cccbf 100644 --- a/internal/transport/rate_limiter_test.go +++ b/internal/transport/rate_limiter_test.go @@ -32,6 +32,34 @@ func TestRateLimiter_SetRateDisableWakesWaiter(t *testing.T) { } } +func TestRateLimiter_NegativeRateIsDisabled(t *testing.T) { + limiter := NewRateLimiter(-1, 100) + if err := limiter.WaitN(context.Background(), 100); err != nil { + t.Fatalf("negative rate should be disabled: %v", err) + } + + limiter.SetRate(-2, 100) + if err := limiter.WaitN(context.Background(), 100); err != nil { + t.Fatalf("negative SetRate should disable limiter: %v", err) + } +} + +func TestRateLimiter_ReenableSeedsFreshBurst(t *testing.T) { + limiter := NewRateLimiter(100, 100) + if err := limiter.WaitN(context.Background(), 100); err != nil { + t.Fatalf("initial WaitN returned error: %v", err) + } + + limiter.SetRate(0, 0) + limiter.SetRate(1000, 1000) + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + if err := limiter.WaitN(ctx, 1000); err != nil { + t.Fatalf("re-enabled limiter did not provide its fresh burst: %v", err) + } +} + func TestRateLimiter_SetRateIncreaseWakesWaiter(t *testing.T) { limiter := NewRateLimiter(1, 0) done := make(chan error, 1) diff --git a/internal/tui/components/add_download_modal.go b/internal/tui/components/add_download_modal.go index 50205bb52..27133f1f3 100644 --- a/internal/tui/components/add_download_modal.go +++ b/internal/tui/components/add_download_modal.go @@ -2,6 +2,7 @@ package components import ( "image/color" + "strings" "github.com/SurgeDM/Surge/internal/tui/colors" "github.com/SurgeDM/Surge/internal/utils" @@ -60,19 +61,19 @@ func (m AddDownloadModal) View() string { ti := m.Inputs[i] ti.SetWidth(inputW) - row := lipgloss.JoinHorizontal(lipgloss.Left, labelStyle.Render(m.Labels[i]), ti.View()) + rowParts := []string{labelStyle.Render(m.Labels[i]), ti.View()} if m.BrowseHintIndex == i { hintStyle := hintBase if m.FocusedInput == i { hintStyle = hintStyle.Foreground(colors.Pink()) } - row = lipgloss.JoinHorizontal(lipgloss.Left, row, hintStyle.Render(m.browseHint())) + rowParts = append(rowParts, hintStyle.Render(m.browseHint())) } - content = append(content, row, "") + content = append(content, lipgloss.JoinHorizontal(lipgloss.Left, rowParts...), "") } content = append(content, m.Help.View(m.HelpKeys)) - return lipgloss.NewStyle().Padding(0, 2).Render(lipgloss.JoinVertical(lipgloss.Left, content...)) + return lipgloss.NewStyle().Padding(0, 2).Render(strings.Join(content, "\n")) } func (m AddDownloadModal) browseHint() string { diff --git a/internal/tui/components/box.go b/internal/tui/components/box.go index 47db594b4..a8e344bfc 100644 --- a/internal/tui/components/box.go +++ b/internal/tui/components/box.go @@ -152,5 +152,7 @@ func RenderBtopBox(leftTitle, rightTitle string, content string, width, height i wrappedLines = append(wrappedLines, borderStyler.Render(vertical)+line+borderStyler.Render(vertical)) } - return lipgloss.JoinVertical(lipgloss.Left, topBorder, strings.Join(wrappedLines, "\n"), bottomBorder) + // Every component line is already padded to the exact box width above, so + // joining directly avoids Lipgloss remeasuring and repadding each line. + return strings.Join([]string{topBorder, strings.Join(wrappedLines, "\n"), bottomBorder}, "\n") } diff --git a/internal/tui/components/confirmation_modal.go b/internal/tui/components/confirmation_modal.go index a5fc8bacd..86182258b 100644 --- a/internal/tui/components/confirmation_modal.go +++ b/internal/tui/components/confirmation_modal.go @@ -53,11 +53,7 @@ func (m ConfirmationModal) renderBody(width int) string { if !strings.Contains(det, "\x1b") { det = getDetailStyle().Render(det) } - content = lipgloss.JoinVertical(lipgloss.Center, - content, - "", - det, - ) + content = strings.Join([]string{content, "", det}, "\n") } if m.ShowYesNoButtons { @@ -70,11 +66,7 @@ func (m ConfirmationModal) renderBody(width int) string { noLabel = "Nope" } - content = lipgloss.JoinVertical(lipgloss.Center, - content, - "", - renderYesNoButtons(yesLabel, noLabel, m.YesNoFocused, m.ButtonColor), - ) + content = strings.Join([]string{content, "", renderYesNoButtons(yesLabel, noLabel, m.YesNoFocused, m.ButtonColor)}, "\n") } return content @@ -112,7 +104,7 @@ func renderYesNoButtons(yesLabel, noLabel string, focused int, btnColor color.Co yesBtn := renderBtn(yesPadStyle, yesFirstStyle, yesRestStyle, yesFirst, yesRest) noBtn := renderBtn(noPadStyle, noFirstStyle, noRestStyle, noFirst, noRest) - return lipgloss.JoinHorizontal(lipgloss.Center, yesBtn, " ", noBtn) + return yesBtn + " " + noBtn } func splitFirst(label string) (string, string) { @@ -181,7 +173,7 @@ func (m ConfirmationModal) RenderWithBtopBox( } lines = append(lines, helpText) - fullContent := lipgloss.JoinVertical(lipgloss.Left, lines...) + fullContent := strings.Join(lines, "\n") // Title goes in the box border return renderBox(titleStyle.Render(" "+m.Title+" "), "", fullContent, m.Width, m.Height, m.BorderColor) diff --git a/internal/tui/components/filepicker_modal.go b/internal/tui/components/filepicker_modal.go index 9f9606f50..5a0f1b8f6 100644 --- a/internal/tui/components/filepicker_modal.go +++ b/internal/tui/components/filepicker_modal.go @@ -2,6 +2,7 @@ package components import ( "image/color" + "strings" "github.com/SurgeDM/Surge/internal/tui/colors" @@ -38,14 +39,14 @@ func NewFilePickerModal(title string, picker *filepicker.Model, helpModel help.M func (m FilePickerModal) View() string { pathStyle := lipgloss.NewStyle().Foreground(colors.LightGray()) - content := lipgloss.JoinVertical(lipgloss.Left, + content := strings.Join([]string{ "", pathStyle.Render(m.Picker.CurrentDirectory), "", m.Picker.View(), "", m.Help.View(m.HelpKeys), - ) + }, "\n") return lipgloss.NewStyle().Padding(0, 2).Render(content) } diff --git a/internal/tui/components/help_modal.go b/internal/tui/components/help_modal.go index 2f5149ba5..447251dc0 100644 --- a/internal/tui/components/help_modal.go +++ b/internal/tui/components/help_modal.go @@ -3,6 +3,7 @@ package components import ( "fmt" "image/color" + "strings" "charm.land/bubbles/v2/help" "charm.land/lipgloss/v2" @@ -54,7 +55,7 @@ func (m HelpModal) RenderWithBtopBox( lines = append(lines, "") } - fullContent := "\n" + lipgloss.JoinVertical(lipgloss.Left, lines...) + "\n" + fullContent := "\n" + strings.Join(lines, "\n") + "\n" return renderBox("", fmt.Sprintf(" %s ", titleStyle.Render(m.Title)), fullContent, m.Width, m.Height, m.BorderColor) } diff --git a/internal/tui/components/list_input_modal.go b/internal/tui/components/list_input_modal.go index 2f4aebdac..e6fc810a5 100644 --- a/internal/tui/components/list_input_modal.go +++ b/internal/tui/components/list_input_modal.go @@ -2,6 +2,7 @@ package components import ( "image/color" + "strings" "github.com/SurgeDM/Surge/internal/tui/colors" @@ -52,7 +53,7 @@ func (m ListInputModal) viewContent() string { valueStyle = lipgloss.NewStyle().Foreground(colors.Gray()) } - labelRow := lipgloss.JoinHorizontal(lipgloss.Left, prefix, labelStyle.Render(item.Label)) + labelRow := prefix + labelStyle.Render(item.Label) var valueStr string if item.IsEditing { @@ -69,7 +70,7 @@ func (m ListInputModal) viewContent() string { rows = append(rows, labelRow, valueStr, "") } - return lipgloss.JoinVertical(lipgloss.Left, rows...) + return strings.Join(rows, "\n") } // RenderWithBtopBox renders the modal using the btop-style box with title in border @@ -148,7 +149,7 @@ func (m ListInputModal) RenderWithBtopBox( lines = append(lines, helpText) } - fullContent := lipgloss.JoinVertical(lipgloss.Left, lines...) + fullContent := strings.Join(lines, "\n") return renderBox(titleStyle.Render(" "+m.Title+" "), "", fullContent, m.Width, m.Height, m.BorderColor) } diff --git a/internal/tui/components/modal_golden_test.go b/internal/tui/components/modal_golden_test.go new file mode 100644 index 000000000..62e8ebf1a --- /dev/null +++ b/internal/tui/components/modal_golden_test.go @@ -0,0 +1,222 @@ +package components + +import ( + "regexp" + "strings" + "testing" + + "charm.land/bubbles/v2/filepicker" + "charm.land/bubbles/v2/help" + "charm.land/bubbles/v2/key" + "charm.land/bubbles/v2/textinput" + "charm.land/lipgloss/v2" +) + +var modalANSI = regexp.MustCompile(`\x1b\[[0-9;]*m`) + +func plainModal(s string) string { + return modalANSI.ReplaceAllString(s, "") +} + +type goldenHelpKeys struct{} + +func (goldenHelpKeys) ShortHelp() []key.Binding { + return []key.Binding{key.NewBinding(key.WithKeys("enter"), key.WithHelp("enter", "select"))} +} + +func (goldenHelpKeys) FullHelp() [][]key.Binding { + return [][]key.Binding{{key.NewBinding(key.WithKeys("enter"), key.WithHelp("enter", "select"))}} +} + +func TestFilePickerModalGolden(t *testing.T) { + tmpDir := t.TempDir() + picker := filepicker.New() + picker.CurrentDirectory = tmpDir + picker.SetHeight(3) + + modal := FilePickerModal{ + Title: " Select Directory ", + Picker: &picker, + Help: help.New(), + HelpKeys: NoKeys{}, + BorderColor: lipgloss.Color("99"), + Width: 44, + Height: 10, + } + got := plainModal(modal.RenderWithBtopBox(RenderBtopBox, lipgloss.NewStyle())) + lines := strings.Split(got, "\n") + // The picker always renders the current directory on the third row; + // normalize that row without depending on platform path separators or + // truncation of the temporary directory name. + if len(lines) > 2 { + lines[2] = "โ”‚ " + strings.Repeat(" ", 35) + "โ”‚" + } + got = strings.Join(lines, "\n") + want := `โ•ญโ”€ Select Directory โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ Bummer. No Files Found. โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ` + if got != want { + t.Fatalf("file-picker golden mismatch:\n got:\n%s\nwant:\n%s", got, want) + } +} + +func TestAddDownloadModalAlignsMultilineRows(t *testing.T) { + input := textinput.New() + input.SetValue(".") + modal := AddDownloadModal{ + Inputs: []textinput.Model{input}, + Labels: []string{"URL:\nextra"}, + FocusedInput: 0, + BrowseHintIndex: -1, + Help: help.New(), + HelpKeys: NoKeys{}, + Width: 40, + } + + got := plainModal(modal.View()) + if !strings.Contains(got, "URL: > .") { + t.Fatalf("multiline input row was not joined top-aligned:\n%s", got) + } +} + +func TestModalGoldens(t *testing.T) { + tests := []struct { + name string + render func() string + want string + }{ + { + name: "confirmation narrow", + render: func() string { + modal := ConfirmationModal{ + Title: "Quit", + Message: "Quit Surge?", + Detail: "1 active download will be paused", + Keys: NoKeys{}, + Help: help.New(), + BorderColor: lipgloss.Color("99"), + Width: 36, + Height: 10, + ShowYesNoButtons: true, + YesLabel: "Yes", + NoLabel: "No", + } + return plainModal(modal.RenderWithBtopBox(RenderBtopBox, lipgloss.NewStyle())) + }, + want: `โ•ญโ”€ Quit โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ +โ”‚ Quit Surge? โ”‚ +โ”‚ โ”‚ +โ”‚1 active download will be paused โ”‚ +โ”‚ โ”‚ +โ”‚ Yes No โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ`, + }, + { + name: "add download narrow", + render: func() string { + url := textinput.New() + url.SetValue("https://example.com/file.zip") + path := textinput.New() + path.SetValue(".") + modal := AddDownloadModal{ + Title: "Add", + Inputs: []textinput.Model{url, path}, + Labels: []string{"URL:", "Path:"}, + FocusedInput: 0, + ShowURL: false, + BrowseHintIndex: -1, + Help: help.New(), + HelpKeys: NoKeys{}, + BorderColor: lipgloss.Color("99"), + Width: 36, + Height: 10, + } + return plainModal(modal.RenderWithBtopBox(RenderBtopBox, lipgloss.NewStyle())) + }, + want: `โ•ญโ”€ Add โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ +โ”‚ โ”‚ +โ”‚ URL: > https://example.com/โ”‚ +โ”‚ โ”‚ +โ”‚ Path: > . โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ`, + }, + { + name: "help narrow", + render: func() string { + modal := HelpModal{ + Title: "Help", + HelpKeys: goldenHelpKeys{}, + Help: help.New(), + BorderColor: lipgloss.Color("99"), + Width: 40, + Height: 10, + } + return plainModal(modal.RenderWithBtopBox(RenderBtopBox, lipgloss.NewStyle())) + }, + want: `โ•ญโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ Help โ”€โ•ฎ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ enter select โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ`, + }, + { + name: "list input narrow", + render: func() string { + input := textinput.New() + input.SetValue("42") + modal := ListInputModal{ + Title: "Limits", + Items: []ListInputItem{{Label: "Global", Value: "10 MB/s"}, {Label: "Workers", Value: "42", IsEditing: true}}, + Cursor: 1, + Input: input, + Help: help.New(), + HelpKeys: NoKeys{}, + BorderColor: lipgloss.Color("99"), + Width: 40, + Height: 12, + } + return plainModal(modal.RenderWithBtopBox(RenderBtopBox, lipgloss.NewStyle())) + }, + want: `โ•ญโ”€ Limits โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ +โ”‚ โ”‚ +โ”‚ Global โ”‚ +โ”‚ 10 MB/s โ”‚ +โ”‚ โ”‚ +โ”‚ โ–ธ Workers โ”‚ +โ”‚ > 42 โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.render() + if got != tt.want { + t.Fatalf("modal golden mismatch:\n got:\n%s\nwant:\n%s", got, tt.want) + } + }) + } +} diff --git a/internal/tui/components/modal_resize_golden_test.go b/internal/tui/components/modal_resize_golden_test.go new file mode 100644 index 000000000..bc470d5b7 --- /dev/null +++ b/internal/tui/components/modal_resize_golden_test.go @@ -0,0 +1,96 @@ +package components + +import ( + "testing" + + "charm.land/bubbles/v2/help" + "charm.land/bubbles/v2/textinput" + "charm.land/lipgloss/v2" +) + +func confirmationModalPlain(width, height int) string { + modal := ConfirmationModal{ + Title: "Quit", + Message: "Quit Surge?", + Detail: "1 active download will be paused", + Keys: NoKeys{}, + Help: help.New(), + BorderColor: lipgloss.Color("99"), + Width: width, + Height: height, + ShowYesNoButtons: true, + YesLabel: "Yes", + NoLabel: "No", + } + return plainModal(modal.RenderWithBtopBox(RenderBtopBox, lipgloss.NewStyle())) +} + +func addDownloadModalPlain(width, height int) string { + url := textinput.New() + url.SetValue("https://example.com/file.zip") + path := textinput.New() + path.SetValue(".") + modal := AddDownloadModal{ + Title: "Add", + Inputs: []textinput.Model{url, path}, + Labels: []string{"URL:", "Path:"}, + FocusedInput: 0, + BrowseHintIndex: -1, + Help: help.New(), + HelpKeys: NoKeys{}, + BorderColor: lipgloss.Color("99"), + Width: width, + Height: height, + } + return plainModal(modal.RenderWithBtopBox(RenderBtopBox, lipgloss.NewStyle())) +} + +func TestModalResizeGoldens(t *testing.T) { + tests := []struct { + name string + render func() string + want string + }{ + { + name: "confirmation wide", + render: func() string { return confirmationModalPlain(48, 12) }, + want: `โ•ญโ”€ Quit โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ +โ”‚ โ”‚ +โ”‚ Quit Surge? โ”‚ +โ”‚ โ”‚ +โ”‚ 1 active download will be paused โ”‚ +โ”‚ โ”‚ +โ”‚ Yes No โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ`, + }, + { + name: "add download wide", + render: func() string { return addDownloadModalPlain(48, 12) }, + want: `โ•ญโ”€ Add โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ +โ”‚ โ”‚ +โ”‚ URL: > https://example.com/file.zip โ”‚ +โ”‚ โ”‚ +โ”‚ Path: > . โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.render() + if got != tt.want { + t.Fatalf("modal resize golden mismatch:\n got:\n%s\nwant:\n%s", got, tt.want) + } + }) + } +} diff --git a/internal/tui/components/modal_windows_golden_test.go b/internal/tui/components/modal_windows_golden_test.go new file mode 100644 index 000000000..3f073e034 --- /dev/null +++ b/internal/tui/components/modal_windows_golden_test.go @@ -0,0 +1,43 @@ +//go:build windows + +package components + +import ( + "strings" + "testing" + + "charm.land/bubbles/v2/filepicker" + "charm.land/bubbles/v2/help" + "charm.land/lipgloss/v2" +) + +func TestFilePickerWindowsPathGolden(t *testing.T) { + picker := filepicker.New() + picker.CurrentDirectory = `C:\Users\Test\Downloads` + picker.SetHeight(3) + + modal := FilePickerModal{ + Title: " Select Directory ", + Picker: &picker, + Help: help.New(), + HelpKeys: NoKeys{}, + BorderColor: lipgloss.Color("99"), + Width: 44, + Height: 10, + } + got := plainModal(modal.RenderWithBtopBox(RenderBtopBox, lipgloss.NewStyle())) + got = strings.ReplaceAll(got, `\`, "/") + want := `โ•ญโ”€ Select Directory โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ +โ”‚ โ”‚ +โ”‚ C:/Users/Test/Downloads โ”‚ +โ”‚ โ”‚ +โ”‚ Bummer. No Files Found. โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ”‚ โ”‚ +โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ` + if got != want { + t.Fatalf("Windows path golden mismatch:\n got:\n%s\nwant:\n%s", got, want) + } +} diff --git a/internal/tui/cpu_bench_test.go b/internal/tui/cpu_bench_test.go index 3b5f69ea1..3f4f5aa71 100644 --- a/internal/tui/cpu_bench_test.go +++ b/internal/tui/cpu_bench_test.go @@ -186,12 +186,25 @@ func BenchmarkCPU_FullView_Old(b *testing.B) { func BenchmarkCPU_FullView_New(b *testing.B) { m := fullBenchModel(8) m.cachedTotalSpeed = m.calcTotalSpeedBps() - // Pre-warm graph cache (as would happen after first render) + // Pre-warm dashboard pane caches (header, log, list, details, graph). _ = m.View() b.ResetTimer() for i := 0; i < b.N; i++ { - // NEW: no list rebuild, speed cached, graph cached + // Stable spinner frames should reuse all unchanged pane output. + _ = m.View() + } +} + +// BenchmarkCPU_DashboardPanes_Cached is a focused regression benchmark for +// spinner-driven redraws. It intentionally renders a stable model so a future +// change that bypasses one of the pane caches shows up in allocations/time. +func BenchmarkCPU_DashboardPanes_Cached(b *testing.B) { + m := fullBenchModel(8) + _ = m.View() + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { _ = m.View() } } diff --git a/internal/tui/graph.go b/internal/tui/graph.go index 15a3db454..eee42fecc 100644 --- a/internal/tui/graph.go +++ b/internal/tui/graph.go @@ -33,6 +33,14 @@ type GraphRenderer struct { styleBuf [][]bool // false = grid style, true = block style (row color) lastRender string + + // Fingerprint of the inputs that produced lastRender, so a no-op frame + // (graph data only updates every GraphUpdateInterval) skips the whole + // rebuild + RLE pass instead of re-rendering identical output. + lastData []float64 + lastWidth int + lastHeight int + lastMax float64 } func NewGraphRenderer() *GraphRenderer { @@ -44,6 +52,7 @@ func NewGraphRenderer() *GraphRenderer { func (g *GraphRenderer) InvalidateCache() { g.baseGrid = nil g.lastRender = "" + g.lastData = nil g.gridStyle = lipgloss.NewStyle().Foreground(colors.Gray()) } @@ -121,6 +130,19 @@ func (g *GraphRenderer) Render(data []float64, width, height int, maxVal float64 return g.lastRender } + effectiveMaxVal := maxVal + if len(data) > 0 && effectiveMaxVal <= 0 { + effectiveMaxVal = 1 + } + + // Fast path: input fingerprint unchanged since the last render. The + // speed history only advances every GraphUpdateInterval, but View() is + // called far more often (spinner ticks), so this skips the per-block + // buffer work and RLE pass on every no-change frame. + if g.lastRender != "" && g.lastWidth == width && g.lastHeight == height && g.lastMax == effectiveMaxVal && sameFloat64s(g.lastData, data) { + return g.lastRender + } + g.resize(width, height) // 1. Deep copy pristine grid and zero style buffer @@ -134,11 +156,6 @@ func (g *GraphRenderer) Render(data []float64, width, height int, maxVal float64 // 2. Map data if len(data) > 0 { - // Bug fix: maxVal <= 0 causes NaN - if maxVal <= 0 { - maxVal = 1 - } - // Bug fix: Downsample if data > width to prevent column loss var plotData []float64 if len(data) > width { @@ -171,7 +188,7 @@ func (g *GraphRenderer) Render(data []float64, width, height int, maxVal float64 if val < 0 { val = 0 } - pct := val / maxVal + pct := val / effectiveMaxVal if pct > 1.0 { pct = 1.0 } @@ -249,5 +266,24 @@ func (g *GraphRenderer) Render(data []float64, width, height int, maxVal float64 } g.lastRender = graphBuilder.String() + + // Record the fingerprint for the next frame's fast-path check. + g.lastData = append(g.lastData[:0], data...) + g.lastWidth = width + g.lastHeight = height + g.lastMax = effectiveMaxVal return g.lastRender } + +// sameFloat64s reports whether a and b have equal length and values. +func sameFloat64s(a, b []float64) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/internal/tui/graph_test.go b/internal/tui/graph_test.go index a1837afbd..8eb800c08 100644 --- a/internal/tui/graph_test.go +++ b/internal/tui/graph_test.go @@ -3,6 +3,7 @@ package tui import ( "strings" "testing" + "time" "charm.land/lipgloss/v2" ) @@ -68,6 +69,24 @@ func TestGraphRenderer_GradientOutput(t *testing.T) { } } +func TestGraphRenderer_NormalizesNonPositiveMaxForCache(t *testing.T) { + g := NewGraphRenderer() + data := []float64{1, 2, 3} + + first := g.Render(data, 10, 5, 0, false) + if g.lastMax != 1 { + t.Fatalf("effective max = %v, want 1", g.lastMax) + } + + second := g.Render(data, 10, 5, 1, false) + if first != second { + t.Fatal("equivalent renders with fallback and effective max differ") + } + if g.lastMax != 1 { + t.Fatalf("cached max = %v, want 1", g.lastMax) + } +} + func TestGraphRenderer_Downsampling(t *testing.T) { g := NewGraphRenderer() @@ -96,3 +115,26 @@ func TestGraphRenderer_ResizeCache(t *testing.T) { t.Errorf("Cached render output during resize does not match initial render!") } } + +func TestGraphBoxCacheExpiresWhenResizeSettles(t *testing.T) { + m := fullBenchModel(1) + stats := m.ComputeViewStats() + const oldBoxWidth, newBoxWidth, boxHeight = 80, 100, 12 + + m.lastResizeTime = time.Time{} + m.renderGraphBox(oldBoxWidth, boxHeight, stats) + oldWidth := m.graphRenderer.lastWidth + + m.lastResizeTime = time.Now() + m.renderGraphBox(newBoxWidth, boxHeight, stats) + if m.graphRenderer.lastWidth != oldWidth { + t.Fatal("graph renderer did not reuse its previous render during resize") + } + + m.lastResizeTime = time.Now().Add(-time.Second) + m.renderGraphBox(newBoxWidth, boxHeight, stats) + wantWidth, _ := GetGraphAreaDimensions(newBoxWidth, newBoxWidth < MinGraphStatsWidth) + if m.graphRenderer.lastWidth != wantWidth { + t.Fatalf("settled graph width = %d, want %d", m.graphRenderer.lastWidth, wantWidth) + } +} diff --git a/internal/tui/helpers.go b/internal/tui/helpers.go index 25589878c..1c368ec68 100644 --- a/internal/tui/helpers.go +++ b/internal/tui/helpers.go @@ -59,6 +59,7 @@ func (m *RootModel) refreshLogViewportContent() { } m.logViewport.SetContent(strings.Join(wrappedEntries, "\n")) + m.logRenderVersion++ } // removeDownloadByID removes a download from the in-memory list. diff --git a/internal/tui/layout_helpers.go b/internal/tui/layout_helpers.go index 58f1347a7..d6b6a37c4 100644 --- a/internal/tui/layout_helpers.go +++ b/internal/tui/layout_helpers.go @@ -202,7 +202,7 @@ func CalculateDashboardLayout(termW, termH int) DashboardLayout { if l.LogoWidth < 4 { l.LogoWidth = 4 } - l.LogWidth = l.LeftWidth - l.LogoWidth - BoxStyle.GetHorizontalFrameSize() + l.LogWidth = l.LeftWidth - l.LogoWidth if l.LogWidth < 4 { l.LogWidth = 4 } diff --git a/internal/tui/layout_regression_test.go b/internal/tui/layout_regression_test.go index 391749c02..e822301d8 100644 --- a/internal/tui/layout_regression_test.go +++ b/internal/tui/layout_regression_test.go @@ -382,6 +382,10 @@ func TestLayout_CalculateDashboardLayout_SumInvariants(t *testing.T) { label, l.LeftWidth, l.RightWidth, l.AvailableWidth) } } + if l.LogoWidth+l.LogWidth != l.LeftWidth { + t.Errorf("[%s] LogoWidth(%d)+LogWidth(%d) != LeftWidth(%d)", + label, l.LogoWidth, l.LogWidth, l.LeftWidth) + } // ListHeight should not exceed AvailableHeight if l.ListHeight > l.AvailableHeight { diff --git a/internal/tui/list.go b/internal/tui/list.go index 3c3dfc7c2..5b69c33ca 100644 --- a/internal/tui/list.go +++ b/internal/tui/list.go @@ -235,6 +235,7 @@ func (m *RootModel) UpdateListItems() { m.list.SetItems(items) // Reset cursor to top when manually switching tabs (standard behavior) m.list.Select(0) + m.listRenderVersion++ return } @@ -310,6 +311,7 @@ func (m *RootModel) UpdateListItems() { // Reset forced selection m.SelectedDownloadID = "" + m.listRenderVersion++ } // GetSelectedDownload returns the currently selected download from the list diff --git a/internal/tui/model.go b/internal/tui/model.go index 3ee23410e..37d888052 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -165,7 +165,17 @@ type RootModel struct { lastSpeedHistoryUpdate time.Time // Last time SpeedHistory was updated (for 0.5s sampling) cachedTotalSpeed int64 // Cached total speed (bytes/s), updated once per progress batch graphRenderer *GraphRenderer - lastResizeTime time.Time + // Dashboard render caches are pointers because View() has a value receiver: + // an inline value would be copied and the write discarded every frame. + chunkMapCache *renderCache[chunkMapRenderKey] + graphBoxCache *graphBoxCache + headerBoxCache *renderCache[headerBoxRenderKey] + logBoxCache *renderCache[logBoxRenderKey] + listBoxCache *renderCache[listBoxRenderKey] + detailsPaneCache *renderCache[detailsPaneRenderKey] + listRenderVersion uint64 + logRenderVersion uint64 + lastResizeTime time.Time // Notification log system logViewport viewport.Model // Scrollable log viewport @@ -531,6 +541,12 @@ func InitialRootModel(serverPort int, currentVersion string, service service.Dow SettingsFocusedPane: 1, SpeedHistory: make([]float64, GraphHistoryPoints), // 60 points of history (60s at 1s interval) graphRenderer: NewGraphRenderer(), + chunkMapCache: &renderCache[chunkMapRenderKey]{}, + graphBoxCache: &graphBoxCache{}, + headerBoxCache: &renderCache[headerBoxRenderKey]{}, + logBoxCache: &renderCache[logBoxRenderKey]{}, + listBoxCache: &renderCache[listBoxRenderKey]{}, + detailsPaneCache: &renderCache[detailsPaneRenderKey]{}, logViewport: viewport.New(viewport.WithWidth(40), viewport.WithHeight(5)), // Default size, will be resized logEntries: make([]string, 0), SettingsInput: settingsInput, @@ -766,6 +782,24 @@ func (m *RootModel) refreshThemeCaches() { applyListTheme(&m.list) applyFilepickerTheme(&m.filepicker) m.logoCache = "" + if m.headerBoxCache != nil { + m.headerBoxCache.Invalidate() + } + if m.logBoxCache != nil { + m.logBoxCache.Invalidate() + } + if m.listBoxCache != nil { + m.listBoxCache.Invalidate() + } + if m.graphBoxCache != nil { + m.graphBoxCache.Invalidate() + } + if m.chunkMapCache != nil { + m.chunkMapCache.Invalidate() + } + if m.detailsPaneCache != nil { + m.detailsPaneCache.Invalidate() + } if m.graphRenderer != nil { m.graphRenderer.InvalidateCache() } diff --git a/internal/tui/perf_budget_test.go b/internal/tui/perf_budget_test.go new file mode 100644 index 000000000..8aad1d021 --- /dev/null +++ b/internal/tui/perf_budget_test.go @@ -0,0 +1,81 @@ +package tui + +import ( + "os" + "testing" + "time" +) + +const ( + stableDashboardRenderBudget = 2 * time.Millisecond + stableDashboardAllocsBudget = 850 + invalidatedDashboardRenderBudget = 3 * time.Millisecond + invalidatedDashboardAllocsBudget = 2200 +) + +// TestTUIRenderPerfBudget is opt-in because wall-clock budgets are sensitive to +// shared CI hosts. Run it with SURGE_PERF_BUDGET=1 to enforce both the stable +// cached-frame latency and allocation budgets. +func TestTUIRenderPerfBudget(t *testing.T) { + if os.Getenv("SURGE_PERF_BUDGET") != "1" { + t.Skip("set SURGE_PERF_BUDGET=1 to enforce TUI performance budgets") + } + + model := fullBenchModel(8) + _ = model.View() // warm all pane caches + + allocs := testing.AllocsPerRun(20, func() { + _ = model.View() + }) + t.Logf("cached frame: %.0f allocs/op (budget %d)", allocs, stableDashboardAllocsBudget) + if allocs > stableDashboardAllocsBudget { + t.Fatalf("stable dashboard frame allocated %.0f objects, budget is %d", allocs, stableDashboardAllocsBudget) + } + + const iterations = 100 + start := time.Now() + for i := 0; i < iterations; i++ { + _ = model.View() + } + perFrame := time.Since(start) / iterations + t.Logf("cached frame: %v/op (budget %v)", perFrame, stableDashboardRenderBudget) + if perFrame > stableDashboardRenderBudget { + t.Fatalf("stable dashboard frame took %v, budget is %v", perFrame, stableDashboardRenderBudget) + } +} + +// TestTUIInvalidatedRenderPerfBudget covers the slower structural-update path +// so a future list rebuild or pane invalidation regression is still visible. +// It shares the opt-in gate with TestTUIRenderPerfBudget because timing budgets +// are intentionally not enforced on ordinary local test runs. +func TestTUIInvalidatedRenderPerfBudget(t *testing.T) { + if os.Getenv("SURGE_PERF_BUDGET") != "1" { + t.Skip("set SURGE_PERF_BUDGET=1 to enforce TUI performance budgets") + } + + model := fullBenchModel(8) + _ = model.View() + + allocs := testing.AllocsPerRun(10, func() { + model.cachedTotalSpeed = 0 + model.UpdateListItems() + _ = model.View() + }) + t.Logf("invalidated frame: %.0f allocs/op (budget %d)", allocs, invalidatedDashboardAllocsBudget) + if allocs > invalidatedDashboardAllocsBudget { + t.Fatalf("invalidated dashboard frame allocated %.0f objects, budget is %d", allocs, invalidatedDashboardAllocsBudget) + } + + const iterations = 30 + start := time.Now() + for i := 0; i < iterations; i++ { + model.cachedTotalSpeed = 0 + model.UpdateListItems() + _ = model.View() + } + perFrame := time.Since(start) / iterations + t.Logf("invalidated frame: %v/op (budget %v)", perFrame, invalidatedDashboardRenderBudget) + if perFrame > invalidatedDashboardRenderBudget { + t.Fatalf("invalidated dashboard frame took %v, budget is %v", perFrame, invalidatedDashboardRenderBudget) + } +} diff --git a/internal/tui/process.go b/internal/tui/process.go index 00cbc1b90..595bc884e 100644 --- a/internal/tui/process.go +++ b/internal/tui/process.go @@ -27,6 +27,9 @@ func (m *RootModel) processProgressMsg(msg types.DownloadEvent) tea.Cmd { d.Elapsed = msg.Elapsed d.Connections = msg.Connections d.rateLimited = msg.RateLimited + // List items retain pointers to downloads, so progress does not require a + // structural list rebuild. It does require invalidating the cached pane. + m.listRenderVersion++ // Update smoothed ETA on every progress tick d.UpdateETA() diff --git a/internal/tui/update_dashboard.go b/internal/tui/update_dashboard.go index ceb35125b..9d1378c0c 100644 --- a/internal/tui/update_dashboard.go +++ b/internal/tui/update_dashboard.go @@ -370,18 +370,22 @@ func (m RootModel) updateDashboard(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { } if key.Matches(msg, m.keys.Dashboard.LogDown) { m.logViewport.ScrollDown(1) + m.logRenderVersion++ return m, nil } if key.Matches(msg, m.keys.Dashboard.LogUp) { m.logViewport.ScrollUp(1) + m.logRenderVersion++ return m, nil } if key.Matches(msg, m.keys.Dashboard.LogTop) { m.logViewport.GotoTop() + m.logRenderVersion++ return m, nil } if key.Matches(msg, m.keys.Dashboard.LogBottom) { m.logViewport.GotoBottom() + m.logRenderVersion++ return m, nil } return m, nil @@ -395,6 +399,7 @@ func (m RootModel) updateDashboard(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { // Pass messages to the list for navigation/filtering var cmd tea.Cmd m.list, cmd = m.list.Update(msg) + m.listRenderVersion++ return m, cmd } diff --git a/internal/tui/update_modals.go b/internal/tui/update_modals.go index 7986a8744..1810598af 100644 --- a/internal/tui/update_modals.go +++ b/internal/tui/update_modals.go @@ -178,7 +178,11 @@ func (m RootModel) updateDuplicateWarning(msg tea.KeyPressMsg) (tea.Model, tea.C // Focus existing download - find it and select in list for i, d := range m.getFilteredDownloads() { if d.URL == m.pendingURL { + oldIndex := m.list.Index() m.list.Select(i) + if m.list.Index() != oldIndex { + m.listRenderVersion++ + } break } } diff --git a/internal/tui/view.go b/internal/tui/view.go index 67ed1fd1a..05fb42822 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -409,7 +409,7 @@ func (m RootModel) View() tea.View { } else { speedVal = lipgloss.NewStyle().Foreground(colors.LightGray()).Render(utils.FormatSpeed(float64(speedBps))) } - speedChunk := lipgloss.JoinHorizontal(lipgloss.Center, speedGlyph, " ", speedVal) + speedChunk := speedGlyph + " " + speedVal // Global rate limit indicator limitGlyph := lipgloss.NewStyle().Foreground(colors.Pink()).Render("\u26A1") @@ -421,9 +421,9 @@ func (m RootModel) View() tea.View { } var limitChunk string if limitVal != "" { - limitChunk = lipgloss.JoinHorizontal(lipgloss.Center, limitGlyph, " ", limitVal) + limitChunk = limitGlyph + " " + limitVal } else { - limitChunk = lipgloss.JoinHorizontal(lipgloss.Center, limitGlyph, " ", lipgloss.NewStyle().Foreground(colors.Gray()).Render("\u221E")) + limitChunk = limitGlyph + " " + lipgloss.NewStyle().Foreground(colors.Gray()).Render("\u221E") } // Auto-shutdown indicator @@ -441,7 +441,7 @@ func (m RootModel) View() tea.View { versionBlue := colors.ThemeColor("#005cc5", "#58a6ff") versionChunk := lipgloss.NewStyle().Foreground(versionBlue).Render(fmt.Sprintf("v%s", m.CurrentVersion)) - rightFooter := lipgloss.NewStyle().PaddingRight(2).Render(lipgloss.JoinHorizontal(lipgloss.Center, + rightFooter := lipgloss.NewStyle().PaddingRight(2).Render(strings.Join([]string{ speedChunk, dimSep, limitChunk, @@ -449,23 +449,19 @@ func (m RootModel) View() tea.View { powerChunk, dimSep, versionChunk, - )) + }, "")) // Hide help text at very narrow widths - right footer is more important var footerContent string rightFooterWidth := lipgloss.Width(rightFooter) if layout.AvailableWidth < 60 { - footerContent = rightFooter + footerContent = lipgloss.NewStyle().Width(layout.AvailableWidth).Align(lipgloss.Right).Render(rightFooter) } else { leftFooterWidth := layout.AvailableWidth - rightFooterWidth if leftFooterWidth < 0 { leftFooterWidth = 0 } - footerContent = lipgloss.JoinHorizontal( - lipgloss.Top, - lipgloss.NewStyle().Width(leftFooterWidth).Render(helpText), - rightFooter, - ) + footerContent = lipgloss.NewStyle().Width(leftFooterWidth).Render(helpText) + rightFooter } footer := footerContent @@ -477,7 +473,12 @@ func (m RootModel) View() tea.View { var bitmapWidth int var totalSize, chunkSize int64 var chunkProgress []int64 + var bitmapVersion uint64 if layout.ShowChunkMap && !layout.HideRightColumn && selected != nil && !selected.done && selected.state != nil { + // Read the version before the bitmap so a mutation between the two + // calls can never pair a stale bitmap with a newer version that a + // later frame would treat as cache-valid. + bitmapVersion = selected.state.GetBitmapVersion() bitmap, bitmapWidth, totalSize, chunkSize, chunkProgress = selected.state.GetBitmap() } @@ -488,7 +489,7 @@ func (m RootModel) View() tea.View { detailWidth = layout.LeftWidth } if selected != nil { - detailContent = renderFocusedDetails(selected, detailWidth-components.BorderFrameWidth, m.spinner.View()) + detailContent = m.renderDetailsContentCached(selected, detailWidth-components.BorderFrameWidth, m.spinner.View()) } else { detailContent = renderEmptyMessage(detailWidth-components.BorderFrameWidth, layout.DetailHeight-components.BorderFrameHeight, "No download selected") } @@ -496,7 +497,7 @@ func (m RootModel) View() tea.View { // Render Components logoColumn := m.renderHeaderBox(layout.LogoWidth, layout.HeaderHeight) logBox := m.renderLogBox(layout.LogWidth, layout.HeaderHeight) - headerBox := lipgloss.JoinHorizontal(lipgloss.Top, logoColumn, logBox) + headerBox := joinHorizontalFixed(logoColumn, logBox) listBox := m.renderDownloadsBox(layout.LeftWidth, layout.ListHeight, stats) @@ -547,10 +548,10 @@ func (m RootModel) View() tea.View { rightParts = append(rightParts, detailBox) if showActualChunkMap { - chunkBox := m.renderChunkMapBox(layout.RightWidth, layout.ChunkMapHeight, selected, bitmap, bitmapWidth, totalSize, chunkSize, chunkProgress) + chunkBox := m.renderChunkMapBox(layout.RightWidth, layout.ChunkMapHeight, selected, bitmapVersion, bitmap, bitmapWidth, totalSize, chunkSize, chunkProgress) rightParts = append(rightParts, chunkBox) } - rightColumn = lipgloss.JoinVertical(lipgloss.Left, rightParts...) + rightColumn = joinVerticalFixed(rightParts...) } // Assembly @@ -558,23 +559,30 @@ func (m RootModel) View() tea.View { if layout.HideRightColumn { if layout.VerticalLayout { detailBox := renderBtopBox("", PaneTitleStyle.Render(" File Details "), detailContent, layout.LeftWidth, layout.DetailHeight, colors.Gray()) - body = lipgloss.JoinVertical(lipgloss.Left, headerBox, listBox, detailBox) + body = joinVerticalFixed(headerBox, listBox, detailBox) } else { - body = lipgloss.JoinVertical(lipgloss.Left, headerBox, listBox) + body = joinVerticalFixed(headerBox, listBox) } } else { - leftColumn := lipgloss.JoinVertical(lipgloss.Left, headerBox, listBox) - body = lipgloss.JoinHorizontal(lipgloss.Top, leftColumn, rightColumn) + leftColumn := joinVerticalFixed(headerBox, listBox) + body = joinHorizontalFixed(leftColumn, rightColumn) } - body = lipgloss.NewStyle(). - Width(layout.AvailableWidth). - Height(layout.AvailableHeight). - MaxWidth(layout.AvailableWidth). - MaxHeight(layout.AvailableHeight). - Render(body) + // The body is already laid out at exactly AvailableWidth ร— AvailableHeight: + // every pane is rendered at its exact box size (renderBtopBox pads each + // line to innerWidth) and the joins pad lines to the widest element, and + // the layout sums panes to AvailableHeight. The previous full + // Style.Render with Width/MaxWidth re-wrapped and re-measured every ANSI + // line each frame (the single largest per-frame cost in the profile) to + // produce the same dimensions the joins already guarantee. Only vertical + // pad/truncate can change anything, and that only needs line counting. + if h := strings.Count(body, "\n") + 1; h < layout.AvailableHeight { + body += strings.Repeat("\n", layout.AvailableHeight-h) + } else if h > layout.AvailableHeight { + body = strings.Join(strings.Split(body, "\n")[:layout.AvailableHeight], "\n") + } - fullView := lipgloss.JoinVertical(lipgloss.Left, body, footer) + fullView := joinVerticalFixed(body, footer) // Place content into available space, then wrap with WindowStyle margins return m.wrapView(lipgloss.Place(layout.AvailableWidth, m.height, lipgloss.Center, lipgloss.Top, fullView)) } diff --git a/internal/tui/view_assembly.go b/internal/tui/view_assembly.go new file mode 100644 index 000000000..d8a320336 --- /dev/null +++ b/internal/tui/view_assembly.go @@ -0,0 +1,48 @@ +package tui + +import ( + "strings" + + "charm.land/lipgloss/v2" +) + +// joinVerticalFixed joins panes whose widths and heights were already resolved +// by the dashboard layout. Unlike lipgloss.JoinVertical, it does not remeasure +// every line to calculate padding that the box renderers already supplied. +func joinVerticalFixed(parts ...string) string { + if len(parts) == 0 { + return "" + } + return strings.Join(parts, "\n") +} + +// joinHorizontalFixed joins panes with equal, precomputed heights. Dashboard +// boxes are rendered at exact widths, so line-wise concatenation is sufficient. +// Keep Lipgloss as a correctness fallback for callers that pass uneven panes. +func joinHorizontalFixed(parts ...string) string { + if len(parts) == 0 { + return "" + } + + lines := make([][]string, len(parts)) + height := -1 + for i, part := range parts { + lines[i] = strings.Split(part, "\n") + if height == -1 { + height = len(lines[i]) + } else if len(lines[i]) != height { + return lipgloss.JoinHorizontal(lipgloss.Top, parts...) + } + } + + var builder strings.Builder + for row := 0; row < height; row++ { + if row > 0 { + builder.WriteByte('\n') + } + for _, partLines := range lines { + builder.WriteString(partLines[row]) + } + } + return builder.String() +} diff --git a/internal/tui/view_assembly_test.go b/internal/tui/view_assembly_test.go new file mode 100644 index 000000000..877eb3771 --- /dev/null +++ b/internal/tui/view_assembly_test.go @@ -0,0 +1,35 @@ +package tui + +import "testing" + +func TestJoinVerticalFixedGolden(t *testing.T) { + got := joinVerticalFixed( + "โ•ญโ”€โ”€โ•ฎ\nโ”‚A โ”‚\nโ•ฐโ”€โ”€โ•ฏ", + "โ•ญโ”€โ”€โ•ฎ\nโ”‚B โ”‚\nโ•ฐโ”€โ”€โ•ฏ", + ) + want := "โ•ญโ”€โ”€โ•ฎ\nโ”‚A โ”‚\nโ•ฐโ”€โ”€โ•ฏ\nโ•ญโ”€โ”€โ•ฎ\nโ”‚B โ”‚\nโ•ฐโ”€โ”€โ•ฏ" + if got != want { + t.Fatalf("vertical join mismatch:\n got:\n%s\nwant:\n%s", got, want) + } +} + +func TestJoinHorizontalFixedGolden(t *testing.T) { + got := joinHorizontalFixed( + "L1 \nL2 ", + "R1\nR2", + ) + want := "L1 R1\nL2 R2" + if got != want { + t.Fatalf("horizontal join mismatch:\n got:\n%s\nwant:\n%s", got, want) + } +} + +func TestJoinHorizontalFixedNarrowLinesGolden(t *testing.T) { + // A one-cell pane is a valid layout at the narrow boundary. The helper + // must concatenate its exact lines without adding width or padding. + got := joinHorizontalFixed("โ•ญโ•ฎ\nโ•ฐโ•ฏ", "โ•ญโ•ฎ\nโ•ฐโ•ฏ") + want := "โ•ญโ•ฎโ•ญโ•ฎ\nโ•ฐโ•ฏโ•ฐโ•ฏ" + if got != want { + t.Fatalf("narrow horizontal join mismatch: got %q, want %q", got, want) + } +} diff --git a/internal/tui/view_cache.go b/internal/tui/view_cache.go new file mode 100644 index 000000000..a45c72cd8 --- /dev/null +++ b/internal/tui/view_cache.go @@ -0,0 +1,31 @@ +package tui + +// renderCache stores a rendered component until one of its declared inputs +// changes. Component-specific keys keep invalidation dependencies explicit. +type renderCache[K comparable] struct { + key K + render string + valid bool +} + +func (c *renderCache[K]) Get(key K) (string, bool) { + if c == nil || !c.valid || c.key != key { + return "", false + } + return c.render, true +} + +func (c *renderCache[K]) Set(key K, render string) string { + c.key = key + c.render = render + c.valid = true + return render +} + +func (c *renderCache[K]) Invalidate() { + if c == nil { + return + } + c.render = "" + c.valid = false +} diff --git a/internal/tui/view_cache_test.go b/internal/tui/view_cache_test.go new file mode 100644 index 000000000..bf7e6752c --- /dev/null +++ b/internal/tui/view_cache_test.go @@ -0,0 +1,47 @@ +package tui + +import "testing" + +func TestRenderCacheKeyAndInvalidation(t *testing.T) { + type key struct { + width, height int + version uint64 + } + + var cache renderCache[key] + initial := key{width: 80, height: 24, version: 1} + if _, ok := cache.Get(initial); ok { + t.Fatal("empty cache reported a hit") + } + if got := cache.Set(initial, "render"); got != "render" { + t.Fatalf("Set returned %q, want render", got) + } + + tests := []struct { + name string + key key + want bool + }{ + {name: "same inputs", key: initial, want: true}, + {name: "resize", key: key{width: 81, height: 24, version: 1}}, + {name: "state change", key: key{width: 80, height: 24, version: 2}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, got := cache.Get(tt.key) + if got != tt.want { + t.Fatalf("cache hit = %t, want %t", got, tt.want) + } + }) + } + + cache.Invalidate() + if _, ok := cache.Get(initial); ok { + t.Fatal("invalidated cache reported a hit") + } + + cache.Set(initial, "") + if render, ok := cache.Get(initial); !ok || render != "" { + t.Fatalf("empty render cache = %q, %t; want empty hit", render, ok) + } +} diff --git a/internal/tui/view_dashboard_chunkmap.go b/internal/tui/view_dashboard_chunkmap.go index 154044f92..81b94c566 100644 --- a/internal/tui/view_dashboard_chunkmap.go +++ b/internal/tui/view_dashboard_chunkmap.go @@ -6,8 +6,38 @@ import ( "github.com/SurgeDM/Surge/internal/tui/components" ) +// chunkMapRenderKey identifies a rendered chunk map box so the per-block +// recompute (visual chunk downsample + lipgloss.Render per block) only runs +// when the underlying bitmap actually changed, mirroring GraphRenderer. +type chunkMapRenderKey struct { + selectedID string + version uint64 + paused bool + totalSize int64 + width int + height int +} + // renderChunkMapBox returns the visual chunk map layout inside a btop box. -func (m *RootModel) renderChunkMapBox(width, height int, selected *DownloadModel, bitmap []byte, bitmapWidth int, totalSize, chunkSize int64, chunkProgress []int64) string { +func (m *RootModel) renderChunkMapBox(width, height int, selected *DownloadModel, bitmapVersion uint64, bitmap []byte, bitmapWidth int, totalSize, chunkSize int64, chunkProgress []int64) string { + // Lazy-allocate: View() has a value receiver so inline fields would be + // discarded each frame; the cache must live behind a pointer like + // graphRenderer to survive between View() calls. + if m.chunkMapCache == nil { + m.chunkMapCache = &renderCache[chunkMapRenderKey]{} + } + key := chunkMapRenderKey{ + selectedID: selected.ID, + version: bitmapVersion, + paused: selected.paused, + totalSize: totalSize, + width: width, + height: height, + } + if render, ok := m.chunkMapCache.Get(key); ok { + return render + } + contentWidth := width - components.BorderFrameWidth contentHeight := height - components.BorderFrameHeight @@ -47,5 +77,6 @@ func (m *RootModel) renderChunkMapBox(width, height int, selected *DownloadModel innerContent = lipgloss.Place(contentWidth, contentHeight, lipgloss.Center, lipgloss.Top, chunkContentWrapper) } - return renderBtopBox("", PaneTitleStyle.Render(" Chunk Map "), innerContent, width, height, colors.Gray()) + render := renderBtopBox("", PaneTitleStyle.Render(" Chunk Map "), innerContent, width, height, colors.Gray()) + return m.chunkMapCache.Set(key, render) } diff --git a/internal/tui/view_dashboard_details.go b/internal/tui/view_dashboard_details.go new file mode 100644 index 000000000..ae3eb16df --- /dev/null +++ b/internal/tui/view_dashboard_details.go @@ -0,0 +1,57 @@ +package tui + +import ( + "fmt" + "time" +) + +// detailsPaneRenderKey identifies focused details content. The elapsed time +// displayed by the pane has one-second precision, so the current second is part +// of the key; progress and mirror state are included to keep the cache correct +// between progress events as well. +type detailsPaneRenderKey struct { + width int + second int64 + spinner string + selected string + fingerprint string +} + +func (m *RootModel) renderDetailsContentCached(d *DownloadModel, width int, spinnerView string) string { + if d == nil { + return "" + } + if m.detailsPaneCache == nil { + m.detailsPaneCache = &renderCache[detailsPaneRenderKey]{} + } + + key := detailsPaneRenderKey{ + width: width, second: time.Now().Unix(), spinner: spinnerView, + selected: d.ID, fingerprint: detailsFingerprint(d), + } + if render, ok := m.detailsPaneCache.Get(key); ok { + return render + } + + content := renderFocusedDetails(d, width, spinnerView) + return m.detailsPaneCache.Set(key, content) +} + +func detailsFingerprint(d *DownloadModel) string { + mirrorState := "" + if d.state != nil { + for _, mirror := range d.state.GetMirrors() { + mirrorState += fmt.Sprintf("%t:%t;", mirror.Active, mirror.Error) + } + } + + errText := "" + if d.err != nil { + errText = d.err.Error() + } + return fmt.Sprintf("%s|%s|%s|%s|%d|%d|%.6f|%d|%d|%t|%t|%t|%t|%t|%t|%t|%d|%d|%s|%s|%d|%t|%s", + d.ID, d.URL, d.Filename, d.Destination, d.Total, d.Downloaded, + d.Speed, d.Connections, d.RateLimit, d.RateLimitSet, d.done, d.started, + d.paused, d.pausing, d.resuming, d.rateLimited, d.Elapsed, d.StartTime.UnixNano(), + errText, mirrorState, d.lastETA, d.hasEtaSpeed, d.FilenameLower) +} diff --git a/internal/tui/view_dashboard_graph.go b/internal/tui/view_dashboard_graph.go index 9b9689cfb..28530b968 100644 --- a/internal/tui/view_dashboard_graph.go +++ b/internal/tui/view_dashboard_graph.go @@ -11,12 +11,45 @@ import ( "github.com/SurgeDM/Surge/internal/utils" ) +// graphBoxCache memoizes the whole rendered graph box. The speed history +// only advances every GraphUpdateInterval and progress batches arrive on +// ReportInterval, but View() is called far more often (spinner ticks), so +// caching the box output avoids re-running axis/stats/join work on frames +// where none of its inputs changed. +type graphBoxRenderKey struct { + width, height int + resizing bool + cachedTotalSpeed int64 + totalDownloaded int64 +} + +type graphBoxCache struct { + renderCache[graphBoxRenderKey] + speedHistory []float64 +} + // renderGraphBox returns the network activity sparkline box layout. func (m *RootModel) renderGraphBox(width, height int, stats ViewStats) string { if width < 1 || height < 1 { return "" } + // Fast path: all inputs unchanged since the last render. Lazy-allocate + // because View() has a value receiver โ€” inline fields would be copied + // and discarded each frame, so the cache must live behind a pointer. + if m.graphBoxCache == nil { + m.graphBoxCache = &graphBoxCache{} + } + c := m.graphBoxCache + resizing := m.isResizing() + key := graphBoxRenderKey{ + width: width, height: height, resizing: resizing, + cachedTotalSpeed: m.cachedTotalSpeed, totalDownloaded: stats.TotalDownloaded, + } + if render, ok := c.Get(key); ok && sameFloat64s(c.speedHistory, m.SpeedHistory) { + return render + } + contentHeight := height - components.BorderFrameHeight if contentHeight < 1 { @@ -128,7 +161,7 @@ func (m *RootModel) renderGraphBox(width, height int, stats ViewStats) string { if hideGraphStats { // No stats box - graph gets almost full width graphAreaWidth, axisWidth := GetGraphAreaDimensions(width, true) - graphVisual := m.graphRenderer.Render(graphData, graphAreaWidth, graphContentHeight, maxSpeed, m.isResizing()) + graphVisual := m.graphRenderer.Render(graphData, graphAreaWidth, graphContentHeight, maxSpeed, resizing) axisStyle := lipgloss.NewStyle().Width(axisWidth).Foreground(colors.Cyan()).Align(lipgloss.Right) axisLines := buildAxisLines(graphContentHeight, axisStyle) @@ -180,7 +213,7 @@ func (m *RootModel) renderGraphBox(width, height int, stats ViewStats) string { statsBox := statsBoxStyle.Render(statsContent) graphAreaWidth, axisWidth := GetGraphAreaDimensions(width, false) - graphVisual := m.graphRenderer.Render(graphData, graphAreaWidth, graphContentHeight, maxSpeed, m.isResizing()) + graphVisual := m.graphRenderer.Render(graphData, graphAreaWidth, graphContentHeight, maxSpeed, resizing) axisStyle := lipgloss.NewStyle().Width(axisWidth).Foreground(colors.Cyan()).Align(lipgloss.Right) axisLines := buildAxisLines(graphContentHeight, axisStyle) @@ -193,5 +226,9 @@ func (m *RootModel) renderGraphBox(width, height int, stats ViewStats) string { } innerContent := lipgloss.JoinVertical(lipgloss.Left, "", graphWithAxis, "") - return renderBtopBox(PaneTitleStyle.Render(" Network Activity "), "", innerContent, width, height, colors.Cyan()) + render := renderBtopBox(PaneTitleStyle.Render(" Network Activity "), "", innerContent, width, height, colors.Cyan()) + + // Record the fingerprint for the next frame's fast-path check. + c.speedHistory = append(c.speedHistory[:0], m.SpeedHistory...) + return c.Set(key, render) } diff --git a/internal/tui/view_dashboard_header.go b/internal/tui/view_dashboard_header.go index 0b543ac87..56311db1d 100644 --- a/internal/tui/view_dashboard_header.go +++ b/internal/tui/view_dashboard_header.go @@ -8,8 +8,26 @@ import ( "github.com/SurgeDM/Surge/internal/tui/components" ) +type headerBoxRenderKey struct { + width, height int + host string + port int + remote bool +} + // renderHeaderBox displays the Surge logo and the server connection status within a box. func (m *RootModel) renderHeaderBox(width, height int) string { + if width < 1 || height < 1 { + return "" + } + + if m.headerBoxCache == nil { + m.headerBoxCache = &renderCache[headerBoxRenderKey]{} + } + key := headerBoxRenderKey{width: width, height: height, host: m.ServerHost, port: m.ServerPort, remote: m.IsRemote} + if render, ok := m.headerBoxCache.Get(key); ok { + return render + } contentWidth := width - components.BorderFrameWidth contentHeight := height - components.BorderFrameHeight @@ -89,5 +107,6 @@ func (m *RootModel) renderHeaderBox(width, height int) string { innerContent = lipgloss.JoinVertical(lipgloss.Center, logoBox, serverPortContent) } - return renderBtopBox("", PaneTitleStyle.Render(" Server "), innerContent, width, height, colors.Gray()) + render := renderBtopBox("", PaneTitleStyle.Render(" Server "), innerContent, width, height, colors.Gray()) + return m.headerBoxCache.Set(key, render) } diff --git a/internal/tui/view_dashboard_list.go b/internal/tui/view_dashboard_list.go index 1f0a94e38..ee49e9e1c 100644 --- a/internal/tui/view_dashboard_list.go +++ b/internal/tui/view_dashboard_list.go @@ -7,8 +7,37 @@ import ( "github.com/SurgeDM/Surge/internal/tui/components" ) +type listBoxRenderKey struct { + width, height int + version uint64 + activeTab int + activeCount, queuedCount, doneCount int + searchActive bool + searchQuery, searchView string + logFocused bool +} + // renderDownloadsBox generates the download list box with the top-left corner search bar string. func (m *RootModel) renderDownloadsBox(width, height int, stats ViewStats) string { + if width < 1 || height < 1 { + return "" + } + if m.listBoxCache == nil { + m.listBoxCache = &renderCache[listBoxRenderKey]{} + } + searchView := "" + if m.searchActive { + searchView = m.searchInput.View() + } + key := listBoxRenderKey{ + width: width, height: height, version: m.listRenderVersion, activeTab: m.activeTab, + activeCount: stats.ActiveCount, queuedCount: stats.QueuedCount, doneCount: stats.DownloadedCount, + searchActive: m.searchActive, searchQuery: m.searchQuery, searchView: searchView, logFocused: m.logFocused, + } + if render, ok := m.listBoxCache.Get(key); ok { + return render + } + contentWidth := width - components.BorderFrameWidth contentHeight := height - components.BorderFrameHeight @@ -81,5 +110,6 @@ func (m *RootModel) renderDownloadsBox(width, height int, stats ViewStats) strin rightTitle := PaneTitleStyle.Render(" Downloads ") - return renderBtopBox(leftTitle, rightTitle, innerContent, width, height, downloadsBorderColor) + render := renderBtopBox(leftTitle, rightTitle, innerContent, width, height, downloadsBorderColor) + return m.listBoxCache.Set(key, render) } diff --git a/internal/tui/view_dashboard_log.go b/internal/tui/view_dashboard_log.go index 886dd37de..07b5d42b3 100644 --- a/internal/tui/view_dashboard_log.go +++ b/internal/tui/view_dashboard_log.go @@ -5,12 +5,26 @@ import ( "github.com/SurgeDM/Surge/internal/tui/components" ) +type logBoxRenderKey struct { + width, height int + version uint64 + focused bool +} + // renderLogBox returns the full Activity Log box with borders and title. func (m *RootModel) renderLogBox(width, height int) string { if width < 1 || height < 1 { return "" } + if m.logBoxCache == nil { + m.logBoxCache = &renderCache[logBoxRenderKey]{} + } + key := logBoxRenderKey{width: width, height: height, version: m.logRenderVersion, focused: m.logFocused} + if render, ok := m.logBoxCache.Get(key); ok { + return render + } + var innerContent string if len(m.logEntries) == 0 { innerContent = renderEmptyMessage(width-components.BorderFrameWidth, height-components.BorderFrameHeight, "Activity log is empty") @@ -23,5 +37,6 @@ func (m *RootModel) renderLogBox(width, height int) string { logBorderColor = colors.Pink() } - return renderBtopBox(PaneTitleStyle.Render(" Activity Log "), "", innerContent, width, height, logBorderColor) + render := renderBtopBox(PaneTitleStyle.Render(" Activity Log "), "", innerContent, width, height, logBorderColor) + return m.logBoxCache.Set(key, render) } diff --git a/internal/tui/view_test.go b/internal/tui/view_test.go index 9e99dee3b..eb0793b62 100644 --- a/internal/tui/view_test.go +++ b/internal/tui/view_test.go @@ -347,6 +347,32 @@ func TestView_NetworkActivityShowsFiveAxisLabelsWhenTall(t *testing.T) { } } +func TestDashboardPaneCachesInvalidateProgress(t *testing.T) { + m := fullBenchModel(1) + _ = m.View() + if m.listBoxCache == nil || !m.listBoxCache.valid { + t.Fatal("expected the dashboard list pane to be cached") + } + + before := m.listRenderVersion + d := m.downloads[0] + m.processProgressMsg(types.DownloadEvent{ + DownloadID: d.ID, + Downloaded: d.Downloaded + 1, + Total: d.Total, + Speed: d.Speed, + Elapsed: time.Second, + }) + if m.listRenderVersion == before { + t.Fatal("progress did not invalidate the cached list pane") + } + + _ = m.View() + if m.listBoxCache.key.version != m.listRenderVersion { + t.Fatalf("list pane cache version = %d, want %d after progress", m.listBoxCache.key.version, m.listRenderVersion) + } +} + func BenchmarkLogoGradient(b *testing.B) { logoText := ` _______ ___________ ____ diff --git a/internal/utils/remove_windows_test.go b/internal/utils/remove_windows_test.go index 844bebac6..26f636fd9 100644 --- a/internal/utils/remove_windows_test.go +++ b/internal/utils/remove_windows_test.go @@ -38,19 +38,19 @@ func TestRemoveFile_WindowsRetry_Success(t *testing.T) { func TestRemoveFile_WindowsRetry_Exhausted(t *testing.T) { dir := t.TempDir() - file := filepath.Join(dir, "testfile_exhaust.txt") - - // Create a file and hold it open to lock it permanently - f, err := os.Create(file) - if err != nil { - t.Fatalf("Failed to create test file: %v", err) + blockedDir := filepath.Join(dir, "non-empty") + if err := os.Mkdir(blockedDir, 0o755); err != nil { + t.Fatalf("Failed to create directory: %v", err) + } + child := filepath.Join(blockedDir, "child.txt") + if err := os.WriteFile(child, []byte("keep"), 0o644); err != nil { + t.Fatalf("Failed to create directory child: %v", err) } - // Make sure we close it at the very end so we don't leak handles - defer func() { _ = f.Close() }() - // Remove it (should exhaust retries and fail) - err = RemoveFile(file) - if err == nil { - t.Fatalf("RemoveFile succeeded unexpectedly while file was locked") + // A non-empty directory is a stable, non-transient removal failure on + // Windows; unlike an open file, its locking behavior does not vary with + // the runtime's file-sharing flags. + if err := RemoveFile(blockedDir); err == nil { + t.Fatal("RemoveFile succeeded unexpectedly for a non-empty directory") } } diff --git a/scripts/compare_tui_perf.py b/scripts/compare_tui_perf.py new file mode 100644 index 000000000..aaad9c3ef --- /dev/null +++ b/scripts/compare_tui_perf.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 +"""Compare cached and invalidated TUI performance budget reports.""" + +from __future__ import annotations + +import argparse +import re +import statistics +from pathlib import Path +from typing import Dict, List, Optional, Tuple + +TIME_RE = re.compile(r"(cached|invalidated) frame: ([0-9.]+)(ns|ยตs|ms)/op") +ALLOCS_RE = re.compile(r"(cached|invalidated) frame: ([0-9.]+) allocs/op") +TIME_SCALE_MS = {"ns": 1e-6, "ยตs": 1e-3, "ms": 1.0} +MODES = ("cached", "invalidated") +TIME_RATIO_LIMIT = 1.25 +ALLOCS_RATIO_LIMIT = 1.10 + + +def percent_delta(current: float, previous: float) -> Optional[float]: + """Return the percentage change, or None when both values are zero.""" + if previous == 0: + if current == 0: + return None + return float("inf") + return (current / previous - 1) * 100 + + +def format_delta(delta: Optional[float]) -> str: + if delta is None: + return "n/a" + if delta == float("inf"): + return "+inf" + return f"{delta:+.1f}%" + + +def parse_report(text: str) -> Tuple[Dict[str, List[float]], Dict[str, List[float]]]: + """Return per-mode latency in milliseconds and allocation samples.""" + times = {mode: [] for mode in MODES} + allocations = {mode: [] for mode in MODES} + + for mode, value, unit in TIME_RE.findall(text): + times[mode].append(float(value) * TIME_SCALE_MS[unit]) + for mode, value in ALLOCS_RE.findall(text): + allocations[mode].append(float(value)) + + return times, allocations + + +def compare_reports(current_text: str, previous_text: str) -> Tuple[str, List[str]]: + """Return a human-readable comparison and any budget violations.""" + current_times, current_allocations = parse_report(current_text) + previous_times, previous_allocations = parse_report(previous_text) + failures: List[str] = [] + lines: List[str] = [] + + for mode in MODES: + if not current_times[mode] or not previous_times[mode]: + raise ValueError(f"missing {mode} latency samples") + if not current_allocations[mode] or not previous_allocations[mode]: + raise ValueError(f"missing {mode} allocation samples") + + current_time = statistics.median(current_times[mode]) + previous_time = statistics.median(previous_times[mode]) + current_allocs = statistics.median(current_allocations[mode]) + previous_allocs = statistics.median(previous_allocations[mode]) + time_delta = percent_delta(current_time, previous_time) + alloc_delta = percent_delta(current_allocs, previous_allocs) + + lines.append( + f"{mode}: {current_time:.3f} ms/op ({format_delta(time_delta)}), " + f"{current_allocs:.0f} allocs/op ({format_delta(alloc_delta)})" + ) + if (previous_time == 0 and current_time > 0) or current_time > previous_time * TIME_RATIO_LIMIT: + failures.append(f"{mode} latency exceeds {TIME_RATIO_LIMIT:.2f}x baseline") + if (previous_allocs == 0 and current_allocs > 0) or current_allocs > previous_allocs * ALLOCS_RATIO_LIMIT: + failures.append(f"{mode} allocations exceed {ALLOCS_RATIO_LIMIT:.2f}x baseline") + + lines.append( + "FAIL: " + "; ".join(failures) + if failures + else "PASS: no material TUI performance regression detected" + ) + return "\n".join(lines) + "\n", failures + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--current", required=True, type=Path) + parser.add_argument("--previous", required=True, type=Path) + parser.add_argument("--output", type=Path, default=Path("perf-comparison.txt")) + args = parser.parse_args() + + try: + report, failures = compare_reports( + args.current.read_text(encoding="utf-8"), + args.previous.read_text(encoding="utf-8"), + ) + except (OSError, ValueError) as exc: + report = f"FAIL: unable to compare TUI performance reports: {exc}\n" + failures = [str(exc)] + + args.output.write_text(report, encoding="utf-8") + print(report, end="") + return 1 if failures else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/test_compare_tui_perf.py b/scripts/test_compare_tui_perf.py new file mode 100644 index 000000000..dd1f93adf --- /dev/null +++ b/scripts/test_compare_tui_perf.py @@ -0,0 +1,67 @@ +#!/usr/bin/env python3 + +import unittest +from typing import Final + +from compare_tui_perf import compare_reports, parse_report + + +REPORT: Final[str] = """\ + perf_budget_test.go:30: cached frame: 550 allocs/op (budget 850) + perf_budget_test.go:41: cached frame: 0.250ms/op (budget 2ms) + perf_budget_test.go:64: invalidated frame: 1500 allocs/op (budget 2200) + perf_budget_test.go:77: invalidated frame: 0.750ms/op (budget 3ms) +""" + +ZERO_REPORT: Final[str] = """\ + perf_budget_test.go:30: cached frame: 0 allocs/op (budget 850) + perf_budget_test.go:41: cached frame: 0.000ms/op (budget 2ms) + perf_budget_test.go:64: invalidated frame: 0 allocs/op (budget 2200) + perf_budget_test.go:77: invalidated frame: 0.000ms/op (budget 3ms) +""" + + +class CompareTUIPerfTests(unittest.TestCase): + def test_parse_report_converts_units(self): + times, allocations = parse_report(REPORT) + self.assertEqual(times["cached"], [0.25]) + self.assertEqual(times["invalidated"], [0.75]) + self.assertEqual(allocations["cached"], [550.0]) + self.assertEqual(allocations["invalidated"], [1500.0]) + + def test_small_change_passes(self): + report, failures = compare_reports(REPORT, REPORT) + self.assertIn("PASS", report) + self.assertEqual(failures, []) + + def test_latency_regression_fails(self): + slower = REPORT.replace("0.250ms", "0.400ms") + report, failures = compare_reports(slower, REPORT) + self.assertIn("cached latency exceeds", report) + self.assertTrue(failures) + + def test_allocation_regression_fails(self): + more_allocations = REPORT.replace("550 allocs", "700 allocs") + report, failures = compare_reports(more_allocations, REPORT) + self.assertIn("cached allocations exceed", report) + self.assertTrue(failures) + + def test_zero_baseline_equal_values_have_no_delta(self): + report, failures = compare_reports(ZERO_REPORT, ZERO_REPORT) + self.assertIn("cached: 0.000 ms/op (n/a), 0 allocs/op (n/a)", report) + self.assertEqual(failures, []) + + def test_nonzero_value_against_zero_baseline_is_regression(self): + nonzero = ZERO_REPORT.replace("0 allocs/op", "1 allocs/op").replace("0.000ms", "0.001ms") + report, failures = compare_reports(nonzero, ZERO_REPORT) + self.assertIn("cached latency exceeds", report) + self.assertIn("cached allocations exceed", report) + self.assertTrue(failures) + + def test_missing_mode_is_rejected(self): + with self.assertRaises(ValueError): + compare_reports(REPORT.replace("invalidated frame:", "other frame:"), REPORT) + + +if __name__ == "__main__": + unittest.main()