Skip to content

Merge pull request #1 from 866-lgtm/codex/check-for-bugs-and-optimizatio #7

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.

Sign up for GitHub

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

Open
866-lgtm wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Merge pull request #1 from 866-lgtm/codex/check-for-bugs-and-optimizatio #7

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8412745
Fix notification safety and reduce duplicate collection checks
866-lgtm Mar 15, 2026
fc430e5
Merge pull request #1 from 866-lgtm/codex/check-for-bugs-and-optimiza…
866-lgtm Mar 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 30 additions & 29 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,25 @@ function getCollectionName(characterName) {
}

// Sanitize character name for collection name (lowercase, replace spaces/special chars)
const sanitized = characterName
const sanitized = String(characterName || "")
.toLowerCase()
.replace(/[^a-z0-9_-]/g, "_")
.replace(/_+/g, "_")
.replace(/^_|_$/g, "")

return `${settings.collectionName}_${sanitized}`
return `${settings.collectionName}_${sanitized || "unknown"}`
}

function showNotification(level, message, title = "Qdrant Memory", options = {}) {
const toastr = window.toastr
if (toastr && typeof toastr[level] === "function") {
toastr[level](message, title, options)
return
}

if (settings.debugMode) {
console.log(`[Qdrant Memory] ${level.toUpperCase()}: ${message}`)
}
}
Comment on lines +223 to 233
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 15, 2026

Choose a reason for hiding this comment

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

Silent notification drop in non-debug mode

When window.toastr is not available and settings.debugMode is false, all notification calls are silently swallowed — the user receives no feedback at all. The original code would have thrown a runtime error (which is worse), but the current fallback could also leave the user confused with no visible response to their actions.

Consider always emitting a console.warn (outside the debugMode guard) so that at minimum a developer inspecting the console can diagnose missing notifications:

Suggested change
function showNotification(level, message, title = "Qdrant Memory", options = {}) {
const toastr = window.toastr
if (toastr && typeof toastr[level] === "function") {
toastr[level](message, title, options)
return
}
if (settings.debugMode) {
console.log(`[Qdrant Memory] ${level.toUpperCase()}: ${message}`)
}
}
function showNotification(level, message, title = "Qdrant Memory", options = {}) {
const toastr = window.toastr
if (toastr && typeof toastr[level] === "function") {
toastr[level](message, title, options)
return
}
console.warn(`[Qdrant Memory] toastr unavailable — ${level.toUpperCase()}: ${message}`)
}

This removes the debugMode gate from the fallback so the notification is never silently discarded.


