fix(query-recipes): make project recipes visible to the CLI parser#93
Merged
Conversation
Project recipes at `<root>/.codemap/recipes/<id>.sql` were silently invisible to the CLI's `--recipe <id>` / `--recipes-json` / `--print-sql <id>` paths. The MCP and HTTP transports always worked — they bootstrap before reaching the loader. Root cause: `parseQueryRest` validates the recipe id by calling `getQueryRecipeSql(id)` (`cmd-query.ts:461`) BEFORE `runQueryCmd` calls `bootstrapCodemap` (`cmd-query.ts:823`). The loader's `getRegistry()` (`query-recipes.ts:80`) calls `getProjectRoot()`, which throws when `_config` is unset (pre-init), the throw is silently caught, and `projectDir` becomes `undefined`. Result: bundled-only registry at parse time, so the parser fails with `codemap: unknown recipe "<project-id>"` even when the recipe is present on disk. Fix: - Add `setQueryRecipesProjectRoot(root)` to `query-recipes.ts`. Caller-supplied root takes precedence over the runtime config; cache invalidates on change. - `main.ts` plumbs the resolved `root` (already returned by `parseBootstrapArgs`) into the override right after argv parse. Single source of truth: same root `bootstrapCodemap` would resolve — no parallel heuristic, no new env var. Adds: - `query-recipes.pre-bootstrap.test.ts` — 3 regression tests for the override-only discovery path (no `initCodemap` called). - `.codemap/recipes/src-deprecated.sql` (+ `.md`) — dogfood project recipe that scopes the bundled `deprecated-symbols` audit to `src/`. Both useful in its own right AND a permanent live regression case.
🦋 Changeset detectedLatest commit: 6ecb862 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR enables query recipe discovery during CLI argument parsing before ChangesPre-bootstrap project recipe resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/application/query-recipes.pre-bootstrap.test.ts (1)
31-33: ⚡ Quick winUse a per-test unique fixture recipe id to avoid future collisions.
Hardcoding
"team-fixture"can become flaky if a bundled recipe ever adds the same id; these assertions uselistQueryRecipeIds()and would then fail for the wrong reason.Suggested patch
describe("setQueryRecipesProjectRoot — pre-bootstrap CLI parse-phase path", () => { let projectRoot: string; + let fixtureId: string; beforeEach(() => { projectRoot = mkdtempSync(join(tmpdir(), "codemap-pre-bootstrap-")); + fixtureId = `team-fixture-${Date.now().toString(36)}`; mkdirSync(join(projectRoot, ".codemap", "recipes"), { recursive: true }); writeFileSync( - join(projectRoot, ".codemap", "recipes", "team-fixture.sql"), + join(projectRoot, ".codemap", "recipes", `${fixtureId}.sql`), "SELECT 1 AS ok\n", ); _resetRecipesCacheForTests(); }); @@ it("loads project recipes when only the override is set (no initCodemap)", () => { setQueryRecipesProjectRoot(projectRoot); const catalog = listQueryRecipeCatalog(); const projectIds = catalog .filter((c) => c.source === "project") .map((c) => c.id); - expect(projectIds).toContain("team-fixture"); - expect(getQueryRecipeSql("team-fixture")).toContain("SELECT 1"); + expect(projectIds).toContain(fixtureId); + expect(getQueryRecipeSql(fixtureId)).toContain("SELECT 1"); }); @@ it("clears project recipes when override is reset to undefined", () => { setQueryRecipesProjectRoot(projectRoot); - expect(listQueryRecipeIds()).toContain("team-fixture"); + expect(listQueryRecipeIds()).toContain(fixtureId); setQueryRecipesProjectRoot(undefined); - expect(listQueryRecipeIds()).not.toContain("team-fixture"); + expect(listQueryRecipeIds()).not.toContain(fixtureId); }); @@ it("re-setting the override to a new root invalidates the cache", () => { setQueryRecipesProjectRoot(projectRoot); - expect(listQueryRecipeIds()).toContain("team-fixture"); + expect(listQueryRecipeIds()).toContain(fixtureId); @@ const ids = listQueryRecipeIds(); expect(ids).toContain("other-fixture"); - expect(ids).not.toContain("team-fixture"); + expect(ids).not.toContain(fixtureId); } finally { rmSync(otherRoot, { recursive: true, force: true }); } }); });Also applies to: 49-57, 62-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/application/query-recipes.pre-bootstrap.test.ts` around lines 31 - 33, Replace the hardcoded recipe id "team-fixture" with a per-test unique id so assertions against listQueryRecipeIds() can't collide with bundled recipes; generate a unique recipeId (e.g., using crypto.randomUUID() or a timestamp suffix), use that variable in the writeFileSync path/join call and in any subsequent assertions or cleanup that reference the recipe id (functions/identifiers to update include writeFileSync, join/projectRoot path construction, and calls to listQueryRecipeIds()) so each test writes and asserts against its own distinct recipe id.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/application/query-recipes.pre-bootstrap.test.ts`:
- Around line 31-33: Replace the hardcoded recipe id "team-fixture" with a
per-test unique id so assertions against listQueryRecipeIds() can't collide with
bundled recipes; generate a unique recipeId (e.g., using crypto.randomUUID() or
a timestamp suffix), use that variable in the writeFileSync path/join call and
in any subsequent assertions or cleanup that reference the recipe id
(functions/identifiers to update include writeFileSync, join/projectRoot path
construction, and calls to listQueryRecipeIds()) so each test writes and asserts
against its own distinct recipe id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 385d6825-3d4b-446b-81d0-0df5ed2a3469
📒 Files selected for processing (6)
.changeset/project-recipes-cli-pre-bootstrap.md.codemap/recipes/src-deprecated.md.codemap/recipes/src-deprecated.sqlsrc/application/query-recipes.pre-bootstrap.test.tssrc/application/query-recipes.tssrc/cli/main.ts
Hardcoded `team-fixture` / `other-fixture` recipe ids could collide with a future bundled recipe of the same name, making the assertions fail for the wrong reason. Suffix both with a per-test timestamp+random so each run uses distinct ids. Addresses CodeRabbit nitpick on #93.
Contributor
Author
|
Applied in |
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
1 participant
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.
Summary
Project recipes at
<root>/.codemap/recipes/<id>.sql(an advertised feature since 0.6.x) were silently invisible to the CLI's--recipe <id>/--recipes-json/--print-sql <id>paths. The MCP and HTTP transports worked correctly throughout — they bootstrap before reaching the loader.Root cause
parseQueryRestvalidates the recipe id by callinggetQueryRecipeSql(id)(cmd-query.ts:461) BEFORErunQueryCmdcallsbootstrapCodemap(cmd-query.ts:823). The loader'sgetRegistry()(query-recipes.ts:80) callsgetProjectRoot(), which throws when_configis unset (pre-init); the throw was silently caught,projectDirbecameundefined, and the registry fell back to bundled-only. Result: parser fails withcodemap: unknown recipe "<project-id>"even when the recipe is present on disk.Fix
Two-line wiring:
src/application/query-recipes.ts— newsetQueryRecipesProjectRoot(root)API. Caller-supplied root takes precedence over the runtime config (which isn't initialised yet during argv parse). Cache invalidates on change.src/cli/main.ts— calls the setter once with the already-resolvedrootfromparseBootstrapArgs, right after argv parse.Single source of truth: the override is the same root
bootstrapCodemapwould resolve to. No parallel walk-up heuristic, no new env var.Empirical proof in this repo
Added a real project recipe at
.codemap/recipes/src-deprecated.sqlthat scopes the bundleddeprecated-symbolsaudit tosrc/only — useful for codemap's own deprecation lifecycle.Before this PR:
After:
Regression tests
src/application/query-recipes.pre-bootstrap.test.ts— 3 tests exercising the override-only path (noinitCodemapcalled):undefinedThe dogfood recipe itself is a permanent live regression case — if this bug recurs,
bun src/index.ts query --recipe src-deprecatedfrom this repo will fail.Migration
None required. Consumers with project recipes authored on 0.6.x–0.7.2 had them working via MCP / HTTP throughout. After upgrading, the CLI auto-picks them up.
Test plan
bun test— 958 pass, 0 fail (3 new + 955 existing).bun run typecheckclean.bun run lintclean.bun run format:checkclean.bun src/index.ts query --recipe src-deprecated --jsonreturns rows.Summary by CodeRabbit
New Features
Tests