refactor: finish the v2→v1 collapse — remove the dead "API v2" parallel stack#228
Merged
Conversation
…emantic-search plumbing The v2→v1 collapse left three route trees under routes/v2/ that were never mounted in index.ts and are referenced by nothing: - routes/v2/conversations/ (crud, search, messages, semantic) — duplicates the live routes/conversations/ tree - routes/v2/keys/ — duplicates routes/v1-keys.ts - routes/v2/analytics/ — superseded by routes/analytics.ts Deleting the semantic/messages routes orphaned services/embeddings.ts (its only importers), and the SemanticSearch*/ContextRetrieval* types in packages/shared described those non-existent endpoints and were imported by nothing. The whole semantic-search feature was unreachable, untested dead code. No behavior change: removed code was unmounted/unimported. tsc + biome clean, all 330 API tests pass (incl. SELF.fetch e2e through the live Worker). Co-Authored-By: Duyet Le <[email protected]> Co-Authored-By: duyetbot <[email protected]>
services/v2-projects.ts was a near-complete parallel duplicate of services/projects.ts (its own getOrCreateOrg, createProject, listProjects, getProjectById, deleteProject, org helpers). The only live consumer was routes/projects.ts importing a single function — updateProject — while the duplicated list/create/get/delete and cursor pagination were wired to no route (left dead by the documented v2→v1 collapse). - Inlined a lean updateProject into services/projects.ts (void return; the route already re-reads via getProjectById, so the old V2ProjectDetail payload assembly + extra apiKeys query was dead). - Repointed routes/projects.ts to import updateProject from services/projects. - Deleted services/v2-projects.ts and test/v2-projects-cursor.test.ts (the latter only exercised the unreachable v2 cursor pagination). Removes the "two project services / misleading v2- prefix" smell. No behavior change: live routes already used services/projects for everything else, and updateProject's observable effect (the DB write) is unchanged. tsc + biome clean, 327 tests pass. Co-Authored-By: Duyet Le <[email protected]> Co-Authored-By: duyetbot <[email protected]>
test/v2-{analytics,conversations,keys,projects}.skip.ts were excluded from the
run (vitest only matches *.test.ts) and were written against the old v2 API
contract — project_id payloads, data/pagination envelopes, and API-key auth on
routes that are now Clerk-authed or no longer exist after the v2→v1 collapse.
The routes they targeted were removed in the prior two commits; their live
equivalents are covered by conversations.test.ts, projects.test.ts,
analytics.test.ts, keys.test.ts, and clerk-dashboard-auth.test.ts, so no
coverage is lost. Removes dormant, misleading dead test artifacts.
No behavior change. tsc + biome clean, 327 tests pass.
Co-Authored-By: Duyet Le <[email protected]>
Co-Authored-By: duyetbot <[email protected]>
The routes/v2/ directory was a relic of an abandoned versioning plan. Its README described "the next version of the API" with conversations/ and projects/ placeholders (both since deleted) and instructed future devs to deprecate v1 in favor of v2 — directly contradicting the established model (CLAUDE.md and index.ts: "one API version — v1 is the latest and only one"). The five routers it held (states, leases, claims, capability-tokens, organizations) are all mounted at /api/v1/*, so "v2" was pure misdirection. - Moved the five route dirs up to routes/ alongside conversations/, webhooks/, oauth/, mcp/ (the existing flat convention) and fixed their import depth. - Renamed the *V2Router import aliases to plain *Router in index.ts. - Deleted routes/v2/README.md (documented a versioning strategy that does not exist). No behavior change: mount paths are unchanged, proven by the state-platform / coordination test suites hitting /api/v1/states, /leases, /claims, etc. tsc + biome clean, 327 tests pass. Co-Authored-By: Duyet Le <[email protected]> Co-Authored-By: duyetbot <[email protected]>
services/v2-conversations.ts was named after an API version that no longer exists; after the v2→v1 collapse its only consumer is the MCP server. The name sat misleadingly next to services/conversations.ts (the canonical REST service), obscuring which is current. Renamed to services/mcp-conversations.ts and documented why it stays separate from the REST service (different response contract; the REST service also fires the conversation.created webhook and uses composite-cursor pagination — merging would change MCP behavior). Dropped the stale "V2:" comment prefixes. No behavior change — pure rename + comments, one importer updated. tsc + biome clean, 327 tests pass (incl. MCP conversation-tool coverage). Co-Authored-By: Duyet Le <[email protected]> Co-Authored-By: duyetbot <[email protected]>
…eprecation) Two orphans surfaced by the earlier v2 cleanup: - lib/helpers.ts:mapConversationToResponse — its only caller was the deleted routes/v2/conversations/crud.ts; conversation responses elsewhere go through serialization.ts:deserializeConversationFull. - lib/deprecation.ts — setDeprecationHeaders/deprecationMiddleware were never wired into any route; the only reference was the (now-deleted) routes/v2 README example. The v1-only API has nothing to deprecate. Neither is re-exported from packages/shared, so both were internal dead code. No behavior change. tsc + biome clean, 327 tests pass. Co-Authored-By: Duyet Le <[email protected]> Co-Authored-By: duyetbot <[email protected]>
services/analytics.ts (cache-key helpers, getSummary/getTimeseries/ getTagAnalytics) had zero consumers repo-wide — its only importer was the deleted routes/v2/analytics router. The live dashboard analytics route (routes/analytics.ts, GET /api/v1/projects/:id/analytics) computes its results with its own inline Drizzle queries and never touched this service. No behavior change. tsc + biome clean, 327 tests pass (analytics.test.ts exercises the live route via SELF.fetch, unaffected). Co-Authored-By: Duyet Le <[email protected]> Co-Authored-By: duyetbot <[email protected]>
The AIProvider interface and WorkersAIProvider advertised generateEmbedding (and an EMBEDDING_MODEL constant), but its only caller was services/embeddings.ts, removed with the dead semantic-search feature. Trimming the contract to what the app actually uses (title + follow-up generation) tightens the provider's boundary. No behavior change. tsc + biome clean, 327 tests pass. Co-Authored-By: Duyet Le <[email protected]> Co-Authored-By: duyetbot <[email protected]>
getCachedCount and parseIncludeParam (lib/helpers.ts) and getConversationCount (services/mcp-conversations.ts) had no remaining call sites — their only callers were the deleted routes/v2 conversation handlers. mcp-conversations.listConversations already returns its own total count, so MCP never needed getConversationCount. Also dropped the now-unused CACHE_COUNT_TTL_S import from helpers.ts. No behavior change. tsc + biome clean, 327 tests pass. Co-Authored-By: Duyet Le <[email protected]> Co-Authored-By: duyetbot <[email protected]>
Closes the standing "services/v2-projects.ts other exports are dead — inline in a follow-up" note and documents why two conversation services remain by design and why the VECTORIZE binding was left as-is. Co-Authored-By: Duyet Le <[email protected]> Co-Authored-By: duyetbot <[email protected]>
Reviewer's GuideCollapses the abandoned API v2 stack into the v1 surface by deleting dead v2 routes/services/tests, flattening v2 route directories into the primary routes tree, consolidating the duplicate projects service, and cleaning up all orphaned helpers, semantic-search plumbing, and unused AI-provider APIs while preserving behavior. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
More reviews will be available in 10 minutes and 58 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR executes a v2→v1 dead-code collapse: it deletes the v2 router trees for conversations, keys, and analytics; removes the embedding, analytics, and deprecation services; inlines Changesv2→v1 Dead-Code Collapse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request completes the v2 to v1 dead-code collapse, removing unused API v2 routes, orphaned services (such as analytics and embeddings), stale tests, and deprecated helpers. The state-platform and coordination features have been consolidated under /api/v1/*, and the updateProject logic has been inlined into services/projects.ts. Feedback on the changes points out a potential issue in updateProject where an empty updates object could cause a runtime error or invalid SQL query in Drizzle ORM, which can be resolved by adding an early return guard.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
services/projects.updateProject, consider using a domain-specific error type or result enum instead of a genericError("Project not found")so the route layer can distinguish not-found from other failures without relying on string matching. - Now that
ai-provider’s interface no longer exposesgenerateEmbedding, double-check any non-defaultAIProviderimplementations (e.g., in other packages or environments) to ensure they’ve dropped the method as well and still conform to the updated interface.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `services/projects.updateProject`, consider using a domain-specific error type or result enum instead of a generic `Error("Project not found")` so the route layer can distinguish not-found from other failures without relying on string matching.
- Now that `ai-provider`’s interface no longer exposes `generateEmbedding`, double-check any non-default `AIProvider` implementations (e.g., in other packages or environments) to ensure they’ve dropped the method as well and still conform to the updated interface.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
updateProject accepts { name?, retention_days? }; if a direct caller passes
neither, the updates object would be empty and Drizzle's .set({}) emits invalid
SQL. The route's UpdateProjectSchema already requires at least one field, so
this only hardens non-route callers — early-return on an empty patch.
Per review feedback on PR #228.
Co-Authored-By: Duyet Le <[email protected]>
Co-Authored-By: duyetbot <[email protected]>
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
2 participants
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.
Goal
Refactor toward clear boundaries, low coupling, and high cohesion — no behavior change. Each commit makes one bounded structural improvement and names the smell it removes.
The codebase carried a whole parallel "API v2" universe left behind by an abandoned versioning effort (already documented in
core-memory: "v1 is the latest and only version"). This PR finishes that collapse and cleans up every orphan it exposed.What changed (one smell per commit)
routes/v2/{conversations,keys,analytics}trees, the orphanedservices/embeddings.ts, and theSemanticSearch*/ContextRetrieval*types inpackages/shared(unreachable, unimported, untested).services/v2-projects.tswas a near-complete duplicate ofservices/projects.ts; inlined its one live function (updateProject) and deleted the rest + its dead-only test.test/v2-*.skip.tssuites (excluded from the run; written against the old v2 contract).routes/v2/→routes/(states/leases/claims/capability-tokens/organizations are all mounted at/api/v1/*) and deleted its fiction README; renamed*V2Routeraliases.services/v2-conversations.ts→services/mcp-conversations.ts(its only consumer is the MCP server).mapConversationToResponse,lib/deprecation.ts,getCachedCount,parseIncludeParam,getConversationCount, and the AI provider's unusedgenerateEmbedding— all left callerless by the deletions above.services/analytics.ts(zero consumers; the live dashboard analytics route computes inline).Net: 17 files removed, 5 moved, ~3.7k LOC deleted.
Before / after shape
routes/+routes/v2/), two project services, two conversation services with confusingv2names, a half-built unreachable semantic-search feature, a fiction "API v2" README, and a trail of orphaned helpers/services./api/v1/*with cohesive top-level dirs; one canonicalservices/projects.ts;services/conversations.ts(REST) and a clearly-namedservices/mcp-conversations.ts(MCP) — kept separate by design (the REST service fires theconversation.createdwebhook and uses composite-cursor pagination; merging would change MCP behavior); an AI provider trimmed to what's used.Verification
Every commit was gated:
tsc --noEmitclean,biome checkclean, and 327 tests pass (the only delta from 330 is the 3 removed v2-cursor tests that exercised unreachable code). Behavior is proven unchanged by theSELF.fetche2e/coordination suites that drive the real Worker over HTTP against local D1.Left intentionally untouched
VECTORIZE_INDEXbinding (types.ts/wrangler.jsonc/deploy script) — removing it is a deploy-infra change that can't be live-tested in this environment.🤖 Generated with Claude Code
https://claude.ai/code/session_01TJsgSoJGYgabmcZEYhCxpQ
Generated by Claude Code
Summary by Sourcery
Remove the unused API v2 stack and consolidate remaining functionality under the v1 API and core services, without changing behavior.
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
Refactor
Chores