fix(brain): address code quality review findings
- Move BrainMemory::create() inside BrainService::remember() for full atomicity (DB + Qdrant in single transaction) - Add forWorkspace() scope to recall() MariaDB query (tenant isolation) - Wrap forget() in DB::transaction (MariaDB first, then Qdrant) - Check qdrantDelete() response and log warnings on failure - Validate embed() response is a non-empty array Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
parent
2c6a095a0e
commit
d82ad2b9b1
2 changed files with 41 additions and 22 deletions
|
|
@ -130,7 +130,7 @@ class BrainRemember extends AgentTool
|
|||
return $this->withCircuitBreaker('brain', function () use (
|
||||
$content, $type, $workspaceId, $agentId, $tags, $project, $confidence, $supersedes, $expiresIn
|
||||
) {
|
||||
$memory = BrainMemory::create([
|
||||
$memory = app(BrainService::class)->remember([
|
||||
'workspace_id' => $workspaceId,
|
||||
'agent_id' => $agentId,
|
||||
'type' => $type,
|
||||
|
|
@ -142,8 +142,6 @@ class BrainRemember extends AgentTool
|
|||
'expires_at' => $expiresIn ? now()->addHours($expiresIn) : null,
|
||||
]);
|
||||
|
||||
app(BrainService::class)->remember($memory);
|
||||
|
||||
return $this->success([
|
||||
'memory' => $memory->toMcpContext(),
|
||||
]);
|
||||
|
|
|
|||
|
|
@ -40,37 +40,51 @@ class BrainService
|
|||
throw new \RuntimeException("Ollama embedding failed: {$response->status()}");
|
||||
}
|
||||
|
||||
return $response->json('embedding');
|
||||
$embedding = $response->json('embedding');
|
||||
|
||||
if (! is_array($embedding) || empty($embedding)) {
|
||||
throw new \RuntimeException('Ollama returned no embedding vector');
|
||||
}
|
||||
|
||||
return $embedding;
|
||||
}
|
||||
|
||||
/**
|
||||
* Store a memory in both MariaDB and Qdrant.
|
||||
*
|
||||
* If the memory supersedes an older one, the old entry is
|
||||
* removed from both stores.
|
||||
* Creates the MariaDB record and upserts the Qdrant vector within
|
||||
* a single DB transaction. If the memory supersedes an older one,
|
||||
* the old entry is soft-deleted from MariaDB and removed from Qdrant.
|
||||
*
|
||||
* @param array<string, mixed> $attributes Fillable attributes for BrainMemory
|
||||
* @return BrainMemory The created memory
|
||||
*/
|
||||
public function remember(BrainMemory $memory): void
|
||||
public function remember(array $attributes): BrainMemory
|
||||
{
|
||||
$vector = $this->embed($memory->content);
|
||||
$vector = $this->embed($attributes['content']);
|
||||
|
||||
$payload = $this->buildQdrantPayload($memory->id, [
|
||||
'workspace_id' => $memory->workspace_id,
|
||||
'agent_id' => $memory->agent_id,
|
||||
'type' => $memory->type,
|
||||
'tags' => $memory->tags ?? [],
|
||||
'project' => $memory->project,
|
||||
'confidence' => $memory->confidence,
|
||||
'created_at' => $memory->created_at->toIso8601String(),
|
||||
]);
|
||||
$payload['vector'] = $vector;
|
||||
return DB::transaction(function () use ($attributes, $vector) {
|
||||
$memory = BrainMemory::create($attributes);
|
||||
|
||||
$payload = $this->buildQdrantPayload($memory->id, [
|
||||
'workspace_id' => $memory->workspace_id,
|
||||
'agent_id' => $memory->agent_id,
|
||||
'type' => $memory->type,
|
||||
'tags' => $memory->tags ?? [],
|
||||
'project' => $memory->project,
|
||||
'confidence' => $memory->confidence,
|
||||
'created_at' => $memory->created_at->toIso8601String(),
|
||||
]);
|
||||
$payload['vector'] = $vector;
|
||||
|
||||
DB::transaction(function () use ($payload, $memory) {
|
||||
$this->qdrantUpsert([$payload]);
|
||||
|
||||
if ($memory->supersedes_id) {
|
||||
BrainMemory::where('id', $memory->supersedes_id)->delete();
|
||||
$this->qdrantDelete([$memory->supersedes_id]);
|
||||
}
|
||||
|
||||
return $memory;
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -112,6 +126,7 @@ class BrainService
|
|||
}
|
||||
|
||||
$memories = BrainMemory::whereIn('id', $ids)
|
||||
->forWorkspace($workspaceId)
|
||||
->active()
|
||||
->latestVersions()
|
||||
->get()
|
||||
|
|
@ -129,8 +144,10 @@ class BrainService
|
|||
*/
|
||||
public function forget(string $id): void
|
||||
{
|
||||
$this->qdrantDelete([$id]);
|
||||
BrainMemory::where('id', $id)->delete();
|
||||
DB::transaction(function () use ($id) {
|
||||
BrainMemory::where('id', $id)->delete();
|
||||
$this->qdrantDelete([$id]);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -236,9 +253,13 @@ class BrainService
|
|||
*/
|
||||
private function qdrantDelete(array $ids): void
|
||||
{
|
||||
Http::timeout(10)
|
||||
$response = Http::timeout(10)
|
||||
->post("{$this->qdrantUrl}/collections/{$this->collection}/points/delete", [
|
||||
'points' => $ids,
|
||||
]);
|
||||
|
||||
if (! $response->successful()) {
|
||||
Log::warning("Qdrant delete failed: {$response->status()}", ['ids' => $ids, 'body' => $response->body()]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Reference in a new issue