ops: wire frontend into shared task infrastructure (#269) by edvardm · Pull Request #271 · OpenFilamentCollective/open-filament-database

edvardm · 2026-04-11T10:47:12Z

Partially addresses #269

What's changed

Taskfile

setup, lint, and check now cover both Python and WebUI. check depends on lint, validate, and test-all.
test runs Python (pytest) only; test-all adds WebUI Playwright tests on top via deps: [test].
Added webui (dev server) and webui-build tasks.
Setup tasks use sentinels so npm install and Playwright browser downloads are skipped when already up to date.
local/Taskfile.yml is gitignored and optionally included — useful for personal tasks not meant for version control.

Tests

Added pytest tests for the utils module covering UUID derivation, slugify, and color normalization (tests/test_utils.py).
Removed placeholder test_dummy.py.
Migrated dev dependencies from [project.optional-dependencies] to [dependency-groups] (PEP 735).

CI

Added pytest.yml GitHub Actions workflow to run Python tests on PRs.
Removed pre-commit config from this branch (handled in a separate PR).

CONTRIBUTING.md

New developer setup guide covering prerequisites, bootstrap via task setup, and key commands. Linked from README.
Added task check to the pre-PR checklist.

Frontend devs can still work directly in webui/ with npm — no need to install task.

- add .setup-webui and .install-playwright with sentinel-based skipping so pnpm install and browser downloads are skipped when already up to date - setup, test, lint, check now delegate to both Python and WebUI - check delegates to lint and test to avoid duplicating commands - add webui (dev server) and webui-build tasks - remove internal: true from setup tasks so they can be run directly

Developer setup docs covering prerequisites (uv, pnpm, task), bootstrap via task setup, and key commands. README links to it from the install requirements step.

pnpm offers stricter dependency isolation (no phantom deps) and a content-addressable store. Sets pnpm >=10 lower bound via engines field. - replace package-lock.json with pnpm-lock.yaml - add engines.pnpm >=10 to package.json - update playwright.config.js webServer command to use pnpm - rename webui-tests.yml -> frontend-tests.yml and switch to pnpm

cjavad · 2026-04-11T10:48:09Z

Hi there @edvardm thanks a lot!

I'll let @coffandro take charge here, i am wondering if we prefer Bun over PNPM though?

Edit: as a side note we might have pointed to NPM as an easy to use beginner friendly tool so less technically minded people could get started, i believe Bun also has an easy install path, not sure about pnpm, but i am sure that is not a concern.

pnpm/action-setup@v4 requires either a version input or packageManager in package.json. Use version: latest to avoid pinning.

edvardm · 2026-04-11T10:58:06Z

I've started to use bun myself when working with FE stuff, so super happy to. I was even thinking of abstracting the tool to use so that devs could choose it, but as they don't use same files I guess that would be extraneous complexity for no reason.

My only small gripe with bun is that IIRC pnpm is a bit better for security-oriented setups, but of course it always depends on the context. So, would I revise the plan and PR to use bun instead of pnpm?

Nice thing with task automation is that it does abstract details like this, in that sense adding taskfile to even frontend could be useful (use 'task dev' to start webui, no need to care what package manager is used). But then again it's extra layer to maintain etc :P

Tests cover UUID generation (spec-defined deterministic values), slugify, normalize_color_hex, and ensure_list — all pure functions in ofd/builder/utils.py. Intended as style reference for future tests.

- Remove module docstring and inline comments - Drop redundant determinism and "names differ" UUID tests - Move uuid import to top level - Use parametrize for slugify and normalize_color_hex - Update testing-guidelines skill: clarify explicit loops are banned, parametrize is recommended

Separate parametrized groups for: normal normalization cases, list input, empty/None inputs, and unparseable passthrough.

cjavad · 2026-04-11T11:28:38Z

Even with bun the default behavior of using something like bun run / bunx is to invoke nodejs as the default runtime anyhow. The advantage i like is the package manager is really good, fast and efficient compared to npm, but even pnpm at times. So usually I'd have nodejs installed on the system, then use bun as the entrypoint/package manager, which functions nicely like uv might.

But as long as the CI can have dep-caching then either solution is fine. As you mentioned the process is entirely opague from the invoker, perhaps making package install generic so bun could be used in CI just for the perf?

…IBUTING

… 735)

…ion control

cjavad · 2026-04-11T12:02:06Z

While we are at it, we should enforce conventionalcommits.org at CI time and pre-commit time. If out of scope of this PR, we'll do it seperately.

edvardm · 2026-04-11T12:05:36Z

