Skip to content

Features/refine chat window #373

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.

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

iceljc
Copy link
Collaborator

@iceljc iceljc commented Aug 13, 2025

PR Type

Enhancement


Description

  • Add function visibility mode configuration for agents

  • Rename routing mode change handler for clarity

  • Update agent type definitions with new property


Diagram Walkthrough

flowchart LR
  A["Agent Overview"] --> B["Function Visibility Mode"]
  B --> C["Manual/Auto Selection"]
  A --> D["Routing Mode Handler"]
  D --> E["Renamed for Clarity"]
Loading

File Walkthrough

Relevant files
Enhancement
agent-overview.svelte
Add function visibility mode UI controls                                 

src/routes/page/agent/[agentId]/agent-components/agent-overview.svelte

  • Import FunctionVisMode enum and Select component
  • Add function visibility mode dropdown with manual/auto options
  • Rename changeMode to changeRoutingMode for clarity
  • Add changeFunctionVisibilityMode handler function
+37/-4   
enums.js
Define function visibility mode enum                                         

src/lib/helpers/enums.js

  • Add new FunctionVisMode enum with Manual and Auto values
  • Export frozen enum object for function visibility modes
+6/-0     
agentTypes.js
Update agent type with visibility mode                                     

src/lib/helpers/types/agentTypes.js

  • Add function_visibility_mode property to AgentModel typedef
  • Property is optional string type for agent configuration
+1/-0     

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 Safety

The handler changeFunctionVisibilityMode accesses e.detail.selecteds with a ts-ignore. Verify the Select component’s event shape or add a proper type guard to avoid runtime errors when the event payload changes or is undefined.

    /**
	 * @param {any} e
	 */
    function changeFunctionVisibilityMode(e) {
        // @ts-ignore
        const values = e?.detail?.selecteds?.map(x => x.value) || [];
        agent.function_visibility_mode = values[0] || null;
        handleAgentChange();
    }
UX Consistency

The new function visibility control uses the custom Select component (with label/value pairs) while routing mode uses a native <Input type="select"> (with id/name). Consider aligning component usage and option shapes to keep UI/behavior consistent and reduce conversion bugs.

<th class="agent-prop-key" style="vertical-align: middle">
    <div class="mt-1">
        Function visibility
    </div>
</th>
<td>							
    <div class="mt-2 mb-2">
        <Select
            tag={'function-visibility-mode-select'}
            containerStyles={'width: 50%; min-width: 100px;'}
            placeholder={'Select'}
            selectedValues={agent.function_visibility_mode ? [agent.function_visibility_mode] : []}
            options={functionVisibilityModeOptions}
            on:select={e => changeFunctionVisibilityMode(e)}
        />
    </div>
</td>

Copy link

qodo-merge-pro bot commented Aug 13, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Persist and validate new mode

Introducing function_visibility_mode without backend contract or persistence
changes risks silent no-ops and inconsistent behavior across clients. Ensure the
API, storage schema, and server-side validation/enum mapping support this new
field and values (manual/auto), and that existing agents receive a safe
default/migration to avoid undefined states.

Examples:

src/routes/page/agent/[agentId]/agent-components/agent-overview.svelte [89-94]
function changeFunctionVisibilityMode(e) {
    // @ts-ignore
    const values = e?.detail?.selecteds?.map(x => x.value) || [];
    agent.function_visibility_mode = values[0] || null;
    handleAgentChange();
}
src/lib/helpers/types/agentTypes.js [43-50]
/**
 * @typedef {Object} AgentModel
 * @property {string} id - Agent Id.
 * @property {string} name - Agent name.
 * @property {string} description - Agent description.
 * @property {string} type - Agent type
 * @property {string?} mode - Agent routing mode
 * @property {string?} function_visibility_mode - Agent function visibility mode

Solution Walkthrough:

Before:

// agent-overview.svelte
function changeFunctionVisibilityMode(e) {
    // ...
    agent.function_visibility_mode = values[0] || null;
    handleAgentChange(); // This likely calls an API to save the agent
}

// Backend (missing from PR)
// No API logic to handle `function_visibility_mode`.
// No database schema update for `function_visibility_mode`.

After:

// Backend API Controller (conceptual)
function updateAgent(agentData) {
    // Validate agentData.function_visibility_mode against allowed values
    if (!['manual', 'auto'].includes(agentData.function_visibility_mode)) {
        throw new Error("Invalid function visibility mode");
    }
    // Persist the new field to the database
    db.agents.update({ id: agentData.id }, {
        ...otherFields,
        function_visibility_mode: agentData.function_visibility_mode
    });
}
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical flaw: the PR adds the function_visibility_mode property only on the frontend, without the necessary backend changes for persistence, making the feature non-functional.

High
Possible issue
Add guards and change checks

Guard against missing agent before assignment to prevent runtime errors during
initial render or unmounted states. Also, update the select handler to no-op if
no change occurred to avoid unnecessary updates.

src/routes/page/agent/[agentId]/agent-components/agent-overview.svelte [80-94]

 function changeRoutingMode(e) {
-    const value = e.target.value || null;
+    const value = e.target?.value ?? null;
+    if (!agent || agent.mode === value) return;
     agent.mode = value;
     handleAgentChange();
 }
 
 /**
  * @param {any} e
  */
 function changeFunctionVisibilityMode(e) {
     // @ts-ignore
     const values = e?.detail?.selecteds?.map(x => x.value) || [];
-    agent.function_visibility_mode = values[0] || null;
+    const next = values[0] ?? null;
+    if (!agent || agent.function_visibility_mode === next) return;
+    agent.function_visibility_mode = next;
     handleAgentChange();
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that adding checks for the existence of agent and for value changes improves code robustness and prevents unnecessary updates, which is a good practice.

Medium
General
Enforce single-select behavior

Ensure the Select component operates in single-select mode to match the state
shape (string|null). Explicitly set single-select props or disable
multi-selection to avoid multiple values and inconsistent assignments.

src/routes/page/agent/[agentId]/agent-components/agent-overview.svelte [327-334]

 <Select
     tag={'function-visibility-mode-select'}
     containerStyles={'width: 50%; min-width: 100px;'}
     placeholder={'Select'}
+    multiple={false}
     selectedValues={agent.function_visibility_mode ? [agent.function_visibility_mode] : []}
     options={functionVisibilityModeOptions}
     on:select={e => changeFunctionVisibilityMode(e)}
 />
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that explicitly setting multiple={false} on the Select component makes the intent clear and prevents potential bugs, aligning with the single-value logic in changeFunctionVisibilityMode.

Low
Use enum keys as labels

Use enum keys for labels while keeping values as the canonical enum values to
improve UX and avoid locale issues. This prevents ambiguous labels if values
change and keeps display consistent.

src/routes/page/agent/[agentId]/agent-components/agent-overview.svelte [31-33]

 const functionVisibilityModeOptions = Object.entries(FunctionVisMode).map(([k, v]) => (
-	{ label: v, value: v }
+	{ label: k, value: v }
 ));
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly proposes using the enum keys (Manual, Auto) for display labels instead of the values (manual, auto), which improves user experience by showing more readable text.

Low
  • Update

@iceljc iceljc marked this pull request as draft August 15, 2025 03:15
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