feat: generate PARAMETERS from the tool registry - #1478
Conversation
The man-page generator now fully owns the `## PARAMETERS` block on every section-3 tool page: every schema property is rendered, required first and then optional, in schema order, with required/default info rendered deterministically and the whole block replaced on regeneration. `$ref` enum members resolve to their underlying JSON type so nullable enums read `string | null` rather than a bare `null`. Parameter-specific facts that previously lived only in the hand-curated PARAMETERS prose of six pages (write-note, search-notes, build-context, edit-note, read-note, recent-activity) — MCP aliases, CLI flags, precedence rules, config fallbacks, accepted forms, output_format semantics — are migrated into the tools' Field/param descriptions so the generated block carries them. Broader explanations stay in the curated DESCRIPTION / GOTCHAS sections. The live-registry drift test now checks every generated page against its actual tool schema, so a schema change without `just man-regen` fails CI. Refs basicmachines-co#610 Signed-off-by: FBISiri <masteragentsiri@gmail.com>
![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: b395d7ce1b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if schema.get("properties"): | ||
| updated = replace_parameters(updated, render_parameters(page.tool, schema)) |
There was a problem hiding this comment.
When a registered tool loses its final parameter, this condition skips PARAMETERS processing entirely, so the previously generated block remains in the bundled page and continues advertising removed arguments. The drift test repeats the same properties check, so both regeneration and CI report success; remove any existing PARAMETERS section for an empty schema and test that transition.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — the if schema.get("properties") guard meant a tool that dropped its last parameter would keep its old block forever, and the drift test had the same blind spot.
Fixed in 7307f74: added remove_parameters() and pulled the per-page update into a pure regenerate_page() so an empty schema now strips any existing PARAMETERS section. The drift test asserts the section is absent for parameterless tools (no more silent continue), and there's a transition test covering a page that carries a stale block + {} / {"properties": {}} schemas. Regen is a no-op on the current pages (diagnostics still has no block, cloud_info untouched); man module stays at 100% coverage.
When a registered tool loses its last parameter, the generator now strips any existing PARAMETERS section instead of skipping it, so a page can no longer keep advertising removed arguments. The drift test asserts the section is absent for parameterless tools rather than silently continuing. Addresses the Codex review on PR basicmachines-co#1478. Refs basicmachines-co#610 Signed-off-by: FBISiri <masteragentsiri@gmail.com>
scripts/ is not a package and CI runs pytest with --import-mode=importlib, so `from scripts.update_man_pages import ...` failed collection on every unit-test job (ModuleNotFoundError: No module named 'scripts'). Load the script with importlib.util.spec_from_file_location, matching tests/test_update_versions.py. Refs basicmachines-co#610 Signed-off-by: FBISiri <masteragentsiri@gmail.com>
phernandez
changed the title
|
@codex-review |
Review ResultNo findings on the latest reviewed head ( The earlier stale-
No files were modified during this report-only review, so no commit or follow-up pull request was created. Verification
|
2 participants
@phernandez — this implements Option A from #610: the registry generator fully owns
## PARAMETERSon every section-3 man page, and the parameter-specific facts that used to live only in hand-curated prose now live in the tools' schema descriptions.Refs #610
What the generator owns
Scope from the issue thread, and how each point is met:
render_parameterswalks the tool's input schemapropertiesin declaration order and partitions byrequired; nothing is filtered.- **name** (type, required|optional[, default: X]) — description. The description is the schema description with internal whitespace collapsed; the renderer never reflows or reinterprets prose.$refenum members resolve to their underlying JSON type so a nullable enum readsstring | nullrather than a barenull.replace_parametersswaps the complete## PARAMETERSblock (or inserts one) onjust man-regen;scripts/update_man_pages.pyis idempotent (second run: "all pages already match the registry").test_section_3_parameters_is_exactly_the_registry_renderingiterates all bundled section-3 pages and asserts byte-equality with the live registry rendering (tools with no parameters get no section). A schema change without regeneration fails here and points at the page.$refenum resolution, and the drift check; no parameterised formatting matrix.Migration of curated PARAMETERS (6 pages → schema descriptions)
Parameter-specific facts moved into
Field/Args descriptions insrc/basic_memory/mcp/tools/*.py; broader explanations stayed in (or moved to) the pages' DESCRIPTION / GOTCHAS sections.directoryaccepts MCP aliasesfolder/dir/path, CLI flag--folder;note_typeis CLI--typeand atype:in content frontmatter takes precedence;metadatanot available from CLI;project_idwins overproject; project resolution order; jsonoutput_formatreturns conflicts asaction: "conflict"page/page_sizealiases (page_number;limit/per_page); metadata filter semantics (integer values match integer YAML fields); temporal aliases (as_of/valid_on,overlaps/valid_during);kindalias and accepted valuesurlacceptsmemory://URI or bare permalink path; project resolution/fallbackidentifieris a CLI positional argument, not a flag; accepted formsidentifieraccepted forms (memory:// URL, permalink, title, search text) and CLI positional; pagination aliases and exhaustion semantics;include_frontmatterCLI flag--frontmatter(--include-frontmatterdeprecated alias)timeframealiases (since/time_range/lookback); project resolution: active/default project first, discovery mode only when neither resolvesThe remaining 15 pages were spot-checked for curated-looking content; nothing was dropped.
Verification
uv run python scripts/update_man_pages.py×2 — idempotent, no diff on second runuv run ruff check --fix --unsafe-fixes src tests test-int && uv run ruff format .— cleanuv run ty check src tests test-int— only the pre-existing diagnostics also present on a cleanmainBASIC_MEMORY_ENV=test uv run pytest tests/test_man_pages.py --cov=basic_memory.man— 36 passed,basic_memory/man/__init__.py100% coveragetest_claude_issue_triage.py, a chatgpt importer date test) and unrelated to this changeSigned-off (DCO). No tool behaviour changes — only descriptions.