just learned there's this new https://github.com/j178/prek which is already getting lots of traction, basically Rust rewrite of pre-commit making it much faster -- just an idea, but to try it out I initially added configuration for that in this repo, very basic stuff, as recommended by Astral software.

But yeah, separate PR to address that is better

edvardm · 2026-04-11T12:45:39Z

Re bun vs pnpm, I'd suggest to keep this more contained we'll just use bun and afterwards see if abstracting FE package manager with taskfile or similar would make sense?

It's simple, but adds at least moderate friction probably for some. I'll add single commit which switches to bun, the whole change affects only 5 files and is pretty isolated, easy to revert if desired.

Edit: even better, let's stick to npm for now as suggested by @cjavad and address pkg manager switch in another PR if desired

cjavad · 2026-04-11T12:46:46Z

Lets perhaps keep npm for this PR then if possible?

coffandro · 2026-04-11T13:21:41Z

Tbh, I would prefer if we keep to using stock NPM with maybe support for bun for power users? The goals of the webui require most people to be able to just run for users who aren't technical so every installation step we'll have to cover in our documentation will hurt our adoption.

edvardm · 2026-04-11T13:28:08Z

Forgot to push, PR should now have less changes and use npm. I fully agree with both; would be outside the scope for this, and in any case should be discussed separately ☺️

Rename .github/workflows/frontend-tests.yml to webui-tests.yml and restore content to match main exactly: workflow name "WebUI Tests", job id test-webui, npm cache config, and build step included.

Add validate as a dep to the check task so it covers lint, data validation, and all tests in one command. Reference task check in a new "Making a PR" section that also encourages single-logical-change PRs.

Why: avoid duplicating the pytest invocation already defined in `test`; aggregate tasks should reuse existing rules.

edvardm · 2026-04-12T11:14:15Z

I'm happy with current changes finally, I can trim it down further if desired

Copilot

Pull request overview

This PR wires the WebUI into the shared Taskfile-based developer workflow, adds initial Python unit tests for utility functions, and introduces a dedicated GitHub Actions workflow for running pytest on PRs.

Changes:

Expanded Taskfile.yml to cover Python + WebUI setup/lint/test flows (including Playwright install and WebUI build/dev tasks).
Added tests/test_utils.py and removed the placeholder tests/test_dummy.py.
Migrated dev tooling dependencies to PEP 735 dependency groups and added a pytest.yml CI workflow; added developer setup docs (CONTRIBUTING.md) and linked them from README.md.

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`uv.lock`	Reflects the dev-dependency metadata changes associated with the dependency group migration.
`tests/test_utils.py`	Adds pytest coverage for UUID derivation, slugification, color normalization, and list normalization utilities.
`tests/test_dummy.py`	Removes placeholder test now that real tests exist.
`Taskfile.yml`	Unifies setup/lint/test/check across Python + WebUI and adds WebUI-specific tasks and sentinels.
`README.md`	Links developers to `CONTRIBUTING.md`.
`pyproject.toml`	Moves dev tooling deps to `[dependency-groups]` (PEP 735).
`CONTRIBUTING.md`	Adds developer setup and common Taskfile/npm workflows documentation.
`.gitignore`	Ignores local Taskfile directory and Python coverage outputs.
`.github/workflows/pytest.yml`	Adds CI job to run Python tests via `task test`.

coffandro · 2026-04-13T14:17:23Z

LGTM assuming those issues are fixed,
if this would create issues with #125 please do says so tho, we're planning to get that merged in the next month or so, so we'll just need to know if we'll need to fix it then :D

edvardm · 2026-04-13T15:25:53Z

Thank you! I tried merging #125 and noticed that after fixing the conflict (using upstream version, as pla+ could be valid substring in filenames?) there would be no conflicts.

Edit: commented on #125 , fixing issues found by Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

edvardm added 3 commits April 11, 2026 13:24

add CONTRIBUTING.md and link it from README

a47cdb3

Developer setup docs covering prerequisites (uv, pnpm, task), bootstrap via task setup, and key commands. README links to it from the install requirements step.

