Added oxford views page#10
Closed
napan24 wants to merge 8 commits into
Closed
Conversation
…#13591) * feat(worker): add production monitoring evaluator templates * fix(worker): keep only new monitoring evaluator templates
|
@claude review |
📝 WalkthroughWalkthroughThis PR introduces Oxford Views, a comprehensive feature for managing versioned prompt configurations. The implementation spans database schema, backend TRPC router with 15 procedures, React components for form creation and detail viewing, version history timeline, table listing with filtering, label management, and delete/duplicate workflows. ChangesOxford Views Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
OX Security reviewed this pull request — nothing to fix.
Branch |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/shared/prisma/migrations/20260526000000_add_oxford_views/migration.sql`:
- Around line 13-14: The migration creates oxford_views.tags and
oxford_views.labels as nullable arrays but Prisma's model OxfordView declares
tags String[] `@default`([]) and labels String[] `@default`([]) (required lists);
update the migration to mark both columns NOT NULL so the DB constraint matches
Prisma—locate the column definitions for "tags" and "labels" in the
migration.sql and add NOT NULL to each DEFAULT ARRAY[]::TEXT[] definition.
In `@web/src/features/oxford-views/components/delete-oxford-view-version.tsx`:
- Around line 52-56: The Popover's onOpenChange should set state to the incoming
open value instead of toggling; update the onOpenChange handler used with
Popover (current: onOpenChange={() => setIsOpen(!isOpen)}) to accept the
provided open parameter and call setIsOpen(open) (or setIsOpen(Boolean(open)))
so isOpen always reflects the Popover's actual open state.
In `@web/src/features/oxford-views/components/delete-oxford-view.tsx`:
- Line 30: The Popover's onOpenChange currently toggles state with
setIsOpen(!isOpen), which can produce stale/inconsistent state; change the
handler to use the provided open boolean (the onOpenChange parameter) to set
state deterministically by calling setIsOpen(open) (or use the functional
updater if necessary), so update the Popover's onOpenChange implementation
referencing isOpen, setIsOpen and Popover to accept the event-provided open
value instead of blindly toggling.
In `@web/src/features/oxford-views/components/NewOxfordViewForm/validation.ts`:
- Around line 27-53: Replace the ad-hoc z.array(z.any()) used for the chatPrompt
field with the shared PromptChatMessageListSchema in both
NewOxfordViewBaseSchema (where the placeholder/name and non-empty refinements
are defined) and NewTextOxfordViewSchema.chatPrompt; keep the existing
placeholder shape check using PlaceholderMessageSchema and the non-empty content
.refine chain but operate on PromptChatMessageListSchema (or chain .refine calls
after PromptChatMessageListSchema) so the shared contract ({ role, content } for
normal items and { type: Placeholder, name } for placeholders) is enforced and
the empty-message refinement still runs to reject invalid payloads before
submit.
In `@web/src/features/oxford-views/components/oxford-view-detail.tsx`:
- Around line 199-207: The Button currently wraps a Link which creates nested
interactive controls; change Button to render its child as the interactive
element by adding the asChild prop on Button and keeping the Link as the direct
child (so the Link becomes the actual interactive element), e.g., use Button
(with className and asChild) around the existing Link (preserve href and
children such as Plus and span) in the oxford-view-detail.tsx component to avoid
nested controls and restore correct keyboard/accessibility behavior.
- Around line 125-137: The memoized generation of pythonCode/jsCode in useMemo
only depends on view?.id but actually uses view.name, view.version and
view.labels, so update the dependency array for the useMemo that computes
pythonCode/jsCode (the block that calls getPythonCode and getJsCode) to include
view.name, view.version and view.labels (or a stable representation like
JSON.stringify(view.labels)) so snippets recompute when labels/name/version
change; keep the existing sortedLabels/getPythonCode/getJsCode logic intact.
- Around line 139-141: The current conditional (if (!viewHistory.data || !view)
return Loading...) hides fetch errors and real "not found" cases; update
oxford-view-detail.tsx to branch explicitly: render a loading state only when
viewHistory.isLoading (or equivalent loading flag) is true, render an error UI
when viewHistory.isError or viewHistory.error exists (include the error
message), and render a clear "View not found" / "Version/label not found"
message when viewHistory.data is present but view is null/undefined; refer to
viewHistory, view, and any status/error flags on the data-fetch hook to
implement these branches.
In `@web/src/features/oxford-views/components/oxford-view-history.tsx`:
- Around line 26-38: The useEffect that auto-scrolls references
currentRef.current but only lists [currentRef.current] so it won't run when
props.currentVersion or view.version change; update the effect dependencies to
include props.currentVersion and view.version (e.g., [props.currentVersion,
view.version, currentRef]) and remove the eslint-disable comment so React
enforces correct deps; keep the existing scrollIntoView call and condition
(props.currentVersion && currentRef.current && props.currentVersion ===
view.version) but rely on the corrected dependency array to trigger the effect
reliably.
- Around line 12-18: There are two problems: a redeclaration of
OxfordViewHistoryNode (it appears both as a local const and later as export
const) and an effect that only depends on currentRef.current so it doesn't
auto-scroll when the selected version changes. Fix by keeping a single
declaration/export of OxfordViewHistoryNode (remove or consolidate the duplicate
declaration/export so only one OxfordViewHistoryNode exists) and update the
scroll effect (the useEffect that references currentRef and calls
scrollIntoView) to include props.currentVersion and/or view.version in its
dependency array (e.g., [currentRef.current, props.currentVersion,
view.version]) so the element scrolls when the active version changes.
In `@web/src/features/oxford-views/components/oxford-views-table.tsx`:
- Around line 191-194: The detail-page list is saving folder-ambiguous IDs (id:
v.name) which breaks navigation for nested folders; update the setDetailPageList
call that maps views.data.prompts so each item uses the folder-aware ID by
calling the existing buildFullPath(...) for each prompt (e.g., replace id:
v.name with id: buildFullPath(v) or equivalent), ensuring the generated IDs
match the IDs used elsewhere for detail navigation.
In `@web/src/features/oxford-views/server/routers/oxfordViewRouter.ts`:
- Around line 244-247: The tag-search Prisma.sql condition currently runs even
when input.searchQuery is undefined, producing ILIKE '%undefined%'; update the
construction of additionalConditions (the branch that checks
input.searchType?.includes("id")) to first ensure input.searchQuery is a
non-empty string before adding the Prisma.sql entry, and if empty/undefined skip
adding that tag ILIKE condition; apply the same guard to the other occurrence
that builds a similar Prisma.sql tag ILIKE condition so neither location
constructs an ILIKE '%undefined%' predicate.
- Around line 669-703: The filterOptions procedure performs project-scoped reads
without verifying the caller's access; add a project access check at the start
of the query handler by invoking the existing throwIfNoProjectAccess (or
equivalent) with the input.projectId and ctx before any ctx.prisma queries. In
practice, inside filterOptions's .query(async ({ input, ctx }) => { ... }), call
await throwIfNoProjectAccess(input.projectId, ctx) (or
ctx.throwIfNoProjectAccess...) immediately, then proceed to run the three
Promise.all queries and return the same result shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: af4d4009-b647-4fa9-8c4c-437b25364fb7
📒 Files selected for processing (19)
packages/shared/prisma/migrations/20260526000000_add_oxford_views/migration.sqlpackages/shared/prisma/schema.prismaweb/src/components/layouts/routes.tsxweb/src/features/oxford-views/components/NewOxfordViewForm/index.tsxweb/src/features/oxford-views/components/NewOxfordViewForm/validation.tsweb/src/features/oxford-views/components/SetOxfordViewVersionLabels/LabelCommandItem.tsxweb/src/features/oxford-views/components/SetOxfordViewVersionLabels/index.tsxweb/src/features/oxford-views/components/delete-oxford-view-folder.tsxweb/src/features/oxford-views/components/delete-oxford-view-version.tsxweb/src/features/oxford-views/components/delete-oxford-view.tsxweb/src/features/oxford-views/components/duplicate-oxford-view.tsxweb/src/features/oxford-views/components/oxford-view-detail.tsxweb/src/features/oxford-views/components/oxford-view-history.tsxweb/src/features/oxford-views/components/oxford-view-new.tsxweb/src/features/oxford-views/components/oxford-views-table.tsxweb/src/features/oxford-views/server/routers/oxfordViewRouter.tsweb/src/pages/project/[projectId]/oxford-views/[[...folder]].tsxweb/src/pages/project/[projectId]/oxford-views/new.tsxweb/src/server/api/root.ts
Comment on lines
+13
to
+14
| "tags" TEXT[] DEFAULT ARRAY[]::TEXT[], | ||
| "labels" TEXT[] DEFAULT ARRAY[]::TEXT[], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
MIGRATION_FILE="$(fd '20260526000000_add_oxford_views/migration.sql$')"
SCHEMA_FILE="$(fd '^schema.prisma$' packages/shared/prisma)"
echo "== Migration column definitions =="
rg -n '"tags"|\"labels\"' "$MIGRATION_FILE" -C1
echo
echo "== Prisma OxfordView field definitions =="
rg -n 'model OxfordView|tags\s+String\[\]|labels\s+String\[\]' "$SCHEMA_FILE" -A8 -B2Repository: flipkart-incubator/langfuse
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
MIGRATION_FILE="$(fd '20260526000000_add_oxford_views/migration.sql$' .)"
SCHEMA_FILE="$(fd '^schema\.prisma$' packages/shared/prisma)"
echo "== Migration column definitions =="
rg -n '"(tags|labels)"' "$MIGRATION_FILE" -C1
echo
echo "== Prisma OxfordView field definitions =="
rg -n 'model OxfordView\b' "$SCHEMA_FILE" -A30Repository: flipkart-incubator/langfuse
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the migration.sql for the referenced migration folder
echo "== Candidate migration.sql paths =="
fd -t f '20260526000000_add_oxford_views/migration\.sql$' . || true
fd -t f '20260526000000_add_oxford_views' packages/shared/prisma/migrations || true
# Find schema.prisma
SCHEMA_FILE="$(fd -t f '^schema\.prisma$' packages/shared/prisma | head -n 1)"
echo "== schema.prisma =="
echo "${SCHEMA_FILE:-<not found>}"
# Find tags/labels columns + Prisma fields
if [ -n "${SCHEMA_FILE:-}" ]; then
echo
echo "== Prisma OxfordView fields =="
rg -n 'model OxfordView\b' "$SCHEMA_FILE" -n -A60 | sed -n '1,80p' || true
rg -n '\b(tags|labels)\b' "$SCHEMA_FILE" | head -n 50 || true
fiRepository: flipkart-incubator/langfuse
Length of output: 3430
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the referenced migration.sql by searching for the table creation
MIGRATION_FILE="$(rg -l --fixed-string 'CREATE TABLE "oxford_views"' packages/shared/prisma/migrations | head -n 1)"
echo "== Using migration file =="
echo "$MIGRATION_FILE"
echo
echo "== Migration column definitions (tags/labels) =="
rg -n '"(tags|labels)"\s+TEXT\[\]' "$MIGRATION_FILE" -C2
echo
echo "== Presence of NOT NULL =="
rg -n '"(tags|labels)"' "$MIGRATION_FILE" -C1 | head -n 50Repository: flipkart-incubator/langfuse
Length of output: 519
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/shared/prisma/migrations/20260526000000_add_oxford_views/migration.sql"
SCHEMA_FILE="packages/shared/prisma/schema.prisma"
echo "== File existence =="
ls -l "$FILE" || true
echo
echo "== Migration.sql lines 1-40 =="
if [ -f "$FILE" ]; then
nl -ba "$FILE" | sed -n '1,40p'
fi
echo
echo "== Migration.sql tags/labels definitions =="
if [ -f "$FILE" ]; then
rg -n '"(tags|labels)"\s+TEXT\[\]' "$FILE" -C2
fi
echo
echo "== Prisma schema.prisma OxfordView tags/labels =="
rg -n 'model OxfordView\b' "$SCHEMA_FILE" -A30 -B2Repository: flipkart-incubator/langfuse
Length of output: 315
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/shared/prisma/migrations/20260526000000_add_oxford_views/migration.sql"
echo "== Migration tags/labels definitions =="
rg -n '"(tags|labels)"' "$FILE" -C3
echo
echo "== CREATE TABLE oxford_views block around columns =="
rg -n 'CREATE TABLE "oxford_views"' "$FILE" -n -C30Repository: flipkart-incubator/langfuse
Length of output: 1973
Make oxford_views.tags/labels NOT NULL to match Prisma OxfordView
In packages/shared/prisma/migrations/20260526000000_add_oxford_views/migration.sql (lines 13-14), tags and labels are created as TEXT[] DEFAULT ARRAY[]::TEXT[] (nullable). Prisma’s packages/shared/prisma/schema.prisma defines model OxfordView with required list fields: tags String[] @default([]) and labels String[] @default([]). Add NOT NULL to keep DB constraints consistent with Prisma.
Suggested migration fix
- "tags" TEXT[] DEFAULT ARRAY[]::TEXT[],
- "labels" TEXT[] DEFAULT ARRAY[]::TEXT[],
+ "tags" TEXT[] NOT NULL DEFAULT ARRAY[]::TEXT[],
+ "labels" TEXT[] NOT NULL DEFAULT ARRAY[]::TEXT[],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/shared/prisma/migrations/20260526000000_add_oxford_views/migration.sql`
around lines 13 - 14, The migration creates oxford_views.tags and
oxford_views.labels as nullable arrays but Prisma's model OxfordView declares
tags String[] `@default`([]) and labels String[] `@default`([]) (required lists);
update the migration to mark both columns NOT NULL so the DB constraint matches
Prisma—locate the column definitions for "tags" and "labels" in the
migration.sql and add NOT NULL to each DEFAULT ARRAY[]::TEXT[] definition.
Comment on lines
+52
to
+56
| <Popover | ||
| key={promptVersionId} | ||
| open={isOpen} | ||
| onOpenChange={() => setIsOpen(!isOpen)} | ||
| > |
There was a problem hiding this comment.
Popover open-state handler should use provided open value (Line 55).
Toggling with !isOpen can drift from actual component intent and produce flaky open/close behavior.
Suggested fix
- onOpenChange={() => setIsOpen(!isOpen)}
+ onOpenChange={(open) => setIsOpen(open)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Popover | |
| key={promptVersionId} | |
| open={isOpen} | |
| onOpenChange={() => setIsOpen(!isOpen)} | |
| > | |
| <Popover | |
| key={promptVersionId} | |
| open={isOpen} | |
| onOpenChange={(open) => setIsOpen(open)} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/oxford-views/components/delete-oxford-view-version.tsx`
around lines 52 - 56, The Popover's onOpenChange should set state to the
incoming open value instead of toggling; update the onOpenChange handler used
with Popover (current: onOpenChange={() => setIsOpen(!isOpen)}) to accept the
provided open parameter and call setIsOpen(open) (or setIsOpen(Boolean(open)))
so isOpen always reflects the Popover's actual open state.
| }); | ||
|
|
||
| return ( | ||
| <Popover open={isOpen} onOpenChange={() => setIsOpen(!isOpen)}> |
There was a problem hiding this comment.
Popover open-state handler should not toggle blindly (Line 30).
Using setIsOpen(!isOpen) in a controlled popover can cause inconsistent state transitions.
Suggested fix
- <Popover open={isOpen} onOpenChange={() => setIsOpen(!isOpen)}>
+ <Popover open={isOpen} onOpenChange={(open) => setIsOpen(open)}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Popover open={isOpen} onOpenChange={() => setIsOpen(!isOpen)}> | |
| <Popover open={isOpen} onOpenChange={(open) => setIsOpen(open)}> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/oxford-views/components/delete-oxford-view.tsx` at line 30,
The Popover's onOpenChange currently toggles state with setIsOpen(!isOpen),
which can produce stale/inconsistent state; change the handler to use the
provided open boolean (the onOpenChange parameter) to set state
deterministically by calling setIsOpen(open) (or use the functional updater if
necessary), so update the Popover's onOpenChange implementation referencing
isOpen, setIsOpen and Popover to accept the event-provided open value instead of
blindly toggling.
Comment on lines
+27
to
+53
| .array(z.any()) | ||
| .refine( | ||
| (messages: Array<{ type?: ChatMessageType; content?: string }>) => | ||
| messages.every((message) => { | ||
| const isPlaceholder = message?.type === ChatMessageType.Placeholder; | ||
| return ( | ||
| !isPlaceholder || | ||
| PlaceholderMessageSchema.safeParse(message).success | ||
| ); | ||
| }), | ||
| "Placeholder name must start with a letter and contain only alphanumeric characters and underscores", | ||
| ) | ||
| .refine( | ||
| (messages: Array<{ type?: ChatMessageType; content?: string }>) => | ||
| messages.every((message) => { | ||
| const isPlaceholder = message?.type === ChatMessageType.Placeholder; | ||
| return isPlaceholder || Boolean(message?.content?.trim()?.length); | ||
| }), | ||
| "Enter a chat message or remove the empty message", | ||
| ), | ||
| textPrompt: z.string(), | ||
| }); | ||
|
|
||
| const NewTextOxfordViewSchema = NewOxfordViewBaseSchema.extend({ | ||
| type: z.literal(PromptType.Text), | ||
| chatPrompt: z.array(z.any()), | ||
| textPrompt: TextPromptContentSchema, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/src/features/oxford-views/components/NewOxfordViewForm/validation.ts"
echo "---- File: $FILE (lines 1-220) ----"
nl -ba "$FILE" | sed -n '1,220p'
echo
echo "---- Ripgrep: PromptChatMessageListSchema usage ----"
rg -n "PromptChatMessageListSchema" -S web/src | head -n 50 || true
echo
echo "---- Ripgrep: Chat Oxford prompt schemas (context) ----"
rg -n "NewOxfordView|chatPrompt|PromptType\.Chat|PlaceholderMessageSchema" -S web/src/features/oxford-views/components/NewOxfordViewForm/validation.ts
echo
echo "---- Locate definition of PromptChatMessageListSchema (prefer shared package) ----"
# Search in repo for the symbol definition
rg -n "PromptChatMessageListSchema" -S . | head -n 50 || trueRepository: flipkart-incubator/langfuse
Length of output: 235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/src/features/oxford-views/components/NewOxfordViewForm/validation.ts"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
echo "---- File: $FILE (lines 1-260) ----"
cat -n "$FILE" | sed -n '1,260p'
echo
echo "---- Ripgrep: PromptChatMessageListSchema usage (web/src) ----"
rg -n "PromptChatMessageListSchema" -S web/src | head -n 50 || true
echo
echo "---- Ripgrep: PromptChatMessageListSchema (whole repo) ----"
rg -n "PromptChatMessageListSchema" -S . | head -n 50 || true
echo
echo "---- Ripgrep: chatPrompt + z.any() in this validation file ----"
rg -n "chatPrompt|z\.any\(\)|PromptType\.Chat|PromptType\.Text" -S "$FILE"Repository: flipkart-incubator/langfuse
Length of output: 5169
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect shared schema definitions
FILE="packages/shared/src/server/llm/types.ts"
echo "---- File: $FILE (relevant lines 220-310) ----"
sed -n '220,310p' "$FILE" | cat -n
echo
echo "---- Ripgrep: PromptChatMessageSchema / PlaceholderMessageSchema definitions ----"
rg -n "PromptChatMessageSchema|PlaceholderMessageSchema" "$FILE"
echo
echo "---- Extract exact definitions for PromptChatMessageSchema and PromptChatMessageListSchema ----"
python3 - <<'PY'
import re, itertools, pathlib
path = pathlib.Path("packages/shared/src/server/llm/types.ts")
txt = path.read_text(encoding="utf-8")
def extract(pattern, context=10):
m = re.search(pattern, txt, flags=re.M)
if not m:
return None
start = max(0, m.start()-context*1)
return m.group(0)
# We'll just print some nearby lines around the exports
lines = txt.splitlines()
for i,l in enumerate(lines,1):
if "export const PromptChatMessageListSchema" in l:
for j in range(i-10, i+15):
if 1<=j<=len(lines):
print(f"{j:4d}: {lines[j-1]}")
print("----")
if "export const PromptChatMessageSchema" in l:
for j in range(i-10, i+30):
if 1<=j<=len(lines):
print(f"{j:4d}: {lines[j-1]}")
print("====")
PYRepository: flipkart-incubator/langfuse
Length of output: 6189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/shared/src/server/llm/types.ts"
echo "---- File: $FILE (lines 150-260) ----"
sed -n '150,260p' "$FILE" | cat -n
echo
echo "---- Ripgrep for ChatMessageType enum/const in this file ----"
rg -n "enum ChatMessageType|ChatMessageType" "$FILE" | head -n 50
echo
echo "---- Ripgrep for PlaceholderMessageSchema exact object shape nearby ----"
rg -n "export const PlaceholderMessageSchema" "$FILE" -nRepository: flipkart-incubator/langfuse
Length of output: 5113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Oxford form component files around NewOxfordViewForm
echo "---- Oxford view form files ----"
fd -a "NewOxfordViewForm" web/src/features/oxford-views/components
echo
echo "---- Inspect NewOxfordViewForm component (likely uses validation schema) ----"
# Find a likely entry file
for f in $(fd -a "NewOxfordViewForm" web/src/features/oxford-views/components -e ts -e tsx -e jsx 2>/dev/null); do
echo "### $f"
sed -n '1,220p' "$f" | cat -n | sed -n '1,220p'
done
echo
echo "---- Inspect prompts NewPromptForm validation and component to see message shape ----"
PROMPT_VAL="web/src/features/prompts/components/NewPromptForm/validation.ts"
PROMPT_MSG="web/src/features/prompts/components/NewPromptForm/PromptChatMessages.tsx"
if [ -f "$PROMPT_VAL" ]; then
echo "### $PROMPT_VAL"
sed -n '1,220p' "$PROMPT_VAL" | cat -n | sed -n '1,220p'
fi
if [ -f "$PROMPT_MSG" ]; then
echo "### $PROMPT_MSG"
sed -n '1,240p' "$PROMPT_MSG" | cat -n | sed -n '1,240p'
fi
echo
echo "---- Search for 'PromptType.Text' / 'PromptType.Chat' usage in Oxford view feature ----"
rg -n "PromptType\.(Chat|Text)|chatPrompt|textPrompt" web/src/features/oxford-views/componentsRepository: flipkart-incubator/langfuse
Length of output: 15067
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/src/features/oxford-views/components/NewOxfordViewForm/index.tsx"
echo "---- File: $FILE (lines 1-220) ----"
cat -n "$FILE" | sed -n '1,220p'
echo
echo "---- File: $FILE (lines 220-420) ----"
cat -n "$FILE" | sed -n '220,420p'
echo
echo "---- Inspect oxford view detail for how prompt payload is rendered (if needed) ----"
DETAIL="web/src/features/oxford-views/components/oxford-view-detail.tsx"
if [ -f "$DETAIL" ]; then
cat -n "$DETAIL" | sed -n '240,360p'
fi
echo
echo "---- Ripgrep: PromptChatMessageListSchema.safeParse usage in oxford views ----"
rg -n "PromptChatMessageListSchema\.safeParse" web/src/features/oxford-views/components || trueRepository: flipkart-incubator/langfuse
Length of output: 22531
Replace Oxford view chatPrompt’s z.array(z.any()) with the shared PromptChatMessageListSchema
web/src/features/oxford-views/components/NewOxfordViewForm/validation.ts imports PromptChatMessageListSchema, but both NewChatOxfordViewSchema.chatPrompt (lines ~26-48) and NewTextOxfordViewSchema.chatPrompt (line ~52) still use z.array(z.any()). The current refinements only validate placeholder shape via PlaceholderMessageSchema and that non-placeholder content isn’t empty, but they don’t enforce the shared contract that non-placeholder items must be { role, content } and chat placeholders must match { type: Placeholder, name }. Replace z.array(z.any()) with PromptChatMessageListSchema and keep/chain the existing “non-empty message” refinement so invalid payloads are rejected before submit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/oxford-views/components/NewOxfordViewForm/validation.ts`
around lines 27 - 53, Replace the ad-hoc z.array(z.any()) used for the
chatPrompt field with the shared PromptChatMessageListSchema in both
NewOxfordViewBaseSchema (where the placeholder/name and non-empty refinements
are defined) and NewTextOxfordViewSchema.chatPrompt; keep the existing
placeholder shape check using PlaceholderMessageSchema and the non-empty content
.refine chain but operate on PromptChatMessageListSchema (or chain .refine calls
after PromptChatMessageListSchema) so the shared contract ({ role, content } for
normal items and { type: Placeholder, name } for placeholders) is enforced and
the empty-message refinement still runs to reject invalid payloads before
submit.
Comment on lines
+125
to
+137
| const { pythonCode, jsCode } = useMemo(() => { | ||
| if (!view?.id) return { pythonCode: null, jsCode: null }; | ||
| const sortedLabels = [...view.labels].sort((a, b) => { | ||
| if (a === PRODUCTION_LABEL) return -1; | ||
| if (b === PRODUCTION_LABEL) return 1; | ||
| return a.localeCompare(b); | ||
| }); | ||
| return { | ||
| pythonCode: getPythonCode(view.name, view.version, sortedLabels), | ||
| jsCode: getJsCode(view.name, view.version, sortedLabels), | ||
| }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [view?.id]); |
There was a problem hiding this comment.
Generated snippets can go stale after label updates (Line 125).
useMemo only depends on view?.id, but snippet content also depends on view.labels, view.version, and view.name. After label edits on the same version, “Use View” code won’t refresh.
Suggested fix
- }, [view?.id]);
+ }, [view?.id, view?.labels, view?.version, view?.name]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { pythonCode, jsCode } = useMemo(() => { | |
| if (!view?.id) return { pythonCode: null, jsCode: null }; | |
| const sortedLabels = [...view.labels].sort((a, b) => { | |
| if (a === PRODUCTION_LABEL) return -1; | |
| if (b === PRODUCTION_LABEL) return 1; | |
| return a.localeCompare(b); | |
| }); | |
| return { | |
| pythonCode: getPythonCode(view.name, view.version, sortedLabels), | |
| jsCode: getJsCode(view.name, view.version, sortedLabels), | |
| }; | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [view?.id]); | |
| const { pythonCode, jsCode } = useMemo(() => { | |
| if (!view?.id) return { pythonCode: null, jsCode: null }; | |
| const sortedLabels = [...view.labels].sort((a, b) => { | |
| if (a === PRODUCTION_LABEL) return -1; | |
| if (b === PRODUCTION_LABEL) return 1; | |
| return a.localeCompare(b); | |
| }); | |
| return { | |
| pythonCode: getPythonCode(view.name, view.version, sortedLabels), | |
| jsCode: getJsCode(view.name, view.version, sortedLabels), | |
| }; | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [view?.id, view?.labels, view?.version, view?.name]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/oxford-views/components/oxford-view-detail.tsx` around lines
125 - 137, The memoized generation of pythonCode/jsCode in useMemo only depends
on view?.id but actually uses view.name, view.version and view.labels, so update
the dependency array for the useMemo that computes pythonCode/jsCode (the block
that calls getPythonCode and getJsCode) to include view.name, view.version and
view.labels (or a stable representation like JSON.stringify(view.labels)) so
snippets recompute when labels/name/version change; keep the existing
sortedLabels/getPythonCode/getJsCode logic intact.
Comment on lines
+12
to
+18
| const OxfordViewHistoryNode = (props: { | ||
| index: number; | ||
| view: OxfordViewVersion; | ||
| currentView?: OxfordViewVersion; | ||
| currentVersion: number | undefined; | ||
| setCurrentVersion: (version: number | undefined) => void; | ||
| }) => { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify duplicate declarations for the same symbol in this file.
rg -n 'const OxfordViewHistoryNode' web/src/features/oxford-views/components/oxford-view-history.tsxRepository: flipkart-incubator/langfuse
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/src/features/oxford-views/components/oxford-view-history.tsx"
echo "---- Declarations / usage context ----"
sed -n '1,80p' "$FILE" | cat -n
echo
sed -n '110,220p' "$FILE" | cat -n
echo
echo "---- All references in this file ----"
rg -n "OxfordViewHistoryNode" "$FILE" || true
echo
echo "---- scroll-into-view / useEffect related ----"
rg -n "scrollIntoView|useEffect|currentRef\.current" "$FILE" || trueRepository: flipkart-incubator/langfuse
Length of output: 5709
Fix redeclaration and auto-scroll dependency issues in oxford-view-history.tsx.
OxfordViewHistoryNodeis declared twice in the same module scope (constat ~12-18 andexport constat ~138-161), causing a compile-time redeclaration error.- The
scrollIntoVieweffect runs based oncurrentRef.currentonly (useEffect(..., [currentRef.current])), so it won’t reliably auto-scroll whencurrentVersionchanges; includeprops.currentVersionand/orview.versionin the dependency list.
Suggested fix
-const OxfordViewHistoryNode = (props: {
+const OxfordViewHistoryItem = (props: {
@@
-export const OxfordViewHistoryNode = (props: {
+export const OxfordViewHistoryNode = (props: {
@@
- <OxfordViewHistoryNode
+ <OxfordViewHistoryItem🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/oxford-views/components/oxford-view-history.tsx` around
lines 12 - 18, There are two problems: a redeclaration of OxfordViewHistoryNode
(it appears both as a local const and later as export const) and an effect that
only depends on currentRef.current so it doesn't auto-scroll when the selected
version changes. Fix by keeping a single declaration/export of
OxfordViewHistoryNode (remove or consolidate the duplicate declaration/export so
only one OxfordViewHistoryNode exists) and update the scroll effect (the
useEffect that references currentRef and calls scrollIntoView) to include
props.currentVersion and/or view.version in its dependency array (e.g.,
[currentRef.current, props.currentVersion, view.version]) so the element scrolls
when the active version changes.
Comment on lines
+26
to
+38
| useEffect(() => { | ||
| if ( | ||
| props.currentVersion && | ||
| currentRef.current && | ||
| props.currentVersion === view.version | ||
| ) { | ||
| currentRef.current.scrollIntoView({ | ||
| behavior: "smooth", | ||
| block: "center", | ||
| }); | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [currentRef.current]); |
There was a problem hiding this comment.
Auto-scroll effect has incorrect dependencies (Line 26).
Depending on currentRef.current means the effect may not run when currentVersion changes, so active-node auto-scroll is unreliable.
Suggested fix
- useEffect(() => {
+ useEffect(() => {
if (
props.currentVersion &&
currentRef.current &&
props.currentVersion === view.version
) {
currentRef.current.scrollIntoView({
behavior: "smooth",
block: "center",
});
}
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [currentRef.current]);
+ }, [props.currentVersion, view.version]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/oxford-views/components/oxford-view-history.tsx` around
lines 26 - 38, The useEffect that auto-scrolls references currentRef.current but
only lists [currentRef.current] so it won't run when props.currentVersion or
view.version change; update the effect dependencies to include
props.currentVersion and view.version (e.g., [props.currentVersion,
view.version, currentRef]) and remove the eslint-disable comment so React
enforces correct deps; keep the existing scrollIntoView call and condition
(props.currentVersion && currentRef.current && props.currentVersion ===
view.version) but rely on the corrected dependency array to trigger the effect
reliably.
Comment on lines
+191
to
+194
| setDetailPageList( | ||
| "oxfordViews", | ||
| views.data.prompts.map((v) => ({ id: v.name })), | ||
| ); |
There was a problem hiding this comment.
Use full-path IDs in detail-page list context.
Line 193 currently stores id: v.name, but this table already computes folder-aware IDs via buildFullPath(...). In nested folders, using only name can break detail navigation and collide across folders.
Proposed fix
useEffect(() => {
if (views.isSuccess) {
setDetailPageList(
"oxfordViews",
- views.data.prompts.map((v) => ({ id: v.name })),
+ views.data.prompts.map((v) => ({
+ id: buildFullPath(currentFolderPath, v.name),
+ })),
);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [views.isSuccess, views.data]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| setDetailPageList( | |
| "oxfordViews", | |
| views.data.prompts.map((v) => ({ id: v.name })), | |
| ); | |
| setDetailPageList( | |
| "oxfordViews", | |
| views.data.prompts.map((v) => ({ | |
| id: buildFullPath(currentFolderPath, v.name), | |
| })), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/oxford-views/components/oxford-views-table.tsx` around lines
191 - 194, The detail-page list is saving folder-ambiguous IDs (id: v.name)
which breaks navigation for nested folders; update the setDetailPageList call
that maps views.data.prompts so each item uses the folder-aware ID by calling
the existing buildFullPath(...) for each prompt (e.g., replace id: v.name with
id: buildFullPath(v) or equivalent), ensuring the generated IDs match the IDs
used elsewhere for detail navigation.
Comment on lines
+244
to
+247
| const additionalConditions = input.searchType?.includes("id") | ||
| ? [ | ||
| Prisma.sql`EXISTS (SELECT 1 FROM UNNEST(p.tags) AS tag WHERE tag ILIKE ${`%${input.searchQuery}%`})`, | ||
| ] |
There was a problem hiding this comment.
Guard tag-search condition when searchQuery is absent.
When searchType contains "id" and searchQuery is undefined, the condition becomes ILIKE '%undefined%', which silently skews results.
Suggested fix
- const additionalConditions = input.searchType?.includes("id")
+ const additionalConditions =
+ input.searchType?.includes("id") && input.searchQuery
? [
Prisma.sql`EXISTS (SELECT 1 FROM UNNEST(p.tags) AS tag WHERE tag ILIKE ${`%${input.searchQuery}%`})`,
]
: [];Also applies to: 335-338
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/oxford-views/server/routers/oxfordViewRouter.ts` around
lines 244 - 247, The tag-search Prisma.sql condition currently runs even when
input.searchQuery is undefined, producing ILIKE '%undefined%'; update the
construction of additionalConditions (the branch that checks
input.searchType?.includes("id")) to first ensure input.searchQuery is a
non-empty string before adding the Prisma.sql entry, and if empty/undefined skip
adding that tag ILIKE condition; apply the same guard to the other occurrence
that builds a similar Prisma.sql tag ILIKE condition so neither location
constructs an ILIKE '%undefined%' predicate.
Comment on lines
+669
to
+703
| filterOptions: protectedProjectProcedure | ||
| .input(z.object({ projectId: z.string() })) | ||
| .query(async ({ input, ctx }) => { | ||
| const [names, tags, labels] = await Promise.all([ | ||
| ctx.prisma.oxfordView.groupBy({ | ||
| where: { projectId: input.projectId }, | ||
| by: ["name"], | ||
| take: 1000, | ||
| orderBy: { name: "asc" }, | ||
| }), | ||
| ctx.prisma.$queryRaw<{ value: string }[]>` | ||
| SELECT tags.tag as value | ||
| FROM oxford_views, UNNEST(oxford_views.tags) AS tags(tag) | ||
| WHERE oxford_views.project_id = ${input.projectId} | ||
| GROUP BY tags.tag | ||
| ORDER BY tags.tag ASC; | ||
| `, | ||
| ctx.prisma.$queryRaw<{ value: string }[]>` | ||
| SELECT labels.label as value | ||
| FROM oxford_views, UNNEST(oxford_views.labels) AS labels(label) | ||
| WHERE oxford_views.project_id = ${input.projectId} | ||
| GROUP BY labels.label | ||
| ORDER BY labels.label ASC; | ||
| `, | ||
| ]); | ||
|
|
||
| return { | ||
| name: names | ||
| .filter((n) => n.name !== null) | ||
| .map((n) => ({ value: n.name })), | ||
| labels, | ||
| tags, | ||
| }; | ||
| }), | ||
|
|
There was a problem hiding this comment.
Add missing project access check in filterOptions.
This procedure reads project-scoped data but skips throwIfNoProjectAccess, allowing unauthorized reads by arbitrary projectId.
Suggested fix
filterOptions: protectedProjectProcedure
.input(z.object({ projectId: z.string() }))
.query(async ({ input, ctx }) => {
+ throwIfNoProjectAccess({
+ session: ctx.session,
+ projectId: input.projectId,
+ scope: "prompts:read",
+ });
+
const [names, tags, labels] = await Promise.all([🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/oxford-views/server/routers/oxfordViewRouter.ts` around
lines 669 - 703, The filterOptions procedure performs project-scoped reads
without verifying the caller's access; add a project access check at the start
of the query handler by invoking the existing throwIfNoProjectAccess (or
equivalent) with the input.projectId and ctx before any ctx.prisma queries. In
practice, inside filterOptions's .query(async ({ input, ctx }) => { ... }), call
await throwIfNoProjectAccess(input.projectId, ctx) (or
ctx.throwIfNoProjectAccess...) immediately, then proceed to run the three
Promise.all queries and return the same result shape.
…r/langfuse into OxfordViewsIntegration # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
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.
What does this PR do?
Fixes # (issue)
Type of change
Mandatory Tasks
Checklist
pnpm run format)npm run lint)Summary by CodeRabbit
Release Notes