Revert "use builtin vitest file snapshots for OpenAPI json file" by robert-inkeep · Pull Request #1813 · inkeep/agents

robert-inkeep · 2026-02-09T04:54:20Z

Reverts #1801

This reverts commit bcb3013.

changeset-bot · 2026-02-09T04:54:25Z

⚠️ No Changeset found

Latest commit: 5b373a4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel · 2026-02-09T04:54:25Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agents-api	Ready	Preview, Comment	Feb 9, 2026 4:56am
agents-manage-ui	Error		Feb 9, 2026 4:56am

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
agents-docs	Skipped		Feb 9, 2026 4:56am

claude

PR Review Summary

4 Key Findings | Risk: Medium

🟠⚠️ Major (4) 🟠⚠️

🟠 1) evaluateConversation.ts:11-12, runDatasetItem.ts:18-19, Agent.ts:44-45, ref.ts:14 Revert introduces import path changes that don't match pre-#1801 state

Issue: This revert changes several files from relative imports to src/ prefix style, but the original files (before PR #1801) used relative imports. This means the revert is not restoring the codebase to its pre-#1801 state — it's introducing new import style changes.

Why: A revert PR should restore the exact previous state. Introducing different import styles:

Creates inconsistency within the same files (e.g., evaluateConversation.ts now has src/data/db on lines 11-12 but ../../../../data/db/runDbClient on line 13)
Deviates from the established codebase convention (>95% of files use relative imports)
The src/ prefix is NOT a TypeScript path alias — it only works via baseUrl: ".", making it less explicit

Fix: See inline comments with 1-click suggestions to restore the correct relative import paths.

Refs:

💭 Consider (2) 💭

💭 1) openapi.test.ts:27-140 Custom JSON diff implementation has no test coverage
Issue + Why: The custom diff logic (computeJsonDiff, formatDiffOutput, summarizeValue) provides better developer experience for OpenAPI changes than Vitest's built-in snapshots, but has no unit test coverage. If this logic has bugs, snapshot test failures could be misleading or miss real issues.
Suggestion: Consider adding unit tests for the diff utilities in a future PR. This is pre-existing code being restored, so not blocking for this revert.

💭 2) openapi.test.ts:262-268 Custom snapshot update mechanism differs from Vitest conventions
Issue + Why: The UPDATE_OPENAPI_SNAPSHOT=true environment variable approach differs from Vitest's standard -u flag. New contributors may try pnpm test -u and be confused when it doesn't update the snapshot.
Suggestion: Ensure this is documented in AGENTS.md or the package README.

📌 Inline Comments (4)

🟠 evaluateConversation.ts:11-12 Import path style doesn't match pre-#1801 state
🟠 runDatasetItem.ts:18-19 Import path style doesn't match pre-#1801 state
🟠 Agent.ts:44-45 Import path style doesn't match pre-#1801 state
🟠 ref.ts:14 Import path style doesn't match pre-#1801 state

💡 APPROVE WITH SUGGESTIONS