edvardm changed the title ~~Wire WebUI into shared task infrastructure (#269)~~ Wire frontend into shared task infrastructure (#269) Apr 11, 2026

edvardm added 2 commits April 11, 2026 13:50

fix: specify pnpm version in CI workflow

6e5a199

pnpm/action-setup@v4 requires either a version input or packageManager in package.json. Use version: latest to avoid pinning.

rename WebUI -> Frontend in CI workflow name and job id

253f1af

fix: install dependencies before Playwright browser install in CI

156c936

edvardm mentioned this pull request Apr 11, 2026

[Feature Request] Add UV tests #269

Open

edvardm added 5 commits April 11, 2026 14:08

add idiomatic pytest examples for utils functions

775d308

Tests cover UUID generation (spec-defined deterministic values), slugify, normalize_color_hex, and ensure_list — all pure functions in ofd/builder/utils.py. Intended as style reference for future tests.

split normalize_color_hex tests by input type and assertion shape

e2fa42e

Separate parametrized groups for: normal normalization cases, list input, empty/None inputs, and unparseable passthrough.

parametrize ensure_list scalar wrapping cases

c7b84df

collapse ensure_list into single parametrized test

4388e96

edvardm added 12 commits April 11, 2026 14:36

add clean task to remove Python cache files and build artefacts

123c24a

split test task: test runs pytest only, test-all includes WebUI

11786af

add test-cov task with terminal and HTML coverage reports

4fb9ad1

skip fully-covered files in test-cov terminal report

b67c8e4

sort test-cov terminal report by coverage ascending

ed0b327

tasks: add fmt alias for format

b5baa79

format code

65b0004

add pre-commit config with ruff hooks; document opt-in setup in CONTR…

cfd1791

…IBUTING

switch pre-commit hooks from pre-commit to prek

973743a

migrate dev deps from optional-dependencies to dependency-groups (PEP…

35580b8

… 735)

add pr-notifs task to list open upstream PRs involving me

738f38f

support optional local/Taskfile.yml for personal tasks not under vers…

282d025

…ion control

add pytest CI workflow

dad0ce3

edvardm added 2 commits April 11, 2026 16:06

remove pre-commit from deps and config (handled in separate PR)

78eeff4

revert frontend tooling to npm, aligning with upstream main

f0a0b1c

edvardm force-pushed the support-frontend-in-task-automation branch from 0e17ce9 to f0a0b1c Compare April 11, 2026 13:25

edvardm commented Apr 11, 2026

View reviewed changes

Comment thread .github/workflows/frontend-tests.yml Outdated

edvardm commented Apr 11, 2026

View reviewed changes

Comment thread tests/test_utils.py

edvardm added 4 commits April 11, 2026 16:33

align webui workflow with main (rename frontend-tests.yml)

26f379a

Rename .github/workflows/frontend-tests.yml to webui-tests.yml and restore content to match main exactly: workflow name "WebUI Tests", job id test-webui, npm cache config, and build step included.

revert to main version

f4c3831

chore: rm local rule

bc1d02a

add task check to pre-PR checklist in CONTRIBUTING

ff556b6

Add validate as a dep to the check task so it covers lint, data validation, and all tests in one command. Reference task check in a new "Making a PR" section that also encourages single-logical-change PRs.

edvardm commented Apr 11, 2026

View reviewed changes

Comment thread CONTRIBUTING.md

edvardm changed the title ~~Wire frontend into shared task infrastructure (#269)~~ [ops] wire frontend into shared task infrastructure (#269) Apr 11, 2026

refactor: test-all depends on test instead of duplicating pytest cmd

47f1b45

Why: avoid duplicating the pytest invocation already defined in `test`; aggregate tasks should reuse existing rules.

edvardm changed the title ~~[ops] wire frontend into shared task infrastructure (#269)~~ ops: wire frontend into shared task infrastructure (#269) Apr 12, 2026

coffandro requested a review from Copilot April 13, 2026 14:09

Copilot started reviewing on behalf of coffandro April 13, 2026 14:10 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Comment thread CONTRIBUTING.md Outdated

Comment thread .github/workflows/pytest.yml

Comment thread tests/test_utils.py Outdated

edvardm and others added 3 commits April 13, 2026 18:34

Update CONTRIBUTING.md

c4bfd6d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

CR: pin python version

c062309

make note about invalid type

bb9ce2f

Conversation

edvardm commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's changed

Uh oh!

cjavad commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edvardm commented Apr 11, 2026

Uh oh!

cjavad commented Apr 11, 2026

Uh oh!

cjavad commented Apr 11, 2026

Uh oh!

edvardm commented Apr 11, 2026

Uh oh!

edvardm commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjavad commented Apr 11, 2026

Uh oh!

coffandro commented Apr 11, 2026

Uh oh!

edvardm commented Apr 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

edvardm commented Apr 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coffandro commented Apr 13, 2026

Uh oh!

edvardm commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

edvardm commented Apr 11, 2026 •

edited

Loading

cjavad commented Apr 11, 2026 •

edited

Loading

edvardm commented Apr 11, 2026 •

edited

Loading

edvardm commented Apr 13, 2026 •

edited

Loading