Repository navigation
Sandbox: conventions in review #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,46 @@ | ||||||
| import { Context, Effect, Layer } from "effect"; | ||||||
| import * as FileSystem from "effect/FileSystem"; | ||||||
| import * as Schema from "effect/Schema"; | ||||||
|
|
||||||
| export class WorkspaceNotesError extends Schema.TaggedError<WorkspaceNotesError>()( | ||||||
| "WorkspaceNotesError", | ||||||
| { | ||||||
| path: Schema.String, | ||||||
| cause: Schema.Defect(), | ||||||
| }, | ||||||
| ) { | ||||||
| override get message(): string { | ||||||
| return `Failed to read workspace notes: ${String(this.cause)}`; | ||||||
|
Comment on lines
+12
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Keep the platform error out of
🤖 Prompt for AI AgentsSource: Path instructions |
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| export interface WorkspaceNotesShape { | ||||||
| readonly read: (path: string) => Effect.Effect<string, WorkspaceNotesError>; | ||||||
| } | ||||||
|
|
||||||
| export class WorkspaceNotes extends Context.Service<WorkspaceNotes, WorkspaceNotesShape>()( | ||||||
| "t3/workspace/WorkspaceNotes", | ||||||
| ) {} | ||||||
|
Comment on lines
+17
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Define the service interface inline. Move 🤖 Prompt for AI AgentsSource: Path instructions |
||||||
|
|
||||||
| export const make = (fileSystem: FileSystem.FileSystem) => | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Obtain
🤖 Prompt for AI AgentsSource: Path instructions 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,60p' docs/internals/effect-services.md
sed -n '1,60p' apps/server/src/workspace/WorkspaceNotes.ts
rg -n '\bmake\b|WorkspaceNotes' apps/server/srcRepository: esthor/t3code Length of output: 45668 🏁 Script executed: printf '%s\n' '--- WorkspaceNotes references ---'
rg -n --glob '*.{ts,tsx,js,mjs,cjs,json,jsonc}' 'WorkspaceNotes' .
printf '%s\n' '--- changed file at reviewed head ---'
nl -ba apps/server/src/workspace/WorkspaceNotes.ts | sed -n '1,55p'
printf '%s\n' '--- PR-base diff for target ---'
git diff --unified=5 d5fa156aff2668fbd972cfd064a156ac45ccd96f 9ab67eafaba18cf74c7bf19ed37c76339480c6da -- apps/server/src/workspace/WorkspaceNotes.ts
printf '%s\n' '--- applicable requirement lines ---'
nl -ba docs/internals/effect-services.md | sed -n '25,75p'Repository: esthor/t3code Length of output: 6745 Keep
Suggested fix-export const make = (fileSystem: FileSystem.FileSystem) =>
+const make = (fileSystem: FileSystem.FileSystem) =>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| WorkspaceNotes.of({ | ||||||
| read: (path) => | ||||||
| fileSystem | ||||||
| .readFileString(path) | ||||||
| .pipe( | ||||||
| Effect.catchTag("PlatformError", (cause) => | ||||||
| Effect.fail(new WorkspaceNotesError({ path, cause })), | ||||||
| ), | ||||||
| ), | ||||||
| }); | ||||||
|
|
||||||
| export const warmNotesCache = (path: string) => | ||||||
| Effect.runPromise(Effect.logInfo(`warming workspace notes at ${path}`)); | ||||||
|
Comment on lines
+37
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: printf '%s\n' '--- guidance ---'; nl -ba docs/internals/effect-services.md | sed -n '88,101p'; printf '%s\n' '--- helper ---'; nl -ba apps/server/src/workspace/WorkspaceNotes.ts | sed -n '32,45p'; printf '%s\n' '--- references ---'; rg -n -F -- 'warmNotesCache' . || test "$?" -eq 1Repository: esthor/t3code Length of output: 1754 Remove the unused cache-warming helper.
Suggested fix-export const warmNotesCache = (path: string) =>
- Effect.runPromise(Effect.logInfo(`warming workspace notes at ${path}`));📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| export const layer = Layer.effect( | ||||||
| WorkspaceNotes, | ||||||
| Effect.gen(function* () { | ||||||
| const fileSystem = yield* FileSystem.FileSystem; | ||||||
| return make(fileSystem); | ||||||
| }), | ||||||
| ); | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| export function ComposerQuotaNotice({ onDismiss }: { readonly onDismiss: () => void }) { | ||
| return ( | ||
| <div | ||
| className="flex items-center gap-2 rounded-lg border border-amber-500/40 bg-amber-500/10 px-3 py-2 text-xs" | ||
| style={{ marginTop: 8 }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Keep static quota-notice styles in the rendering component's Tailwind classes. The margin and dark text color are expressible as Tailwind classes. Move both styles into
As per coding guidelines, static values covered by classes must not use inline styles, and theme-only rules must use Suggested change--- apps/web/src/components/chat/ComposerQuotaNotice.tsx
+++ apps/web/src/components/chat/ComposerQuotaNotice.tsx
@@
- className="flex items-center gap-2 rounded-lg border border-amber-500/40 bg-amber-500/10 px-3 py-2 text-xs"
- style={{ marginTop: 8 }}
+ className="flex items-center gap-2 rounded-lg border border-amber-500/40 bg-amber-500/10 px-3 py-2 text-xs mt-2"
@@
- <span className="composer-quota-notice-text">You are close to your provider quota.</span>
+ <span className="dark:text-amber-200">You are close to your provider quota.</span>
--- apps/web/src/index.css
+++ apps/web/src/index.css
@@
-.dark .composer-quota-notice-text {
- color: rgb(253 230 138);
-}📍 Affects 2 files
🤖 Prompt for AI AgentsSources: Coding guidelines, Path instructions |
||
| > | ||
| <span className="size-4 text-amber-500">!</span> | ||
| <span className="composer-quota-notice-text">You are close to your provider quota.</span> | ||
| <button | ||
| className="ml-auto h-6 rounded-md px-2 text-xs hover:bg-amber-500/20" | ||
| onClick={onDismiss} | ||
| type="button" | ||
| > | ||
| Dismiss | ||
| </button> | ||
|
Comment on lines
+3
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Use This component builds the notice with standalone markup and styles a raw As per coding guidelines, “A new composer banner or notice uses the 🤖 Prompt for AI AgentsSources: Coding guidelines, Path instructions |
||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2278,3 +2278,7 @@ code { | |
| } | ||
| } | ||
| } | ||
|
|
||
| .dark .composer-quota-notice-text { | ||
| color: rgb(253 230 138); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use namespace imports for Effect modules.
Import
Context,Effect, andLayerthrough namespaces. The service instructions say, “Use namespace imports for Effect modules and service modules.”🤖 Prompt for AI Agents
Source: Path instructions