Skip to content

Conversation

@msiessedblcm
Copy link
Collaborator

No description provided.


it('broadcasts an error when the LLM client throws', function () {
// Bind a permissive moderation
app()->bind(ModerationClient::class, fn () => new class implements ModerationClient {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: could use fake implementation and just making it safe by default

});

// Bind an LLM client that throws
app()->bind(LlmClient::class, fn () => new class implements LlmClient {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: can use fake llm client and force to throw exception

<template>
<div class="min-h-screen w-full flex items-center justify-center bg-gradient-to-b from-slate-50 to-slate-100 p-6">
<div class="w-full max-w-md">
<div class="bg-white/90 backdrop-blur rounded-2xl shadow-xl p-8 border border-slate-200">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could have been merged with abode div


<form @submit.prevent="submit" class="space-y-4">
<div>
<label class="block text-sm font-medium text-slate-700 mb-1">Email</label>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for attribute is missing


<button type="submit" :disabled="loading"
class="w-full inline-flex items-center justify-center gap-2 rounded-xl bg-indigo-600 text-white font-medium px-4 py-2.5 hover:bg-indigo-700 disabled:opacity-60 disabled:cursor-not-allowed transition">
<svg v-if="loading" class="h-5 w-5 animate-spin" viewBox="0 0 24 24" fill="none">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could have been an asset

const email = ref('')
const password = ref('')
const { user, fetchUser } = useAuth()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch user could have been directly chained in useAuth.
useAuth composable could have been something like :

const { user, isLoggedIn } = useAuth()

gap: 1rem;
const sendPrompt = async () => {
if (!prompt.value.trim() || loading.value) return
loading.value = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loading.value should be mutated in useLLMChannel composable

margin-right: 1rem;
width: 70%;
}
const envelope = computed<LlmResultEnvelope | null>(() => result.value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessary


<script setup lang="ts">
import type { LlmErrorResult, LlmPayload } from '@/types/llm'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type definitions are missing

(r as LlmErrorResult).status === 'error'
</script>

<style scoped>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be removed

</div>

<!-- Error -->
<div v-else-if="isError(data.result)" class="rounded-xl border border-rose-200 bg-rose-50 text-rose-700 p-4"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to many as


channel = echo.private('analysis')

const { data } = await api.post('/api/prompt', { prompt })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should managed errors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants