-
-
Notifications
You must be signed in to change notification settings - Fork 8
Santiago seisdedos implementation #2
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
base: main
Are you sure you want to change the base?
Santiago seisdedos implementation #2
Conversation
Hola Santi! Soy Joaco de NaNLABS. Estoy a cargo de revisar tu challenge. 🥷 Muy bueno lo que vi hasta ahora, y muchas gracias por la demo! Me queda pendiente para el lunes revisar más en detalle para poder dejarte algo de feedback más concreto. Que tengas un gran finde! |
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.
Hola de nuevo Santi! Te dejo algunos comentarios. La verdad que en general lo vi muy prolijo y me gustaron varias de las ideas que tuviste e implementaste. Por ejemplo el Chain of Thought es genial de ver y de usar, y lo de los costos puede ser muy útil más allá de que tiene algún bug dando vueltas.
Pude levantarlo localmente, ahí usé los pasos de la documentación SETUP_AND_USAGE.md
y tuve algunos problemas menores. En instructions.md
hay una lista similar de pasos para levantar los servicios, y en la descripción del PR hay otra más (que parece la correcta).
Me funcionó bien, probé varios casos y tanto el backend como la UI son bastante responsivos e intuitivos. Ahí te dejé una sugerencia también para el hook del websocket que parece que está un poco bugueado.
Fuiste con todo y el resultado quedó muy bien. Muchos de los "nice to have" que se mencionan en la consigna también los incluiste y eso suma puntos.
Como curiosidad, te consulto:
- Usaste alguna herramienta de GenAI? Cuáles y por qué?
- Te quedaste con ganas de mejorar alguna parte del sistema? Cuál/es?
Desde ya muchas gracias por el tiempo que le dedicaste! 🧨
import { useState, useEffect, useCallback } from "react"; | ||
import { socketService } from "@/lib/websocket/socket"; | ||
|
||
interface ChainOfThoughtsState { |
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.
Está buenísimo este componente. Además de que se ve muy bien, es clave para el usuario saber qué está haciendo el LLM específicamente. Y sumándole la implementación de websockets se lo podemos mostrar en tiempo real.
Es toltamente overkill para lo que es el challenge pero está genial que tires magias 🪄
COPY package*.json ./ | ||
|
||
# Install dependencies | ||
RUN npm ci --only=production --legacy-peer-deps |
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.
Al intentar levantar locamente, me falló el compose en esta parte.
El --only=production
provoca que no se instale @nestjs/cli (dev), entonces no existe nest para compilar. Solución: (1) quitar la flag en dev (lo que hice yo, suficiente para ambientes no productivos), o (2) usar multi-stage: compilar con dev deps y luego prunar a prod para el runtime.
Ejemplo de la opción 2 (AI generated así que puede fallar):
# deps (instala dev+prod)
FROM node:18-alpine AS deps
WORKDIR /app
COPY package*.json ./
RUN npm ci --legacy-peer-deps
# build (compila)
FROM node:18-alpine AS builder
WORKDIR /app
COPY --from=deps /app/node_modules ./node_modules
COPY . .
RUN npx prisma generate
RUN npm run build
# runtime (solo prod)
FROM node:18-alpine AS runner
WORKDIR /app
ENV NODE_ENV=production
COPY package*.json ./
COPY --from=builder /app/node_modules ./node_modules
RUN npm prune --production
COPY --from=builder /app/dist ./dist
COPY --from=builder /app/prisma ./prisma
EXPOSE 3001
CMD ["node", "dist/main.js"]
cd fullstack-engineer-ai-content-workflow-challenge | ||
|
||
# Copy environment variables | ||
cp .env.example .env |
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.
Acá agregaría que también se necesita copiar el .env
del directorio /backend
, para que prisma tenga de donde sacar el DATABASE_URL
cd backend | ||
|
||
# Run database migrations | ||
npx prisma migrate dev --name init |
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.
Siguiendo con el comentario de arriba, si no copiaramos el .env
podríamos aprovechar las variables de entorno que ya tenemos en el container y correr el comando dentro del mismo, como mencionás en la descripción del PR
-d '{ | ||
"name": "Q4 Product Launch", | ||
"description": "Marketing campaign for our new product launch", | ||
"targetAudience": "Tech professionals", |
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.
Tanto targetAudience
como budget
quedaron viejos de algún draft. Los demás curl
me respondieron perfecto
connectionStatus: 'connecting' | 'connected' | 'disconnected' | 'error'; | ||
} | ||
|
||
export const useWebSocket = () => { |
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.
Este debe tener un bug en algún lado, por lo que vimos en la demo de los múltiples toast. Localmente me pasó lo mismo y en red me figura que tengo 3 websocket con 3 client id diferentes. Algunas de las cosas que se pueden revisar:
- Hacer el efecto sin dependencias (
[]
) para no recrear la conexión por cambios deaddToast
. - Desconectar el socket en el cleanup del mismo efecto.
- (Opcional) Hacer que
socketService.connect()
sea idempotente (si ya hay socket, reutilizarlo).
@@ -0,0 +1,191 @@ | |||
// Centralized error handling and API response management |
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.
Siempre falta algo como esto en los proyectos y nos damos cuenta demasiado tarde. Una solución sencilla para error handling 🔥
@@ -0,0 +1,557 @@ | |||
"use client"; | |||
|
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.
Este file es gigante. Un enfoque que se puede tomar acá es buscar alguna librería de manejo de estados que nos aliviane un poco la carga.
Ejemplo con zustand:
// store/realtime.ts
import { create } from "zustand";
import { immer } from "zustand/middleware/immer";
import { socketService } from "@/lib/websocket/socket";
type State = {
campaigns: Campaign[];
contentPieces: Record<string, ContentPiece[]>;
drafts: Record<string, Draft[]>;
documents: Record<string, Document[]>;
hydrate: () => void; // registra listeners 1 sola vez
};
export const useRealtime = create<State>()(immer((set, get) => ({
campaigns: [],
contentPieces: {},
drafts: {},
documents: {},
hydrate: () => {
// idempotente
socketService.connect();
socketService.onCampaignCreated((c) => {
set(s => { s.campaigns.push(c as any); });
});
// ...el resto de eventos con set(s => {...}) y listo
},
})));
}; | ||
|
||
// Get user-friendly error message from status code | ||
export const getErrorMessage = ( |
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.
Tanto getErrorMessage
como handleApiResponse
son usados solo en este file así que no debería ser necesario exportarlos
Description
This PR delivers my implementation for the Fullstack Engineer Challenge – AI Content Workflow.
Summary of Changes
Backend (NestJS + Prisma + PostgreSQL)
Frontend (Next.js + React Query + Tailwind + shadcn/ui)
Infrastructure
.env.example
with required API keys and connection stringsSETUP_AND_USAGE.md
Fixes # (no issue was opened – challenge submission).
Type of Change
How Has This Been Tested?
npm run dev
):Demo Video
Quick Start (recommended)
1. Clone fork and checkout branch
git clone <your-fork-url>
cd fullstack-engineer-ai-content-workflow-challenge
git fetch --all
git checkout SantiagoSeisdedosImplementation
2. Environment setup
cp .env.example .env
3. Run all services
docker-compose up -d
docker-compose ps
# check container status4. Apply Prisma migrations inside backend container
docker exec -it acme-backend npx prisma migrate deploy
5. Access frontend & backend
🔗frontend
🔗backend
Dev Mode (optional, if you want to run frontend/backend locally)
Run infra only (Postgres + Redis)
docker-compose up -d postgres redis
Backend (terminal 1)
cd backend
npm install
npx prisma migrate dev --name init
npm run start:dev
Frontend (terminal 2)
cd frontend
npm install
npm run dev