Summary: The OpenAPI test revert itself looks fine and restores the custom diff logic that provides better developer feedback for API contract changes. However, the revert is not clean — it introduces src/ prefix imports that didn't exist before PR #1801. I've provided 1-click fix suggestions in the inline comments. Once those imports are corrected to use relative paths (matching the pre-#1801 state), this should be good to merge.

Discarded (1)

Location	Issue	Reason Discarded
`stream-helpers.test.ts:23-27`	Mock type changed from `Mock<...>` to `ReturnType<typeof vi.fn>`	Cosmetic TypeScript change with no behavioral impact

Reviewer Stats

Reviewer	Returned	Inline Comments	Discarded
`pr-review-tests`	3	0	1
`pr-review-consistency`	1	4	0
`pr-review-standards`	0	0	0
Total	4	4	1

Note: The 4 inline comments cover the same issue (import path inconsistency) across 4 files — they were consolidated from pr-review-consistency's multi-file finding.

claude · 2026-02-09T05:01:01Z

agents-api/src/domains/evals/workflow/functions/evaluateConversation.ts

+import { manageDbClient } from 'src/data/db';
+import manageDbPool from 'src/data/db/manageDbPool';


MAJOR Import path style introduced by revert doesn't match pre-#1801 state

This file used relative imports (../../../../data/db) before PR #1801 was merged. The revert should restore those exact imports rather than introducing the src/ prefix style. Currently this creates inconsistency with line 13 which still uses relative imports.

Suggested change

import { manageDbClient } from 'src/data/db';

import manageDbPool from 'src/data/db/manageDbPool';

import { manageDbClient } from '../../../../data/db';

import manageDbPool from '../../../../data/db/manageDbPool';

claude · 2026-02-09T05:01:06Z

agents-api/src/domains/evals/workflow/functions/runDatasetItem.ts

+import { manageDbClient } from 'src/data/db';
+import manageDbPool from 'src/data/db/manageDbPool';


MAJOR Import path style introduced by revert doesn't match pre-#1801 state

Same issue as evaluateConversation.ts — this file used relative imports before PR #1801. The revert should restore those exact imports.

Suggested change

import { manageDbClient } from 'src/data/db';

import manageDbPool from 'src/data/db/manageDbPool';

import { manageDbClient } from '../../../../data/db';

import manageDbPool from '../../../../data/db/manageDbPool';

claude · 2026-02-09T05:01:12Z

agents-api/src/domains/run/agents/Agent.ts

+import manageDbPool from 'src/data/db/manageDbPool';
+import { env } from 'src/env';


MAJOR Import path style introduced by revert doesn't match pre-#1801 state

Agent.ts used relative imports (../../../data/db/manageDbPool and ../../../env) before PR #1801. The revert introduces src/ prefix style which creates inconsistency with line 46.

Suggested change

import manageDbPool from 'src/data/db/manageDbPool';

import { env } from 'src/env';

import manageDbPool from '../../../data/db/manageDbPool';

import { env } from '../../../env';

claude · 2026-02-09T05:01:17Z

agents-api/src/middleware/ref.ts

 } from '@inkeep/agents-core';
 import type { Context, Next } from 'hono';
-import { manageDbClient } from '../data/db';
+import { manageDbClient } from 'src/data/db';


MAJOR Import path style introduced by revert doesn't match pre-#1801 state

ref.ts used the relative import ../data/db before PR #1801.

Suggested change

import { manageDbClient } from 'src/data/db';

import { manageDbClient } from '../data/db';

Revert "use builtin vitest file snapshots for OpenAPI json file (#1801)"

5b373a4

This reverts commit bcb3013.

vercel bot temporarily deployed to Preview – agents-docs February 9, 2026 04:54 Inactive

vercel bot had a problem deploying to Preview – agents-manage-ui February 9, 2026 04:56 Failure

vercel bot deployed to Preview – agents-api February 9, 2026 04:56 View deployment

claude bot reviewed Feb 9, 2026

View reviewed changes

github-actions bot deleted a comment from claude bot Feb 9, 2026

robert-inkeep closed this Feb 9, 2026

robert-inkeep deleted the revert-1801-use-builtin-vitest-snapshots-for-openapi branch February 9, 2026 05:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "use builtin vitest file snapshots for OpenAPI json file"#1813

Revert "use builtin vitest file snapshots for OpenAPI json file"#1813
robert-inkeep wants to merge 1 commit intomainfrom
revert-1801-use-builtin-vitest-snapshots-for-openapi

robert-inkeep commented Feb 9, 2026

Uh oh!

changeset-bot bot commented Feb 9, 2026

Uh oh!

vercel bot commented Feb 9, 2026 •

edited

Loading

Uh oh!

claude bot left a comment

Uh oh!

claude bot Feb 9, 2026

Uh oh!

claude bot Feb 9, 2026

Uh oh!

claude bot Feb 9, 2026

Uh oh!

claude bot Feb 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		import { manageDbClient } from 'src/data/db';
		import manageDbPool from 'src/data/db/manageDbPool';

		import manageDbPool from 'src/data/db/manageDbPool';
		import { env } from 'src/env';

	import { manageDbClient } from 'src/data/db';
	import { manageDbClient } from '../data/db';

Conversation

robert-inkeep commented Feb 9, 2026

Uh oh!

changeset-bot bot commented Feb 9, 2026

⚠️ No Changeset found

Uh oh!

vercel bot commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

PR Review Summary

🟠⚠️ Major (4) 🟠⚠️

💭 Consider (2) 💭

📌 Inline Comments (4)

💡 APPROVE WITH SUGGESTIONS

Uh oh!

claude bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel bot commented Feb 9, 2026 •

edited

Loading