Repository navigation
fix(cli): make template directives true on the file's language and tests - #38
Merged
Merged
Conversation
Problem: an audit of 20 hosted maps found 11 false directives. 2 of them named a test that does not test the file. On jamiepine/voicebox the directive for backend/backends/__init__.py named backend/tests/__init__.py, a 7 line package file. On vitejs/vite the directive for packages/vite/src/node/server/index.ts named packages/plugin-legacy/src/__tests__/index.spec.ts. That spec imports '../index', and the resolver maps it correctly to plugin-legacy's own src/index.ts. The name pass caused both: it matched the stem "index" or "__init__" for any test under the same top folder, packages/ or backend/. Fix: a covering test must run a test case. __init__.py never counts, and a Python or Go file counts only with a test function. The name pass now needs the same package (the folder of the nearest manifest) and the same language family. A stem that 2 ranked files of 1 package share matches only the file beside the test or beside its test folder. The pass also records which tests only the name found. The spec states the rule. cli/test/directive-truth.test.js rebuilds both cases; 8 Python fixtures in classify.test.js gain the test function a real pytest file has. cd cli && npm test: 789 tests, exit 0. The signal goldens did not change. Intent: a directive names only tests that exercise the file. Assisted-by: Claude
Problem: an audit of 20 hosted maps found 11 false directives. 4 of them named a test command for the wrong language, because template prose used 1 repository wide command. hkuds/nanobot's webui/src/lib/types.ts got pytest from the root pyproject.toml, though webui/package.json runs vitest run. 3 Python files of debpalash/voicestudio's backend got npm test, which runs only the Electron suite, though pytest runs their tests. Fix: testCommandFor in signals.js gives each file the nearest test setup of its language: the nearest package.json with a test script for JavaScript and TypeScript, the nearest pytest.ini, conftest.py or pytest section for Python, go.mod for Go, Cargo.toml for Rust. A setup in a subfolder is named: "run the full test suite in `webui` with `npm test`". When no setup fits, the directive names no command. check and the MCP server keep detectTestCommand. 4 tests in cli/test/directive-truth.test.js rebuild the 2 cases and cover each language. cd cli && npm test: 792 tests, exit 0; truth.test.js passes. Intent: a "run the full test suite" directive names a command that runs the tests of that file's language. Assisted-by: Claude
…easured Problem: an audit of 20 hosted maps found 11 false directives. 5 of them, all on chartjs/chart.js, said "because no test covers it" for src/core/core.controller.js, core.scale.js, core.datasetController.js, core.layouts.js and src/plugins/plugin.tooltip.js. Each has a test: test/specs/core.scale.tests.js and the others reach the file through the global Chart, with no import. The name pass missed them for 2 reasons: the .tests.js suffix kept the stem from matching, and test/ and src/ are different top folders. The previous commit made the package the scope. The prose then claimed more than the tool measured. Fix: the name pass strips .tests and .specs, so the stem of core.scale.tests.js is core.scale. The signals carry name_tests, the covering tests that only the file name found. When every covering test is such a match, the paragraph says "No test file imports it, and `test/specs/core.scale.tests.js` matches its name", and the directive says "run `test/specs/core.scale.tests.js`, which matches it by name". With no test at all, the prose says "no test file imports it or matches its name", which is what was measured. The 2 signal goldens gain the empty name_tests key. cli/test/directive-truth.test.js rebuilds the chart.js layout. cd cli && npm test: 793 tests, exit 0; truth.test.js passes. Intent: a directive never says no test covers a file that has a test, and it states how each named test was found. Assisted-by: Claude
Problem: the 3 fixes on this branch change the directives that template prose writes, and CHANGELOG.md had no entry for them. An audit of 20 hosted maps found 11 false directives among 95. Fix: 1 Unreleased entry states the audit, the 3 causes with counts, the new rules and wording, and the result: on the 5 audited repositories at the audited commits, all 11 directives now name a true command or test. Intent: record why the directive wording and the covering test rules changed. Assisted-by: Claude
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
An audit of 20 hosted maps checked 95 "Before editing this file" directives of template prose against the repositories. 11 were false. They came from 3 causes in the CLI.
detectTestCommand. hkuds/nanobot'swebui/src/lib/types.tsgotpytestfrom the rootpyproject.toml, thoughwebui/package.jsonrunsvitest run. 3 Python files in debpalash/voicestudio'sbackend/services/gotnpm test, which runs only the Electron suite. Fix:testCommandForincli/src/signals.jstakes the nearest test setup of the file's language. That is the nearestpackage.jsonwith a test script for JavaScript and TypeScript, the nearestpytest.ini,conftest.pyor pytest section for Python,go.modfor Go andCargo.tomlfor Rust. A setup in a subfolder is named ("run the full test suite inwebuiwithnpm test"). When no setup fits, the directive names no command.checkand the MCP server still usedetectTestCommand.backend/tests/__init__.py. vitejs/vite namedpackages/plugin-legacy/src/__tests__/index.spec.tsforpackages/vite/src/node/server/index.ts. The import resolver is correct: that spec's'../index'resolves to plugin-legacy's ownsrc/index.ts. The name pass incli/src/graph.jscaused both cases. It matched the stemsindexand__init__for any test under the same top folder (packages/,backend/). Fix: a covering test must run a test case.__init__.pynever counts, and a Python or Go file counts only with a test function. A name match needs the same package (the folder of the nearest manifest) and the same language family. A stem that 2 ranked files of 1 package share matches only the file beside the test or beside its test folder.test/specs/core.scale.tests.jsand 4 more specs test their files through the globalChart, with no import. The.tests.jssuffix kept the stem from matching, and the old top folder rule kepttest/apart fromsrc/. Fix: the stem drops.testsand.specs, and the signals carryname_tests, the tests that only the name found. The paragraph and the directive now state how each test was found: "runtest/specs/core.scale.tests.js, which matches it by name". When there is no test at all, the prose says "no test file imports it or matches its name", because that is what the tool measured.content/spec.mdstates the new covering test rule. Its partnerscli/README.md,skill/SKILL.tmpl.mdandcontent/index.htmldo not describe the stem rule, so they needed no change.cli/binneeded no change for thecli/binandcli/srcseam. The.testsstem change is in the first commit (graph.js), although the third commit message describes it as part of the chart.js fix.How it was tested
cd cli && npm test: 793 tests, exit 0.cd cli && node --test test/truth.test.js: 45 pass, exit 0.npm run buildat the root: exit 0.cli/test/directive-truth.test.jsadds 5 tests. They rebuild the nanobot, voicestudio, voicebox, vite and chart.js cases in small file trees, and they covertestCommandForper language. The 4 case tests failed onorigin/mainwith the reported wrong output. The vite assertion failed on its own against the originalgraph.js.cli/test/classify.test.jsgain thedef test_function that a real pytest file has. The 2 signal goldens gain the emptyname_testskey. No other golden value changed.node cli/bin/complex-md.js generate --template --no-wire --yes. I used no provider key and no--via, so no model ran. Each clone was a fetch of the audited sha with 2001 commits of history, and I deleted the clones after. The 11 directives before and after:webui/src/lib/types.tspytestwebuiwithnpm testbackend/backends/__init__.pybackend/tests/__init__.py,test_designed_profiles.pyandtest_kokoro_trim.pytest_designed_profiles.py,test_kokoro_trim.pyandtest_models_load_by_name.pypackages/vite/src/node/server/index.tsplugin-legacy/src/__tests__/index.spec.ts,config.spec.tsanddev.spec.tsconfig.spec.ts,dev.spec.tsandenvironment.spec.tsbackend/services/model_manager.pynpm testpytestbackend/services/tts_backend.pynpm testpytestbackend/services/asr_backend.pynpm testpytestsrc/core/core.controller.jstest/specs/core.controller.tests.js, which matches it by namesrc/core/core.scale.jstest/specs/core.scale.tests.js, which matches it by namesrc/core/core.datasetController.jstest/specs/core.datasetController.tests.js, which matches it by namesrc/core/core.layouts.jstest/specs/core.layouts.tests.js, which matches it by namesrc/plugins/plugin.tooltip.jstest/specs/plugin.tooltip.tests.js, which matches it by nametest_models_load_by_name.pyimportsbackend.backendsand defines 2 tests.environment.spec.tsimportscreateServerfrom../server. The 5 chart.js specs exist undertest/specs/. voicestudio's rootpytest(testpaths = ["tests"]) runstests/test_model_manager_preload.pyand other tests of these modules. Itsbackend/tests/folder runs as a separate pytest session in CI.electronwithnpm test", the suite that the root script delegates to. 2 voicebox app files now say "no test file imports it or matches its name"; that app has no test files.AI assistance
Assisted-by:or aCo-Authored-By:trailer.Claude (Claude Code) wrote all of this change: the code, the tests, the spec and CHANGELOG text, the check on the 5 repositories and this description. Each commit carries
Assisted-by: Claude. No person has read the whole diff yet.🤖 Generated with Claude Code