chore(deps): pin Playwright + jsonschema as dev dependencies (#95 / #94 follow-up)#100
Conversation
…/ #94) Pins the two third-party Python packages that the repo's dev tooling imports, so a fresh clone is reproducible without ambient pip state: - playwright>=1.54,<2.0 — tools/capture-gallery-hero.py (#67 / #94 hero PNG) - jsonschema>=4.0,<5.0 — verify-plugin.sh, verify-security.sh, verify-seed-expectations.sh, mock-bootstrap.sh, generate-spec-anchor-audit.py Preview Forge is not a Python package, so no pyproject.toml — a plain requirements-dev.txt matches the script-driven nature of the Python side. The capture-gallery-hero.py docstring now references the new install contract. Verified: - python3 -m pip install -r requirements-dev.txt (fresh venv) — succeeds - bash scripts/verify-plugin.sh — Pass: 57 / Fail: 0 - bash tests/fixtures/security/verify-security.sh — all defenses holding - bash tests/e2e/mock-bootstrap.sh {standard,pro,max} — PASS x3
Adds a "Python dev setup (optional, for tooling / verify-* scripts)" subsection under "Local setup", documenting the requirements-dev.txt contract added in the preceding commit (Playwright + jsonschema) and the optional `playwright install` step for hero-screenshot regen. Notes that Preview Forge is not a Python package (no pyproject.toml) and that verify-plugin.sh degrades gracefully on missing jsonschema locally but is fail-closed in CI.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 32 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request establishes a Python development environment for tooling by pinning dependencies in a new requirements-dev.txt and updating documentation in CONTRIBUTING.md and capture-gallery-hero.py. A review comment points out that the documentation incorrectly claims verify-plugin.sh is fail-closed in CI, suggesting a correction to align the text with the actual script behavior.
| installed by `bash scripts/verify-plugin.sh` itself (it degrades gracefully | ||
| when `jsonschema` is missing, except in CI where it is fail-closed). |
There was a problem hiding this comment.
The documentation states that verify-plugin.sh is fail-closed in CI when jsonschema is missing, but the current implementation of scripts/verify-plugin.sh (lines 100-109) only prints a warning and does not trigger a failure or exit with a non-zero code. This creates a risk of silent validation regressions in CI, which violates the general rule requiring hard failures for missing validation dependencies in CI environments. Please either update the script to enforce this failure in CI or update the documentation to reflect the current behavior.
| installed by `bash scripts/verify-plugin.sh` itself (it degrades gracefully | |
| when `jsonschema` is missing, except in CI where it is fail-closed). | |
| installed by bash scripts/verify-plugin.sh itself (it degrades gracefully | |
| when jsonschema is missing). |
References
- In CI environments, treat missing optional dependencies required for validation checks (like schema validation) as hard failures to prevent silent regressions.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 532d52daf2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # tools/capture-gallery-hero.py — regenerates docs/assets/v1.6-gallery-hero.png | ||
| # (the README/SUBMISSION above-the-fold gallery hero, #67 / #94). | ||
| # --------------------------------------------------------------------------- | ||
| playwright>=1.54,<2.0 |
There was a problem hiding this comment.
Use an exact Playwright version instead of a range here. With >=1.54,<2.0, pip install -r requirements-dev.txt can resolve to different Playwright builds over time, and Playwright releases carry browser/runtime changes that can alter screenshot rendering. In practice, two fresh clones run on different dates can produce different v1.6-gallery-hero.png output, which breaks the reproducibility goal described in this commit.
Useful? React with 👍 / 👎.
1 participant
Summary
Closes the Dev tooling pinning cluster from #95 (1 item deferred from #94).
requirements-dev.txtpinning the two third-party Python packages the repo's tooling actually imports:playwright>=1.54,<2.0— used bytools/capture-gallery-hero.pyto regendocs/assets/v1.6-gallery-hero.png(🟡 I-5: README hero is text-first — violates Godin gallery-first guidance #67 / docs(readme): gallery-first hero shot above the fold (#67) #94).jsonschema>=4.0,<5.0— used byverify-plugin.sh,verify-security.sh,verify-seed-expectations.sh,mock-bootstrap.sh, andgenerate-spec-anchor-audit.py.CONTRIBUTING.mdexplaining install (incl. optionalplaywright install --with-deps firefox chromium).tools/capture-gallery-hero.pydocstring to reference the new install contract.File-format choice
Picked
requirements-dev.txtoverpyproject.toml:__init__.py, no entry points.bash scripts/verify-plugin.sh,bash tests/e2e/mock-bootstrap.sh).package.json/pnpm-workspace.yaml(unchanged).Side-effect / breaking-change assessment
pip install -r requirements-dev.txton fresh clonepython3 -m venv /tmp/pf-i95-venv— installs cleanly;import playwright, jsonschemaboth succeed.tools/capture-gallery-hero.pyafter the pinbash scripts/verify-plugin.shbash tests/fixtures/security/verify-security.shbash tests/e2e/mock-bootstrap.sh {standard,pro,max}pip install -r requirements-dev.txtstep in this PR — current CI already installsjsonschemaambiently (or scripts degrade gracefully); changing CI is out of scope. P3 below.playwright>=1.54works on both; jsonschema is pure-Python.pip installis additive; no system-level changes.Codex review (1 pass)
P2 — pin exact versions (deferred)
Codex flagged
playwright>=1.54,<2.0/jsonschema>=4.0,<5.0as ranges, not exact pins, arguing reproducibility could drift across new Playwright 1.x releases (browser revisions ship with the package). Counter-arguments for keeping the ranges in this PR:capture-gallery-hero.pyis "same Playwright + same gallery.html → same PNG", not "stable across Playwright upgrades". The pin does not regress that contract.playwright==1.54.0) would force lockstep upgrades across contributors and require arequirements-dev.lockworkflow, which is out of scope for a 30-min PR.playwright installresolves to whatever's bundled), so an exact pip pin would be a half-measure withoutplaywright install --version.Suggested follow-up: introduce
pip-tools/uv pip compileand arequirements-dev.lockif/when the hero PNG drifts.P3 — CI integration (deferred)
Add
python3 -m pip install -r requirements-dev.txtto.github/workflows/ci.ymlbefore any Python-using job. Current CI works because GitHub-hosted runners shipjsonschema(or scripts no-op). Worth doing for explicit-contract reasons but not required to land this PR.Test plan
python3 -m venv /tmp/pf-i95-venv && /tmp/pf-i95-venv/bin/pip install -r requirements-dev.txtsucceedsbash scripts/verify-plugin.sh— Pass: 57 / Fail: 0bash tests/fixtures/security/verify-security.sh— all defenses holdingbash tests/e2e/mock-bootstrap.sh standard— PASSbash tests/e2e/mock-bootstrap.sh pro— PASSbash tests/e2e/mock-bootstrap.sh max— PASSRefs