Features/refine chat window

[iceljc]

PR Type

Enhancement, Bug fix

---

Description

• Refactor global loading and error handling to use centralized store-based approach

• Relocate spinner positioning from fixed to absolute for better layout control

• Enhance navigation to use SvelteKit's goto() instead of direct URL manipulation

• Improve sidebar menu item selection and scrolling with URL-based ID matching

• Add abort signal support for cancellable async requests in agent listing

• Add page mount state tracking to prevent navigation callbacks after unmount

---

Diagram Walkthrough

flowchart LR
  A["Global Layout"] -->|removed loader logic| B["GlobalHeader Component"]
  B -->|subscribes to stores| C["loaderStore & errorStore"]
  C -->|provides state| D["VerticalLayout & ChatBox"]
  D -->|uses LoadingToComplete| E["Spinner Display"]
  F["Navigation"] -->|uses goto| G["SvelteKit Router"]
  G -->|replaces window.location| H["Client-side Navigation"]

Loading

File Walkthrough

	Relevant files
Enhancement	22 files LoadingToComplete.svelte Add spinner styling customization props                                    +8/-2      GlobalHeader.svelte Create new global header store subscription component        +29/-0    Index.svelte Add GlobalHeader and LoadingToComplete integration              +14/-1    Sidebar.svelte Refactor menu navigation and URL handling logic                    +39/-26  +page.svelte Replace window.location with SvelteKit goto                            +6/-4      chat-box.svelte Add GlobalHeader and refine spinner positioning                    +13/-4    +page.svelte Add abort controller and page mount state tracking              +22/-4    +page.svelte Refactor agent loading with reactive statements                    +19/-6    +page.svelte Refactor agent loading with reactive statements                    +17/-9    card-agent.svelte Replace Link with Button for navigation control                    +23/-13  +page.svelte Add page mount state tracking for safe navigation                +12/-12  +page.svelte Add dialog loading and refactor component structure            +41/-27  +page.svelte Add page mount state tracking for safe navigation                +8/-5      +page.svelte Add collection details fetching and conditional loading    +31/-10  +page.svelte Add conditional collection detail loading                                +12/-10  +page.svelte Add page mount state tracking for safe navigation                +7/-1      +page.svelte Add loading state management and cleanup unused code          +8/-9      +page.svelte Add page mount state tracking for safe navigation                +14/-2    _common.scss Add text-btn styling for button-as-link elements                  +14/-0    http.js Refine loader skip patterns and remove debug logs                +20/-12  common.js Add getCleanUrl utility function                                                  +13/-0    agent-service.js Add abort signal parameter for cancellable requests            +4/-2
Refactoring	2 files +layout.svelte Remove global loader and error handling logic                        +0/-36    conv-dialogs.svelte Move dialog fetching to parent component                                  +7/-9
Formatting	7 files +page.svelte Clean up unused imports and variables                                        +2/-4      +layout.svelte Reorder imports for consistency                                                    +1/-2      agent-rule.svelte Format LoadingToComplete component usage                                  +7/-1      +page.svelte Format LoadingToComplete and improve indentation                  +37/-32  +page.svelte Format LoadingToComplete and improve indentation                  +8/-3      +page.svelte Format LoadingToComplete component usage                                  +7/-2      +page.svelte Format LoadingToComplete component usage                                  +1/-0
Bug fix	2 files _loader.scss Change loader positioning from fixed to absolute                  +1/-1      role-service.js Change getRoles from POST to GET with query params              +5/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Debug info exposure Description: Debug logging of the token refresh retry queue length via console.log ('queue', this.queue.length) was introduced, which can leak internal state and aid attackers in probing auth flow or DoS conditions; remove verbose logs or guard them by environment. http.js [31-35] Referred Code /** @param {{config: import('axios').InternalAxiosRequestConfig, resolve: (value: any) => void, reject: (reason?: any) => void}} item */ enqueue(item) { this.queue.push(item); console.log('queue', this.queue.length); if (!this.isRefreshingToken) {
	Selector injection risk Description: The code builds a CSS selector using an unescaped URL-derived value in document.querySelector(".vertical-menu a[id='" + curUrl + "']"), which can break the selector or enable DOM selection manipulation if an attacker can influence IDs via crafted links; sanitize or escape selector components before concatenation. Sidebar.svelte [20-25] Referred Code const curUrl = getCleanUrl($page.url.pathname); if (curUrl) { const item = document.querySelector(".vertical-menu a[id='" + curUrl + "']"); if (item) { item.classList.add('mm-active'); const parent1 = item.parentElement;
	Unvalidated DOM id from URL Description: Navigation links set element id attributes from getCleanUrl(user-controlled link) and use them in selectors, potentially allowing DOM manipulation or unexpected behavior if IDs contain special characters; validate/escape IDs or use data attributes. Sidebar.svelte [221-260] Referred Code <li class="menu-title" key="t-menu">{$_(item.label)}</li> {:else if item.subMenu} <li> <Link class="has-arrow waves-effect clickable" href={null}> <i class={item.icon} /> <span>{$_(item.label)}</span> </Link> <ul class="sub-menu mm-collapse"> {#each item.subMenu as subMenu} {#if subMenu.isChildItem} <li> <Link class="has-arrow waves-effect clickable" href={null}> <span>{$_(subMenu.label)}</span> </Link> <ul class="sub-menu mm-collapse"> {#each subMenu.childItems as childItem} <li> <Link class="clickable" id={getCleanUrl(childItem.link)} href={null} on:click={() => goToPage(childItem.link)}> {$_(childItem.label)} </Link> </li> ... (clipped 19 lines)
	Request cancellation race Description: Added optional AbortSignal handling to getAgents without clearing or resetting the abort controller in callers may lead to request cancellation race conditions; if attacker can trigger rapid requests, this could cause inconsistent UI/state—ensure abort controllers are properly scoped and errors handled distinctly. agent-service.js [16-36] Referred Code * Get agent list * @param {import('$agentTypes').AgentFilter} filter * @param {boolean} checkAuth * @param {AbortSignal | null} signal * @returns {Promise<import('$commonTypes').PagedItems<import('$agentTypes').AgentModel>>} */ export async function getAgents(filter, checkAuth = false, signal = null) { let url = endpoints.agentListUrl; const response = await axios.get(url, { params: { ...filter, checkAuth : checkAuth }, paramsSerializer: { dots: true, indexes: null, }, signal: signal || undefined }); return response.data; }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Console Logging: The code introduces a console.log of the retry queue length which could leak internal details and should not appear in production-facing code. Referred Code /** @param {{config: import('axios').InternalAxiosRequestConfig, resolve: (value: any) => void, reject: (reason?: any) => void}} item */ enqueue(item) { this.queue.push(item); console.log('queue', this.queue.length); if (!this.isRefreshingToken) { Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing Audit Logs: The PR adds navigation changes, abortable requests, and state changes across multiple pages without adding or referencing any audit logging for critical actions, which may be handled elsewhere but is not visible in this diff. Referred Code loaderStore.set(false); const originalRequest = error?.config || {}; const user = getUserStore(); if (!user?.token || user.renew_token_count >= retryQueue.maxRenewTokenCount) { retryQueue.queue = []; redirectToLogin(); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Abort Handling: Aborting in-flight requests was added, but catch handlers in callers often swallow errors without logging or user feedback, which may be intentional UI behavior but limits diagnostics. Referred Code * Get agent list * @param {import('$agentTypes').AgentFilter} filter * @param {boolean} checkAuth * @param {AbortSignal | null} signal * @returns {Promise<import('$commonTypes').PagedItems<import('$agentTypes').AgentModel>>} */ export async function getAgents(filter, checkAuth = false, signal = null) { let url = endpoints.agentListUrl; const response = await axios.get(url, { params: { ...filter, checkAuth : checkAuth }, paramsSerializer: { dots: true, indexes: null, }, signal: signal || undefined }); return response.data; } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Nonstructured Log: The added console.log uses unstructured logging and may divulge operational queue size without ensuring sensitive data exclusion; it might be acceptable during development but should be removed or structured and gated by environment. Referred Code /** @param {{config: import('axios').InternalAxiosRequestConfig, resolve: (value: any) => void, reject: (reason?: any) => void}} item */ enqueue(item) { this.queue.push(item); console.log('queue', this.queue.length); if (!this.isRefreshingToken) { Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Re-evaluate the global loader implementation The current method of using extensive URL regex lists in http.js to control the global loader is not maintainable. A better approach would be to let individual API calls or components decide whether to show the loader, making the system more modular. Examples: src/lib/helpers/http.js [178-306] function skipLoader(config) { /** @type {RegExp[]} */ const postRegexes = [ new RegExp('http(s*)://(.*?)/conversation/(.*?)/(.*?)', 'g'), new RegExp('http(s*)://(.*?)/agent', 'g'), new RegExp('http(s*)://(.*?)/knowledge/vector/(.*?)/page', 'g'), new RegExp('http(s*)://(.*?)/knowledge/(.*?)/search', 'g'), new RegExp('http(s*)://(.*?)/knowledge/vector/(.*?)/create', 'g'), new RegExp('http(s*)://(.*?)/knowledge/document/(.*?)/page', 'g'), new RegExp('http(s*)://(.*?)/users', 'g'), ... (clipped 119 lines) Solution Walkthrough: Before: // src/lib/helpers/http.js axios.interceptors.request.use( (config) => { if (!skipLoader(config)) { loaderStore.set(true); } // ... } ); function skipLoader(config) { const getRegexes = [ new RegExp('http(s*)://(.*?)/plugin/menu', 'g'), new RegExp('http(s*)://(.*?)/setting/(.*?)', 'g'), // ... many more regexes ]; if (config.method === 'get' && getRegexes.some(regex => regex.test(config.url || ''))) { return true; } // ... return false; } After: // src/lib/helpers/http.js axios.interceptors.request.use( (config) => { // Default to showing loader unless explicitly told not to. if (config.showLoader !== false) { loaderStore.set(true); } // ... } ); // src/lib/services/agent-service.js export async function getAgents(filter, checkAuth = false, signal = null, showLoader = true) { let url = endpoints.agentListUrl; const response = await axios.get(url, { params: { ... }, signal: signal, showLoader: showLoader // Pass config to interceptor }); return response.data; } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a brittle, hard-to-maintain regex-based approach for managing the global loader in http.js and proposes a more robust, decentralized design.	High
Possible issue	Prevent race conditions on unmount Prevent a race condition by aborting the getAgents API request in the onDestroy lifecycle hook. This ensures callbacks do not execute on an unmounted component. src/routes/page/agent/+page.svelte [110-134] onDestroy(() => { isPageMounted = false; unsubscriber?.(); + abortController?.abort(); }); function getPagedAgents() { isLoading = true; if (abortController) { abortController.abort(); } abortController = new AbortController(); const innerFilter = { ... }; getAgents(innerFilter, true, abortController?.signal).then(data => { agents = data; - }).catch(() => { - agents = { items: [], count: 0 }; + }).catch((err) => { + if (err.name !== 'CanceledError') { + agents = { items: [], count: 0 }; + } }).finally(() => { + if (abortController?.signal.aborted) return; isLoading = false; refreshPager(agents.count); }); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a race condition where a promise can resolve after component unmount, leading to errors. Aborting the request in onDestroy is the correct way to fix this potential bug.	Medium
	Ensure user data is refetched To prevent stale user data when the agentId changes, move the myInfo() call from onMount into the loadAgent function. This ensures user information is refetched along with agent data. src/routes/page/agent/[agentId]/+page.svelte [45-67] $: if (agentId) { loadAgent(agentId); } async function loadAgent(/** @type {string} */ id) { isLoading = true; - agent = await getAgent(id); + [agent, user] = await Promise.all([ + getAgent(id), + myInfo() + ]); isLoading = false; } onMount(async () => { - user = await myInfo(); - unsubscriber = globalEventStore.subscribe((/** @type {import('$commonTypes').GlobalEvent} */ event) => { if (event.name !== GlobalEvent.Search) return; const similarName = event.payload?.trim(); if (similarName) { const url = `page/agent?page=${1}&pageSize=${12}&similarName=${encodeURIComponent(similarName)}`; window.location.href = url; } }); }); Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that user data can become stale if agentId changes, as myInfo() is only called once on mount. Moving the call into loadAgent ensures data consistency and is a good practice for data loading tied to route parameters.	Medium
General	Improve navigation accessibility and UX Improve navigation accessibility and user experience by setting the href attribute on Link components to the destination URL instead of using href={null} with an on:click handler for navigation. src/routes/VerticalLayout/Sidebar.svelte [237-241] <li> - <Link class="clickable" id={getCleanUrl(childItem.link)} href={null} on:click={() => goToPage(childItem.link)}> + <Link class="clickable" id={getCleanUrl(childItem.link)} href={childItem.link} on:click={() => goToPage()}> {$_(childItem.label)} </Link> </li> Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that using href={null} with an on:click handler for navigation is an anti-pattern that harms accessibility and user experience. Restoring the href attribute is a significant improvement.	Medium
Update