// Get embedding dimensions for the selected model
Expand Down Expand Up @@ -1013,6 +1025,7 @@ async function saveChunkToQdrant(chunk, participants) {

// NEW: Check for duplicates before saving
let alreadyExists = false
const readyParticipants = []

for (const characterName of participants) {
const collectionName = getCollectionName(characterName)
Expand All @@ -1023,6 +1036,8 @@ async function saveChunkToQdrant(chunk, participants) {
continue
}

readyParticipants.push(characterName)

const exists = await chunkExistsInCollection(
collectionName,
embedding,
Expand All @@ -1041,8 +1056,7 @@ async function saveChunkToQdrant(chunk, participants) {

if (alreadyExists) {
if (settings.showMemoryNotifications) {
const toastr = window.toastr
toastr.info("Similar conversation already saved", "Qdrant Memory", { timeOut: 1500 })
showNotification("info", "Similar conversation already saved", "Qdrant Memory", { timeOut: 1500 })
}
return false
}
Comment on lines 1057 to 1062
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 15, 2026

Choose a reason for hiding this comment

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

readyParticipants is incomplete when loop breaks early

When a duplicate is detected in participant i, the loop breaks immediately. Participants at positions i+1 and beyond have not had ensureCollection called and are therefore absent from readyParticipants. Because the function returns false immediately in this branch, the incomplete list is harmless today.

However, if this block is ever refactored (e.g., to save the checked participants even when a duplicate is found), the subtle incompleteness of readyParticipants could introduce a silent data-loss bug. Adding a short comment here would make the invariant explicit:

Suggested change
if (alreadyExists) {
if (settings.showMemoryNotifications) {
const toastr = window.toastr
toastr.info("Similar conversation already saved", "Qdrant Memory", { timeOut: 1500 })
showNotification("info", "Similar conversation already saved", "Qdrant Memory", { timeOut: 1500 })
}
return false
}
if (alreadyExists) {
// readyParticipants may be a partial list here (loop broke early); only used
// in the save path below, which is skipped on this return branch.
if (settings.showMemoryNotifications) {
showNotification("info", "Similar conversation already saved", "Qdrant Memory", { timeOut: 1500 })
}
return false
}

Expand All @@ -1060,16 +1074,9 @@ async function saveChunkToQdrant(chunk, participants) {
}

// Save to all participant collections
const savePromises = participants.map(async (characterName) => {
const savePromises = readyParticipants.map(async (characterName) => {
const collectionName = getCollectionName(characterName)

// Ensure collection exists
const collectionReady = await ensureCollection(characterName, embedding.length)
if (!collectionReady) {
console.error(`[Qdrant Memory] Cannot save chunk - collection creation failed for ${characterName}`)
return false
}

// Add character name to payload only if using shared collection
const characterPayload = settings.usePerCharacterCollections
? payload
Expand Down Expand Up @@ -1110,7 +1117,7 @@ async function saveChunkToQdrant(chunk, participants) {
const successCount = results.filter((r) => r).length

if (settings.debugMode) {
console.log(`[Qdrant Memory] Chunk saved to ${successCount}/${participants.length} collections`)
console.log(`[Qdrant Memory] Chunk saved to ${successCount}/${readyParticipants.length} collections`)
}

return successCount > 0
Expand Down Expand Up @@ -1486,17 +1493,16 @@ function createChunkFromMessages(messages) {
async function indexCharacterChats() {
const context = getContext()
const characterName = context.name2
const toastr = window.toastr
const $ = window.$

if (!characterName) {
toastr.warning("No character selected", "Qdrant Memory")
showNotification("warning", "No character selected")
return
}

const providerError = getEmbeddingProviderError()
if (providerError) {
toastr.error(providerError, "Qdrant Memory")
showNotification("error", providerError)
return
}

Expand Down Expand Up @@ -1626,18 +1632,18 @@ async function indexCharacterChats() {

if (cancelled) {
$("#qdrant_index_status").text("Indexing cancelled")
toastr.info(`Indexed ${savedChunks} chunks before cancelling`, "Qdrant Memory")
showNotification("info", `Indexed ${savedChunks} chunks before cancelling`)
} else {
$("#qdrant_index_status").text("Indexing complete!")
toastr.success(`Indexed ${savedChunks} new chunks, skipped ${skippedChunks} existing`, "Qdrant Memory")
showNotification("success", `Indexed ${savedChunks} new chunks, skipped ${skippedChunks} existing`)
}

setCancelButtonToClose()
} catch (error) {
console.error("[Qdrant Memory] Error indexing chats:", error)
$("#qdrant_index_status").text("Error during indexing")
$("#qdrant_index_details").text(error.message)
toastr.error("Failed to index chats", "Qdrant Memory")
showNotification("error", "Failed to index chats")
setCancelButtonToClose()
}
}
Expand Down Expand Up @@ -1737,9 +1743,8 @@ globalThis.qdrantMemoryInterceptor = async (chat, contextSize, abort, type) => {
console.log(`[Qdrant Memory] Injected ${memories.length} memories at position ${insertIndex}`)
}

const toastr = window.toastr
if (settings.showMemoryNotifications) {
toastr.info(`Retrieved ${memories.length} relevant memories`, "Qdrant Memory", { timeOut: 2000 })
showNotification("info", `Retrieved ${memories.length} relevant memories`, "Qdrant Memory", { timeOut: 2000 })
}
} else {
if (settings.debugMode) {
Expand Down Expand Up @@ -1990,17 +1995,15 @@ async function showMemoryViewer() {
const characterName = context.name2

if (!characterName) {
const toastr = window.toastr
toastr.warning("No character selected", "Qdrant Memory")
showNotification("warning", "No character selected")
return
}

const collectionName = getCollectionName(characterName)
const info = await getCollectionInfo(collectionName)

if (!info) {
const toastr = window.toastr
toastr.warning(`No memories found for ${characterName}`, "Qdrant Memory")
showNotification("warning", `No memories found for ${characterName}`)
return
}

Expand Down Expand Up @@ -2061,13 +2064,11 @@ async function showMemoryViewer() {
$(this).prop("disabled", true).text("Deleting...")
const success = await deleteCollection(collectionName)
if (success) {
const toastr = window.toastr
toastr.success(`All memories deleted for ${characterName}`, "Qdrant Memory")
showNotification("success", `All memories deleted for ${characterName}`)
$("#qdrant_modal").remove()
$("#qdrant_overlay").remove()
} else {
const toastr = window.toastr
toastr.error("Failed to delete memories", "Qdrant Memory")
showNotification("error", "Failed to delete memories")
$(this).prop("disabled", false).text("Delete All Memories")
}
}
Expand Down