Skip to content

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Aug 22, 2025

PR Type

Enhancement


Description

  • Update knowledge data structure from data to payload

  • Refactor vector item components to use new property name

  • Update type definitions for consistency


Diagram Walkthrough

flowchart LR
  A["Knowledge Items"] --> B["Vector Components"]
  B --> C["Data Structure"]
  C --> D["item.data → item.payload"]
  D --> E["Updated UI Components"]
Loading

File Walkthrough

Relevant files
Enhancement
vector-item-edit-modal.svelte
Update edit modal data property references                             

src/routes/page/knowledge-base/common/vector-table/vector-item-edit-modal.svelte

  • Replace item?.data references with item?.payload
  • Update question and answer text extraction
  • Modify inner payloads mapping logic
+5/-5     
vector-item.svelte
Refactor vector item display components                                   

src/routes/page/knowledge-base/common/vector-table/vector-item.svelte

  • Change item?.data to item?.payload in display logic
  • Update question/answer text rendering
  • Modify detail view payload iteration
+10/-10 
+page.svelte
Update documents page data assignment                                       

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

  • Update found item assignment from data to payload
+1/-1     
+page.svelte
Update Q&A page data assignment                                                   

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

  • Change found item assignment from data to payload
+1/-1     
Documentation
knowledgeTypes.js
Update knowledge type definitions                                               

src/lib/helpers/types/knowledgeTypes.js

  • Update JSDoc comment from data to payload
  • Maintain type definition consistency
+1/-1     

@iceljc iceljc merged commit 73260a3 into SciSharp:main Aug 22, 2025
1 of 2 checks passed
@qodo-merge-pro
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Type Handling

When constructing innerPayloads, foundType?.value is assigned to data_type while also spreading the existing payload object; confirm that data_type is consistently a string and not overwritten unexpectedly. Also ensure data_type?.toLowerCase() consumers can handle undefined.

innerPayloads = Object.keys(item?.payload || {}).filter(key => !excludedPayloads.includes(key)).map(key => {
    const foundType = dataTypeOptions.find(x => x.value === item?.payload[key]?.data_type);
    return {
        uuid: uuidv4(),
        key: key,
        value: {
            ...item?.payload[key] || {},
            data_type: foundType?.value
        }
Null Safety

Rendering relies on item?.payload[key]?.data_type and data_value; add safe fallbacks to avoid runtime errors and ensure UI degrades gracefully when payload keys are missing or not objects.

{#if item?.payload}
    {#each Object.keys(item.payload) as key, idx (`${key}-${item?.id}`)}
        {#if (!excludedPayloads.includes(key))}
            <li class="more-detail-item wrappable">
                <span>{key} {`(${item.payload[key]?.data_type?.toLowerCase()})`}: </span>
                {#if key === KnowledgePayloadName.FileUrl}
                    <!-- svelte-ignore a11y-click-events-have-key-events -->
                    <!-- svelte-ignore a11y-no-static-element-interactions -->
                    <span
                        class="link clickable"
                        on:click={() => window.open(item.payload[key]?.data_value)}
                    >
                        link
                    </span>
                {:else}
                    <span>{item.payload[key]?.data_value}</span>
                {/if}

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Ensure backend alignment

This PR renames the core field from data to payload across UI and types but
doesn’t show corresponding backend/API contract updates. If server responses or
persistence layers still emit data, the UI will silently break or render empty
fields. Confirm and coordinate the API schema, SDK clients, and storage
migrations so both read/write paths consistently use payload (or add
backward-compat handling).

Examples:

src/routes/page/knowledge-base/common/vector-table/vector-item.svelte [82-86]
        <div class="ellipsis">{item?.payload?.text?.data_value || item?.payload?.question?.data_value || ''}</div>
    </td>
    {#if isQuestionAnswerCollection}
        <td class="knowledge-text-qa">
            <div class="ellipsis">{item?.payload?.answer?.data_value || ''}</div>
src/routes/page/knowledge-base/documents/+page.svelte [614]
			found.payload = { ...newData };

Solution Walkthrough:

Before:

// In a Svelte component, assuming API provides `item` object
// This code expects `item.payload` and will fail silently if the API still sends `item.data`.

function displayItem(item) {
  const question = item?.payload?.question?.data_value || '';
  const answer = item?.payload?.answer?.data_value || '';
  render(question, answer);
}

function updateItem(found, newData) {
  found.payload = { ...newData };
  // items array is updated
}

After:

// Suggestion: Add a backward-compatibility layer to handle both formats

function normalizeItemFromApi(apiItem) {
  const payload = apiItem.payload || apiItem.data; // Handle both old and new field names
  return {
    ...apiItem,
    payload: payload,
  };
}

// Component code remains clean and uses the new `payload` property
function displayItem(item) {
  const question = item?.payload?.question?.data_value || '';
  const answer = item?.payload?.answer?.data_value || '';
  render(question, answer);
}
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical integration risk, as the frontend-only change to the data/payload field will break the UI if the backend API is not updated accordingly.

High
Possible issue
Add robust payload and URL guards

Guard against non-object payload values to prevent runtime errors during
Object.keys and property access. Also ensure data_value is a string before
passing to window.open to avoid unintended navigation or exceptions.

src/routes/page/knowledge-base/common/vector-table/vector-item.svelte [181-201]

-{#if item?.payload}
+{#if item?.payload && typeof item.payload === 'object' && !Array.isArray(item.payload)}
     {#each Object.keys(item.payload) as key, idx (`${key}-${item?.id}`)}
         {#if (!excludedPayloads.includes(key))}
             <li class="more-detail-item wrappable">
-                <span>{key} {`(${item.payload[key]?.data_type?.toLowerCase()})`}: </span>
+                <span>{key} {`(${item.payload[key]?.data_type?.toLowerCase?.() || ''})`}: </span>
                 {#if key === KnowledgePayloadName.FileUrl}
                     <!-- svelte-ignore a11y-click-events-have-key-events -->
                     <!-- svelte-ignore a11y-no-static-element-interactions -->
                     <span
                         class="link clickable"
-                        on:click={() => window.open(item.payload[key]?.data_value)}
+                        on:click={() => {
+                            const url = item.payload[key]?.data_value;
+                            if (typeof url === 'string' && url) window.open(url);
+                        }}
                     >
                         link
                     </span>
                 {:else}
                     <span>{item.payload[key]?.data_value}</span>
                 {/if}
             </li>
         {/if}
     {/each}
 {/if}
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that item.payload could be a non-object, causing Object.keys to fail, and improves robustness by adding type checks for item.payload and the URL for window.open.

Medium
Safely iterate payload entries

Validate that item.payload is a plain object before iterating keys to avoid
errors if it's null, an array, or a primitive. Additionally, default value to an
empty object when the payload entry is not an object to prevent spread failures.

src/routes/page/knowledge-base/common/vector-table/vector-item-edit-modal.svelte [110-120]

-innerPayloads = Object.keys(item?.payload || {}).filter(key => !excludedPayloads.includes(key)).map(key => {
-    const foundType = dataTypeOptions.find(x => x.value === item?.payload[key]?.data_type);
+innerPayloads = Object.keys(
+    (item?.payload && typeof item.payload === 'object' && !Array.isArray(item.payload)) ? item.payload : {}
+)
+.filter(key => !excludedPayloads.includes(key))
+.map(key => {
+    const entry = (item?.payload && typeof item.payload[key] === 'object' && item.payload[key] !== null) ? item.payload[key] : {};
+    const foundType = dataTypeOptions.find(x => x.value === entry?.data_type);
     return {
         uuid: uuidv4(),
-        key: key,
+        key,
         value: {
-            ...item?.payload[key] || {},
+            ...entry,
             data_type: foundType?.value
         }
     };
 });
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion improves code robustness by ensuring item.payload is a plain object before iteration, preventing potential runtime errors if it were an array or primitive, which is a valid defensive improvement.

Low
  • More

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