test: fingerprint — file-based project detection rules#479
test: fingerprint — file-based project detection rules#479anandgupta42 wants to merge 1 commit intomainfrom
Conversation
Fingerprint.detect() drives skill selection and system prompt generation but had zero tests verifying that specific project markers (dbt_project.yml, profiles.yml, .sqlfluff, airflow.cfg, databricks.yml) produce the correct tags. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughA new Bun test suite for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/opencode/test/altimate/fingerprint-detect.test.ts (1)
71-79: Add a cache-key regression case forrootchanges.The cache implementation uses only
cwdas the cache key (line 34) but thedetect()method accepts an optionalrootparameter that affects detection (line 40). Ifdetect()is called with the samecwdbut differentrootvalues, the second call returns a stale cached result from the first call. Add a test to guard against this regression:Suggested test addition
test("caches result for same cwd", async () => { await using tmp = await tmpdir() await fs.writeFile(path.join(tmp.path, "dbt_project.yml"), "name: test\n") const r1 = await Fingerprint.detect(tmp.path) // Remove the file — cached result should still have dbt await fs.rm(path.join(tmp.path, "dbt_project.yml")) const r2 = await Fingerprint.detect(tmp.path) expect(r1).toBe(r2) // Same reference (cached) }) + + test("does not return stale tags when root changes for same cwd", async () => { + await using tmp = await tmpdir() + const subdir = path.join(tmp.path, "models") + await fs.mkdir(subdir) + await fs.writeFile(path.join(subdir, "model.sql"), "SELECT 1") + + const first = await Fingerprint.detect(subdir) + expect(first.tags).toContain("sql") + expect(first.tags).not.toContain("dbt") + + await fs.writeFile(path.join(tmp.path, "dbt_project.yml"), "name: test\n") + const second = await Fingerprint.detect(subdir, tmp.path) + expect(second.tags).toContain("dbt") + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/altimate/fingerprint-detect.test.ts` around lines 71 - 79, The cache currently keys only by cwd causing stale results when detect is called with different root values; update the Fingerprint.detect caching to include the optional root in the cache key (e.g., combine cwd and root or use a nested Map keyed by cwd then root) so cached results are distinct per (cwd, root) pair, and add the regression test that calls Fingerprint.detect(tmp.path) then Fingerprint.detect(tmp.path, differentRoot) to assert they are not the same cached reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/opencode/test/altimate/fingerprint-detect.test.ts`:
- Around line 71-79: The cache currently keys only by cwd causing stale results
when detect is called with different root values; update the Fingerprint.detect
caching to include the optional root in the cache key (e.g., combine cwd and
root or use a nested Map keyed by cwd then root) so cached results are distinct
per (cwd, root) pair, and add the regression test that calls
Fingerprint.detect(tmp.path) then Fingerprint.detect(tmp.path, differentRoot) to
assert they are not the same cached reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b54b07d1-225f-45c8-b527-b930ba635265
📒 Files selected for processing (1)
packages/opencode/test/altimate/fingerprint-detect.test.ts
Consolidates tests from PRs #440, #441, #479, #483, #484, #485, #490, #491, #492, #495, #496. Deduplicates overlapping MongoDB normalize tests (3 PRs → 1), `keybind` tests (2 PRs → 1), and `connections.test.ts` additions (4 PRs → 1 merged file). New test files: - `sql-analyze-tool.test.ts` — `SqlAnalyzeTool` formatting (6 tests) - `sql-analyze-format.test.ts` — `sql.analyze` success semantics + dispatcher (5 tests) - `builtin-commands.test.ts` — altimate builtin command registration (10 tests) - `hints-discover-mcps.test.ts` — `Command.hints()` + discover-and-add-mcps (11 tests) - `fingerprint-detect.test.ts` — file-based project detection rules (11 tests) - `dbt-lineage-helpers.test.ts` — dbt lineage helper functions (13 tests) - `registry-env-loading.test.ts` — `ALTIMATE_CODE_CONN_*` env var loading (8 tests) - `warehouse-telemetry.test.ts` — MongoDB auth detection telemetry (4 tests) - `bus-event.test.ts` — bus event registry payloads (5 tests) - `git.test.ts` — git utility functions (5 tests) - `keybind.test.ts` — keybind parsing, matching, `fromParsedKey` (22 tests) Merged into existing files: - `connections.test.ts` — `loadFromEnv` (5), `detectAuthMethod` (14), credential stripping (1), `containerToConfig` (1), dbt profiles edge cases (3) - `driver-normalize.test.ts` — MongoDB alias resolution (13 tests) Fixes: - `fixture.ts` — disable `commit.gpgsign` in test tmpdir to prevent CI failures - `hints-discover-mcps.test.ts` — corrected sort order expectation - `registry-env-loading.test.ts` — fixed flaky assertion that assumed isolated env Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Consolidated into #498. Tests deduplicated and merged into a single PR. |
* test: consolidate 11 test PRs into single suite — 305 new tests Consolidates tests from PRs #440, #441, #479, #483, #484, #485, #490, #491, #492, #495, #496. Deduplicates overlapping MongoDB normalize tests (3 PRs → 1), `keybind` tests (2 PRs → 1), and `connections.test.ts` additions (4 PRs → 1 merged file). New test files: - `sql-analyze-tool.test.ts` — `SqlAnalyzeTool` formatting (6 tests) - `sql-analyze-format.test.ts` — `sql.analyze` success semantics + dispatcher (5 tests) - `builtin-commands.test.ts` — altimate builtin command registration (10 tests) - `hints-discover-mcps.test.ts` — `Command.hints()` + discover-and-add-mcps (11 tests) - `fingerprint-detect.test.ts` — file-based project detection rules (11 tests) - `dbt-lineage-helpers.test.ts` — dbt lineage helper functions (13 tests) - `registry-env-loading.test.ts` — `ALTIMATE_CODE_CONN_*` env var loading (8 tests) - `warehouse-telemetry.test.ts` — MongoDB auth detection telemetry (4 tests) - `bus-event.test.ts` — bus event registry payloads (5 tests) - `git.test.ts` — git utility functions (5 tests) - `keybind.test.ts` — keybind parsing, matching, `fromParsedKey` (22 tests) Merged into existing files: - `connections.test.ts` — `loadFromEnv` (5), `detectAuthMethod` (14), credential stripping (1), `containerToConfig` (1), dbt profiles edge cases (3) - `driver-normalize.test.ts` — MongoDB alias resolution (13 tests) Fixes: - `fixture.ts` — disable `commit.gpgsign` in test tmpdir to prevent CI failures - `hints-discover-mcps.test.ts` — corrected sort order expectation - `registry-env-loading.test.ts` — fixed flaky assertion that assumed isolated env Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address CodeRabbit review comments - `bus-event.test.ts`: move `BusEvent.define` into tests instead of module scope - `builtin-commands.test.ts`: fix misleading "lexicographic" → "numeric" sort label - `git.test.ts`: use `tmpdir({ git: true })` fixture instead of manual `git init` - `registry-env-loading.test.ts`: remove unused `fs`, `os`, `path` imports Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
What does this PR do?
1.
Fingerprint.detect—src/altimate/fingerprint/index.ts(11 new tests)This module detects project type by scanning for well-known files (
dbt_project.yml,profiles.yml,.sqlfluff,airflow.cfg,databricks.yml,dags/directory,*.sqlfiles). It feeds intoselectSkillsWithLLM()andSystemPrompt.skills()— if fingerprinting is wrong, users get irrelevant skills in their session.The existing
fingerprint.test.tsonly smoke-tested against the real project root directory. None of its tests verified that a specific file produces a specific tag. A regression in any detection rule (e.g., changing theprofiles.ymlregex, renaming a marker file check) would pass all existing tests silently.New coverage includes:
cwdreturns cached reference; file removal after caching doesn't change resultcwdandrootcontain the same marker, tags appear only once (exercises theSetdedup logic)detect(subdir, root)picks up markers from both directories (real scenario: user opens a subdirectory of a dbt monorepo)Type of change
Issue for this PR
N/A — proactive test coverage for untested project detection logic
How did you verify your code works?
Checklist
https://claude.ai/code/session_015TPuQ2dEJD2CYoHF3LTQzD
Summary by CodeRabbit