feat: rewrite #19

lishaduck · 2026-01-17T18:26:46Z

No description provided.

Copilot

Pull request overview

This PR represents a major rewrite that significantly simplifies the application by removing most of its core functionality. The changes transform the project from a full-featured encrypted notes application with real-time collaboration into a minimal SvelteKit starter with Storybook integration and basic authentication demos.

Changes:

Removed the entire notes application including encryption, CRDT sync, CodeMirror editor, and file organization features
Added Storybook with example components and Vitest testing infrastructure
Simplified database schema to basic user/session tables with an unused 'age' field
Replaced remote functions/forms with standard SvelteKit form actions
Removed Effect, DaisyUI, Loro CRDT, and other major dependencies

Reviewed changes

Copilot reviewed 76 out of 80 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
vite.config.ts	Added Vitest and Storybook test configuration with browser testing support
tsconfig.json	Updated types for Vitest browser testing, removed Effect language service
svelte.config.js	Simplified config, added Storybook path to TypeScript includes
src/lib/server/db/schema.ts	Drastically simplified schema - removed notes/encryption tables, renamed session table
src/lib/server/auth.ts	Removed helper functions (guardLogin, requireLogin), changed cookie handlers to accept RequestEvent
src/hooks.server.ts	Removed automatic authentication redirects - critical security change
src/routes/demo/lucia/*	Added demo authentication pages with basic login/register
src/stories/*	Added Storybook example components (Button, Header, Page)
package.json	Major dependency changes - removed 40+ packages, added Storybook/Vitest
drizzle/*	Deleted all existing migrations without providing upgrade path

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-17T18:30:37Z

src/lib/server/db/schema.ts

+export const session = sqliteTable("session", {
+  id: text("id").primaryKey(),
  userId: text("user_id")
    .notNull()
-    .references(() => users.id),
+    .references(() => user.id),
  expiresAt: integer("expires_at", { mode: "timestamp" }).notNull(),
 });


The database schema has been significantly modified with the session table's primary key renamed from 'token' to 'id', but there are no migration files to handle this change. The existing database migration snapshot and SQL files have been deleted, which means upgrading from the previous schema will fail. A migration file is needed to handle the transition from the old 'sessions' table (with 'token' as PK) to the new 'session' table (with 'id' as PK).

Copilot · 2026-01-17T18:30:37Z

src/lib/server/db/schema.ts

-export const users = sqliteTable("users", {
+export const user = sqliteTable("user", {
  id: text("id").primaryKey(),
+  age: integer("age"),


The 'age' field has been added to the user table but is not used anywhere in the authentication logic or application. Consider removing this field if it's not needed, or ensure it's properly utilized in the application.

Copilot · 2026-01-17T18:30:37Z

drizzle.config.ts


 export default defineConfig({
-  out: "./drizzle",
  schema: "./src/lib/server/db/schema.ts",


The drizzle configuration no longer specifies an output directory for migrations with the 'out' field removed. This means migrations will use the default location, which may not align with the project structure. Consider explicitly setting the output directory to maintain consistency.

Suggested change

schema: "./src/lib/server/db/schema.ts",

schema: "./src/lib/server/db/schema.ts",

out: "./drizzle",

Copilot · 2026-01-17T18:30:38Z

pnpm-workspace.yaml

@@ -1,8 +1,12 @@
+allowBuilds:
+  "@tailwindcss/oxide@4.1.18": true
+  esbuild@0.25.12 || 0.27.2: true


The workspace configuration has breaking changes with the removal of 'onlyBuiltDependencies' (which was limiting builds to specific packages) in favor of 'allowBuilds' with a much more permissive pattern. The new pattern allows all esbuild versions matching '0.25.12 || 0.27.2', which is overly broad and could allow unintended builds. Consider being more specific with the esbuild version constraint.

Suggested change

esbuild@0.25.12 || 0.27.2: true

esbuild@0.27.2: true

Copilot · 2026-01-17T18:30:38Z

src/lib/server/auth.ts

 export function setSessionTokenCookie(
-  cookies: Cookies,
+  event: RequestEvent,
  token: string,
  expiresAt: Date,
 ): void {
-  cookies.set(sessionCookieName, token, {
+  event.cookies.set(sessionCookieName, token, {
    expires: expiresAt,
    path: "/",
  });
 }

-export function deleteSessionTokenCookie(cookies: Cookies): void {
-  cookies.delete(sessionCookieName, {
+export function deleteSessionTokenCookie(event: RequestEvent): void {
+  event.cookies.delete(sessionCookieName, {
    path: "/",
  });
 }


The function parameters and return type have changed from accepting a 'Cookies' object to accepting the full 'RequestEvent', but the function names still reference 'cookie' in singular form. For consistency and clarity, consider renaming these functions to better reflect that they operate on the event object (e.g., 'setSessionTokenOnEvent', 'deleteSessionTokenFromEvent') or update the documentation to clarify the parameter change.

Copilot · 2026-01-17T18:30:39Z

eslint.config.ts

  {
    linterOptions: {
-      reportUnusedDisableDirectives: "error",
+      reportUnusedDisableDirectives: "error", // wait, is this the default now?


The comment on line 20 appears to be a question rather than documentation. Comments should be declarative statements explaining the code's purpose. Either remove the comment if the default behavior is now known, or rephrase it as a proper explanation.

Suggested change

reportUnusedDisableDirectives: "error", // wait, is this the default now?

reportUnusedDisableDirectives: "error", // Treat unused eslint-disable comments as errors.

Copilot · 2026-01-17T18:30:39Z

CLAUDE.md

@@ -0,0 +1 @@
+AGENTS.md


The CLAUDE.md file only contains a reference to 'AGENTS.md' without any additional context. This creates an indirect reference that makes navigation less clear. Consider either consolidating the content into one file or providing more context about why this indirection exists.

Copilot · 2026-01-17T18:30:39Z

src/demo.spec.ts

+import { describe, it, expect } from "vitest";
+
+describe("sum test", () => {
+  it("adds 1 + 2 to equal 3", () => {
+    expect(1 + 2).toBe(3);
+  });
+});


The test suite only contains a single trivial test that adds 1 + 2. This provides no real coverage of the application functionality. Given that the 'merge-checks' command now includes test execution (line 26 in package.json), having only a placeholder test means the test gate will always pass without validating any actual code.

Copilot · 2026-01-17T18:30:39Z

vite.config.ts

+    expect: {
+      requireAssertions: true,
+    },
+    coverage: {
+      enabled: true,
+    },
+    projects: [
+      {
+        extends: true,
+        test: {
+          name: "server",
+          environment: "node",
+          dir: "src/",
+          include: ["**/*.{test,spec}.{js,ts}"],
+        },
+      },
+      {
+        extends: true,
+        plugins: [
+          // The plugin will run tests for the stories defined in your Storybook config
+          // See options at: https://storybook.js.org/docs/next/writing-tests/integrations/vitest-addon#storybooktest
+          storybookTest({
+            configDir: path.join(import.meta.dirname, ".storybook"),
+          }),
+        ],
+        test: {
+          name: "storybook",
+          browser: {
+            enabled: true,
+            headless: true,
+            provider: playwright({}),
+            instances: [
+              {
+                browser: "chromium",
+              },
+            ],
+          },
+          setupFiles: [".storybook/vitest.setup.ts"],
+          expect: {
+            requireAssertions: false,
+          },


The Vitest configuration requires assertions in all tests (requireAssertions: true) on line 13, but then disables this requirement specifically for Storybook tests on line 51. This creates an inconsistent testing standard where Storybook tests could pass without any assertions, potentially leading to false positives. Consider either enforcing assertions everywhere or documenting why Storybook tests are exempt from this requirement.

Copilot · 2026-01-17T18:30:40Z

vite.config.ts

+          browser: {
+            enabled: true,
+            headless: true,
+            provider: playwright({}),


The import of 'playwright' function appears on line 4 but is only used once on line 42. The 'playwright({})' call passes an empty configuration object. Consider documenting why an empty configuration is used or if there are specific options that should be configured for the browser provider.

Copilot AI review requested due to automatic review settings January 17, 2026 18:26

Copilot started reviewing on behalf of lishaduck January 17, 2026 18:27 View session

Copilot AI reviewed Jan 17, 2026

View reviewed changes

chore: wipe

fc856ac

lishaduck force-pushed the rewrite branch from f16554f to fc856ac Compare January 17, 2026 18:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: rewrite #19

feat: rewrite #19

Uh oh!

lishaduck commented Jan 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	schema: "./src/lib/server/db/schema.ts",
	schema: "./src/lib/server/db/schema.ts",
	out: "./drizzle",

	reportUnusedDisableDirectives: "error", // wait, is this the default now?
	reportUnusedDisableDirectives: "error", // Treat unused eslint-disable comments as errors.

		@@ -0,0 +1 @@
		AGENTS.md No newline at end of file

feat: rewrite #19

Are you sure you want to change the base?

feat: rewrite #19

Uh oh!

Conversation

lishaduck commented Jan 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants