mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 19:35:04 +02:00
d3919713bc9e7ec85389ba0096df6dd95f92ae9f
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandPaperclip
|
d3919713bc |
[codex] Document Storybook visual baseline platform lock (#9216)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Storybook visual baselines protect UI surfaces from unintended visual drift. > - Pixel-perfect screenshot baselines are sensitive to OS, font rasterization, and browser environment. > - The suite already stores external baseline artifacts and has an opt-in CI path. > - Local runs on non-matching platforms can report false-positive diffs unless the platform lock is explicit. > - This pull request documents the Linux/Ubuntu baseline constraint and makes the local static server port explicit. > - The benefit is clearer visual-review guidance and more predictable Playwright web server startup. ## Linked Issues or Issue Description No public GitHub issue exists. ### What happened? The Storybook visual baseline suite requires a matching Linux capture environment for pixel-exact comparisons, but the docs did not clearly warn local users that non-Linux environments can produce false-positive diffs. The Playwright web server command also relied on the static server's default port instead of passing the configured port explicitly. ### Expected behavior Developers should see clear Linux/Ubuntu baseline guidance before running the visual suite locally, and Playwright should start the Storybook static server on the same explicit port that the test config expects. ### Steps to reproduce 1. Review the Storybook visual docs before this PR. 2. Run or inspect the Storybook visual Playwright config. 3. Notice the missing platform guidance and implicit static server port coupling. ### Paperclip version or commit Reproducible on `master` before this branch. ### Deployment mode Local dev (pnpm dev) / built from source. ## What Changed - Documents the Linux/Ubuntu-only baseline limitation in the developer docs and visual-suite README. - Adds `--port` parsing and validation to the Storybook static server helper. - Adds regression coverage for `--port` followed by another flag. - Passes the Playwright web server port explicitly from the Storybook visual config. ## Verification - Passed: `node --check scripts/serve-storybook-static.mjs` - Passed: `node --test scripts/__tests__/serve-storybook-static.test.mjs` - Passed: `node --test scripts/__tests__/storybook-visual-baseline.test.mjs` - Greptile: 5/5 with no unresolved review threads after commit `94a649755a2ae7c4a34a3e8a1f16ec4d26d738fd`. - Not run: full `pnpm test:storybook-visual`, because it builds Storybook and runs the browser visual suite; this PR only changes docs plus server port plumbing. ## Risks Low risk. The server still defaults to port 6106 when no explicit port is provided, and invalid port values now fail fast with a clear error before the Playwright server waits for an unreachable URL. ## Model Used OpenAI GPT-5 Codex coding agent with local command execution and repository editing tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
c07e650cd7 |
feat(ui): single-source design tokens, visual regression suite, and theme retune (#9134)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Its UI is the operator's daily surface: task lists, boards, budgets, agent status — all built on shadcn components and Tailwind > - Visual values (colors, spacing, type sizes, radii) were hardcoded at ~1,600 call sites: the same "small gray label" was 9/10/11px depending on the file, charts disagreed with chips about status colors, two toggle-switch implementations coexisted in two greens, and there was no visual regression coverage > - This made the UI drift-prone and made any restyle a hundreds-of-files project, which discourages design iteration > - This pull request extracts visual values into a single token layer in `ui/src/index.css`, adds a Storybook visual regression suite backed by external immutable baseline archives, and then applies a deliberate retune reviewed change-by-change on screenshot diffs > - The benefit is that Paperclip's look becomes a config surface: retheming is a token edit reviewed as a snapshot diff, drift is blocked by a token gate, and future UI PRs can prove exactly what changed visually without committing hundreds of PNGs ## Linked Issues or Issue Description No existing public issue covers this work (searched "design tokens", "visual regression", "design system" across issues and PRs). Related in spirit: Refs #8982 (theming a hardcoded panel — a one-off instance of the same problem class this PR addresses systematically). **Problem (feature-request form):** UI visual values are hardcoded per call site with no source of truth and no regression coverage; consistency depends on reviewer memory, and restyling requires mass file edits. **Proposed solution (this PR):** a single token layer + enforcement gate + externally stored visual snapshot suite, then an intentional restyle on top of that foundation. ## What Changed - **Token extraction (zero visual change, machine-verified during development):** committed codemods (`scripts/codemod-*.mjs`) moved ~1,600 hardcoded color/type/spacing/radius/shadow/misc values into named tokens in a non-inline `:root` block of `ui/src/index.css`. - **Visual regression suite:** `pnpm test:storybook-visual` covers 255 stories × light/dark = 510 Playwright screenshots at `maxDiffPixels: 0`, plus new primitive-coverage stories and deterministic-render fixes. - **External visual baselines:** committed PNG snapshots were removed. `tests/storybook-visual/baseline-manifest.json` pins an immutable archive URL/hash/size/count, and `scripts/storybook-visual-baseline.mjs` handles `download`, `verify`, `pack`, and trusted maintainer `upload` flows. - **Opt-in visual CI artifacts:** added a `Storybook Visual` workflow that runs on manual dispatch or PRs labeled `storybook-visual`, downloads/verifies the baseline, runs Playwright, and uploads Playwright report/test-result artifacts for review. Normal PR runs do not mutate baseline objects. - **Token gate:** `pnpm check:token-gates` — zero hex literals, zero arbitrary bracket values, zero raw font-sizes in `ui/src/components/**` and `ui/src/pages/**`, with a documented inline allowlist for legitimate opt-outs. - **Theme retune (intentional, snapshot-reviewed):** new base theme values; radius ladder derived from a single `--radius` knob; micro-type cluster collapsed to a named ladder (`--text-nano/micro/compact` + Tailwind `text-xs`/`text-sm`); letter-spacing collapsed to named steps. - **One status-color vocabulary:** charts, quota/budget bar fills, RUNNING/live chips, and liveness indicators all use the canonical `--status-*` hues. Light-mode legibility fixes for red alert surfaces that used dark-tuned text classes. - **One switch:** `ToggleSwitch` restyled to the registry capsule form, second hand-rolled implementation removed, and all call sites unified. - **Docs:** `DESIGN.md` is the design contract; `doc/design/` holds audit reports, decision logs, and updated guidance for external baseline review/update workflows. - Dead code removed (`agentStatusBadge` duplicate map), byte-identical contrast constants consolidated, semantic renames (`--project-seed`/`--project-none`, `--liveness-blue`). ## Verification - `pnpm check:token-gates` — 3/3 gates CLEAN during the design-system run - `pnpm typecheck` && `pnpm --filter @paperclipai/ui build` — green during the design-system run - `node --test scripts/__tests__/storybook-visual-baseline.test.mjs` — pass after external-baseline rework - `pnpm exec tsc --noEmit --pretty false --module NodeNext --moduleResolution NodeNext --target ES2022 --types node,@playwright/test tests/storybook-visual/playwright.config.ts tests/storybook-visual/storybook-visual.spec.ts` — pass after external-baseline rework - `git diff --check origin/pr/9134..HEAD` — pass after external-baseline rework - `find tests/storybook-visual -type f -name '*.png' -print | wc -l` — `0` - `node scripts/storybook-visual-baseline.mjs verify` — intentionally fails closed until the first trusted maintainer publishes the baseline archive and updates `baseline-manifest.json` ## Risks - **Large but shallow:** the PR still touches many UI files due to mechanical token extraction and retune work, but committed PNG snapshot churn has been removed from the branch. - **Baseline publication required before the visual suite can pass in clean clones:** the manifest currently has placeholder archive metadata. A trusted maintainer must publish the first immutable archive, then update `baseline-manifest.json`. - **Rendering platform variance:** the external baseline should be captured in the documented Linux/Chromium environment. Future CI runs verify against the pinned archive and fail closed on checksum/count mismatch. - **Visual CI is opt-in while stabilizing:** add the `storybook-visual` label or dispatch the workflow manually to produce downloadable Playwright report/test-result artifacts. - **Scheduled follow-ups, deliberately out of scope:** Tailwind palette classes map to semantic tokens in a dedicated pass; card/pill component consolidation; ESLint ratchet. Tracked in `doc/design/DECISION-SHEET.md`. ## Model Used Claude Fable 5 (Anthropic, `claude-fable-5`, Mythos-class tier) with extended thinking, running in Claude Code with tool use; mechanical phases delegated to Claude Sonnet subagents. Follow-up external-baseline rework assisted by OpenAI Codex (`gpt-5` coding agent with repository, terminal, and GitHub tool use). All bulk rewrites executed via deterministic, idempotent scripts committed in `scripts/`; intentional visual changes were human-reviewed on screenshot contact sheets. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run targeted local verification and documented the intentional baseline-publication failure above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green *(pending new CI run after this rework)* - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups *(pending review)* - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) and OpenAI Codex --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Dotta <bippadotta@protonmail.com> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Devin FoleyandPaperclip
|
ef37203a48 |
perf(ci): build standalone public packages concurrently (#8567)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - CI runs a Canary Dry Run job that exercises `release.sh`, which builds the standalone sandbox-provider packages for publish > - That step (`scripts/build-standalone-public-packages.mjs`) built the 7 provider plugins serially — each doing `rm -rf dist && tsc` — making it the dominant cost (~49s) inside the slowest PR check (~4.9m wall) after the general-server lane was already sharded > - The packages are independent (their own `node_modules` via `--ignore-workspace`, their own `dist`), so the serial build is pure latency with no correctness benefit > - This pull request builds them with a bounded-concurrency pool sized to the runner CPU count (overridable via `STANDALONE_BUILD_CONCURRENCY`), buffering each package's output and flushing it as one block so parallel logs stay readable, and aggregating failures by original index > - The benefit is a faster Canary Dry Run / PR feedback loop without changing what gets built or published ## Linked Issues or Issue Description No public GitHub issue exists. Inline feature/perf description: ### Problem or motivation `build-standalone-public-packages.mjs` builds standalone provider packages serially, making it the largest single cost inside the slowest PR check. ### Proposed solution Run independent per-package builds through a bounded-concurrency worker pool sized to runner CPU count, with an env override and readable buffered logs. ### Alternatives considered Keep the serial build for simpler logs, but that preserves the avoidable CI latency. ### Roadmap alignment This is CI maintenance and does not overlap planned core roadmap work. ## What Changed - `scripts/build-standalone-public-packages.mjs`: replaced the serial per-package build loop with a bounded-concurrency pool (default = runner CPU count, override via `STANDALONE_BUILD_CONCURRENCY`); per-package stdout/stderr is buffered and flushed as a single block; failures are aggregated by original package index so one failure neither aborts the others mid-flight nor obscures which package broke. - `scripts/__tests__/build-standalone-concurrency.test.mjs`: new `node:test` unit suite covering the pool (limit respected, all items run, ordered failure aggregation, env-override resolution). - `.github/workflows/pr.yml`: wired the new unit test into the policy job. ## Verification - `node --test ./scripts/__tests__/build-standalone-concurrency.test.mjs` → 6/6 pass - `node ./scripts/release-package-map.mjs check` → OK (29 enabled for CI publish) - `git diff --check origin/master..HEAD` → clean ## Risks - Low risk. Build inputs/outputs are unchanged; only scheduling differs. The concurrency is bounded by CPU count and overridable; output is buffered per package so logs remain attributable. If a package fails, all failures are still reported with their package index. ## Model Used - Claude (Anthropic), `claude-opus-4-8`, extended thinking with tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Devin FoleyandPaperclip
|
2853a9ae69 |
perf(ci): shard the general-server test lane across 3 runners (#8360)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Every PR runs the `PR` GitHub Actions workflow, whose `verify` gate fans out into parallel test lanes (general tests, serialized server route suites, build, typecheck) > - The `General tests (server)` lane had grown into the run's critical path: it executed all ~213 non-route server suites serially in a single job (~7.2m of test time), more than 2x any other job > - It runs serially because `server/vitest.config.ts` pins `maxWorkers: 1`, so server suites cannot parallelize within a single runner — the only lever is spreading them across runners > - This pull request shards that lane into 3 even partitions that run on separate runners, mirroring the 4-way sharding already used for the serialized route suites > - The benefit is the lane drops from ~7.7m to ~2.4m/shard, cutting overall PR wall time roughly in half (~8.5m → ~4.2m) ## Linked Issues or Issue Description No public GitHub issue exists for this work, so the underlying issue is described inline following the feature-request template. ### Problem or motivation PR CI wall time had crept back up to ~8.5m. On a recent fully-green run, the `General tests (server)` job took 7.72m — more than double any other job and the clear critical path. Of that, 7.23m was pure test execution (dependency install was a cached 0.27m). The job ran all server suites that are not route/authz tests (213 files) one after another, because the server vitest project pins `maxWorkers: 1`, making these suites inherently serial within a single runner. ### Proposed solution Shard the general-server lane across 3 parallel runners — the same technique the route/authz suites already use — so the suite set is split into even, deterministic partitions that run concurrently. Add a regression test that proves the shards always cover the full suite set with no gaps or overlap. ### Alternatives considered - **Raise `maxWorkers` for the server project** to parallelize within one runner — rejected: the server suites share process-level state (DB/port), which is exactly why `maxWorkers: 1` is pinned. - **Two shards instead of three** — would leave the lane at ~3.6m, still above the next bottleneck (Canary Dry Run, ~4.1m wouldn't be the gate). Three lands the lane comfortably below it. - **Do nothing / accept the slow lane** — rejected: it gates every PR. ### Roadmap alignment Developer-experience / CI tooling. Not core product roadmap work; does not overlap with planned features in `ROADMAP.md`. ## What Changed - `scripts/run-vitest-stable.mjs`: the `general-server` general-test group now accepts `--shard-index` / `--shard-count`. It enumerates the full server test set (the whole `server/src` tree, minus the route/authz suites that already run in their own serialized shards) and splits it deterministically by modulo. The non-sharded local invocation (`pnpm test:run:general --group general-server`) is unchanged. - `.github/workflows/pr.yml`: the `general_tests` matrix runs `general-server` as 3 parallel shards (1/3, 2/3, 3/3). Workspace groups are unchanged. The `verify` gate already aggregates the whole matrix result, so the required check name is unaffected. - `scripts/__tests__/run-vitest-stable-shard.test.mjs`: a `node:test` suite asserting the 3 shards form a complete, non-overlapping partition of the general-server set, that no route/authz suite leaks into it, and that shard flags are rejected for the parallel workspace groups. Wired into the `policy` job. ## Verification - New partition test passes locally: `node --test ./scripts/__tests__/run-vitest-stable-shard.test.mjs` (3/3). - Confirmed the 3 shards form a complete, non-overlapping partition of all 213 files (71/71/71). - Ran a live thin shard (3 real server suites, including one outside `__tests__`) — 23 tests passed, confirming positional-include execution works end to end. - This PR's own CI is the authoritative check: all three `General tests (server (n/3))` jobs went green on the prior run, collectively covering every suite the old single job ran. ## Risks - Low risk. No product code changes — only test orchestration and CI matrix. Shard partitioning is deterministic and is now covered by an automated test that fails if the partition ever develops a gap or overlap. Modulo-on-sorted-filenames balances duration reasonably, matching the approach already proven by the serialized route shards. ## Model Used - Claude (Anthropic), `claude-opus-4-8`, extended thinking + tool use (agentic coding via Paperclip). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots (N/A — no UI change) - [x] I have updated relevant documentation to reflect my changes (inline comments explain the sharding rationale) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending this PR's run) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending review) - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |