Add minimal root npm wrapper#86
Conversation
✅ Deploy Preview for comptext-v7 canceled.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request introduces a root-level package.json that serves as a command wrapper, delegating validation tasks to the dashboard and showcase applications via npm --prefix and running Python tests with pytest. Documentation across the repository has been updated to reflect this new workflow. Feedback includes a suggestion to centralize command documentation to avoid duplication, an optimization for the check script to provide faster feedback by reordering tasks, and the removal of redundant assertions in the repository layout tests.
| Root wrapper checks: | ||
|
|
||
| ```bash | ||
| npm run layout | ||
| npm run typecheck | ||
| npm run validate | ||
| npm run build | ||
| npm test | ||
| npm run check | ||
| ``` | ||
|
|
There was a problem hiding this comment.
To avoid duplicating documentation, consider removing this list of commands from README.md. The preceding sentence already directs users to docs/validation.md for validation commands, and that file now contains a comprehensive explanation of these root wrapper scripts. Centralizing this information makes it easier to maintain.
| "test:core": "pytest tests/test_core_foundation_ts.py -q", | ||
| "test:replay": "pytest tests/test_paper_replay_bench.py tests/test_agent_trace_replay.py tests/test_replay_continuity.py -q", | ||
| "layout": "python scripts/check_repo_layout.py", | ||
| "check": "npm run layout && npm run typecheck && npm run validate && npm run build && npm run test" |
There was a problem hiding this comment.
The Python tests run by npm test are likely independent of the frontend typecheck, validate, and build steps. Running the Python tests earlier in the check script can provide faster feedback if they fail, as you won't have to wait for the frontend checks to complete.
| "check": "npm run layout && npm run typecheck && npm run validate && npm run build && npm run test" | |
| "check": "npm run layout && npm run test && npm run typecheck && npm run validate && npm run build" |
| assert (REPO_ROOT / "package.json").is_file() | ||
| assert (REPO_ROOT / "dashboard" / "app" / "package.json").is_file() | ||
| assert (REPO_ROOT / "showcase" / "app" / "package.json").is_file() |
There was a problem hiding this comment.
These assertions are redundant. The check_repo_layout() function already verifies the existence of these files. If any of them are missing, check_repo_layout() will return a list of errors, and the subsequent assert check_repo_layout() == [] will fail with a more descriptive message about what the function under test reported. It's better to rely on the function's output for the assertion.
Motivation
package.jsonso common root npm commands work and delegate to the real app directories without changing dependency management.unexpected root package.jsonbehavior.Description
package.jsonat the repository root namedcomptextv7-rootwith scripts that delegate to the apps usingnpm --prefixand run Python tests withpytest.scripts/check_repo_layout.pyto include the rootpackage.jsoninREQUIRED_FILESand removed the old error that flagged an existing root package as unexpected.tests/test_check_repo_layout.pyto assert the presence of the root and apppackage.jsonfiles before assertingcheck_repo_layout()returns[].docs/validation.md,README.md,docs/AGENT_WORKFLOW.md, anddocs/BENCHMARK_INTEGRATION.mdto document the root wrapper commands and clarify that app dependencies remain indashboard/appandshowcase/app.Testing
python scripts/check_repo_layout.py, which printedrepository layout OK(success).pytest tests/test_check_repo_layout.py -q, which passed (1 passed).pytest -q, which completed successfully (52 passed).npm run layout,npm run typecheck,npm run validate,npm run build,npm test, andnpm run check, and each command completed successfully (builds and typechecks for both apps completed and Python tests passed).Codex Task