Skip to content

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Aug 12, 2025

PR Type

Enhancement


Description

  • Add sorting functionality to knowledge base search

  • Implement sort field input and direction controls

  • Update search parameters to include sort options

  • Refactor advanced search UI layout


Diagram Walkthrough

flowchart LR
  A["Advanced Search Component"] --> B["Sort Field Input"]
  A --> C["Sort Direction Radio Buttons"]
  B --> D["Search Parameters"]
  C --> D
  D --> E["Knowledge API Call"]
  E --> F["Sorted Results"]
Loading

File Walkthrough

Relevant files
Enhancement
_knowledgebase.scss
Add responsive styling for search operators                           

src/lib/scss/custom/pages/_knowledgebase.scss

  • Add .operator-container styling with responsive layout
  • Include mobile-friendly flex direction changes
  • Add .operator-item and .operator-title styling
+18/-0   
advanced-search.svelte
Implement sort controls in advanced search                             

src/routes/page/knowledge-base/common/search/advanced-search.svelte

  • Add sortOrder and sortField export properties
  • Create sortDirections array with ASC/DESC options
  • Restructure operator UI with new container layout
  • Add sort field input and direction radio buttons
+93/-32 
+page.svelte
Integrate sorting into document knowledge page                     

src/routes/page/knowledge-base/documents/+page.svelte

  • Add sort-related variables (sortField, sortOrder, innerSort)
  • Update search functions to include sort parameters
  • Implement buildSearchSort function
  • Bind sort properties to AdvancedSearch component
+54/-20 
+page.svelte
Integrate sorting into Q&A knowledge page                               

src/routes/page/knowledge-base/question-answer/+page.svelte

  • Add sort-related variables and state management
  • Update API calls to include sort parameters
  • Implement buildSearchSort function
  • Bind sort properties to AdvancedSearch component
+54/-17 
knowledgeTypes.js
Add TypeScript definitions for sorting                                     

src/lib/helpers/types/knowledgeTypes.js

  • Add VectorSort typedef with field and order properties
  • Update KnowledgeFilter to include optional order_by parameter
+7/-0     

@qodo-merge-pro
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Unused Handler

The radio inputs for sort direction attach an empty on:change handler which is unnecessary and may confuse future readers; consider removing it or implementing needed logic.

    id={op.id}
    name="searchSort"
    value={op.value}
    disabled={disabled}
    bind:group={sortOrder}
    on:change={e => {}}
/>
Sort When Disabled

Sorting is applied even when advanced search is off if a user previously set sort; ensure sort resets appropriately or is gated by the same toggle in all code paths (init, reset, search, loadMore).

		if (e.key === 'Enter') {
			e.preventDefault();
		}

		search();
	}

	/** @param {boolean} skipLoader */
	function reset(skipLoader = false) {
		resetStates();
		getData({
			...defaultParams,
			startId: null,
			isReset: true,
			skipLoader: skipLoader,
			filterGroups: innerSearchGroups,
			sort: innerSort
		});
	}

	function initPage() {
		resetStates();
    	initData();
	}

	function resetEditData() {
		editCollection = '';
		editItem = null;
	}

	function resetStates() {
		text = "";
		nextId = null;
		isSearching = false;
		searchDone = false;
		isFromSearch = false;
		textSearch = false;
		selectedOperator = 'or';
		innerSearchGroups = [];
		sortField = '';
		sortOrder = 'desc';
		innerSort = {
			field: sortField,
			order: sortOrder
		};
	}
Null Field Sort

buildSearchSort returns { field: '', order: 'desc' } when adv search is on but field is empty; although you guard with !!params.sort?.field, also prevent creating empty-field sorts earlier to avoid sending inconsistent state elsewhere.

/**
 *  @param {string} field
 *  @param {string} order
 */
function buildSearchSort(field, order) {
	return isAdvSearchOn ? {
		field: field,
		order: order
	} : null;
}

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Validate and normalize sort inputs

Normalize and validate inputs in buildSearchSort to return null when field is
empty and to constrain order to 'asc' or 'desc'. This prevents emitting invalid
sort objects and reduces duplicated checks elsewhere.

src/routes/page/knowledge-base/documents/+page.svelte [747-752]

 function buildSearchSort(field, order) {
-		return isAdvSearchOn ? {
-			field: field,
-			order: order
-		} : null;
+		if (!isAdvSearchOn) return null;
+		const f = (typeof field === 'string' ? field.trim() : '');
+		if (!f) return null;
+		const ord = (order || '').toLowerCase() === 'asc' ? 'asc' : 'desc';
+		return { field: f, order: ord };
 	}
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion improves the buildSearchSort function by centralizing validation and normalization logic, making the code more robust and preventing invalid sort objects from being created.

Medium
Possible issue
Validate sort field before sending

Guard against empty sort.field values so you don't send { field: '', order:
'desc' } to the backend. This prevents API errors or unintended default sorting.
Only attach order_by when field is a non-empty string.

src/routes/page/knowledge-base/documents/+page.svelte [334-348]

 function getKnowledgeListData(params = {
 		startId: null,
 		isReset: false,
 		filterGroups: [],
 		sort: null
 	}) {
 		return new Promise((resolve, reject) => {
+			const hasValidSortField = typeof params.sort?.field === 'string' && params.sort.field.trim().length > 0;
 			const filter = {
 				size: pageSize,
 				start_id: params.startId,
 				with_vector: enableVector,
 				fields: [],
 				filter_groups: params.filterGroups,
-				order_by: !!params.sort?.field ? params.sort : null
+				order_by: hasValidSortField ? { field: params.sort.field.trim(), order: params.sort.order } : null
 			};
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that a whitespace-only sort.field would be sent to the backend, and the proposed change with trim() fixes this, improving robustness.

Low
Enforce valid sort before API call

Apply the same strict validation on params.sort.field here to avoid passing an
empty field to the API. This keeps pagination and ordering consistent and
prevents backend errors.

src/routes/page/knowledge-base/question-answer/+page.svelte [325-339]

 function getKnowledgeListData(params = {
 		startId: null,
 		isReset: false,
 		filterGroups: [],
 		sort: null
 	}) {
 		return new Promise((resolve, reject) => {
+			const hasValidSortField = typeof params.sort?.field === 'string' && params.sort.field.trim().length > 0;
 			const filter = {
 				size: pageSize,
 				start_id: params.startId,
 				with_vector: enableVector,
 				fields: [],
 				filter_groups: params.filterGroups,
-				order_by: !!params.sort?.field ? params.sort : null
+				order_by: hasValidSortField ? { field: params.sort.field.trim(), order: params.sort.order } : null
 			};
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: This suggestion correctly applies the same robust validation as in the other file, using trim() to prevent sending whitespace-only sort fields to the API.

Low
  • More

@iceljc iceljc merged commit 3a7b02e into SciSharp:main Aug 12, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant