Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

add file_count for InstructCompletion #1203

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

Merged
iceljc merged 2 commits into SciSharp:master from yileicn:master
Oct 29, 2025
Merged

Conversation

@yileicn
Copy link
Collaborator

@yileicn yileicn commented Oct 24, 2025 •
edited by qodo-merge-pro bot
Loading

PR Type

Enhancement

Description

  • Add file_count state tracking for InstructCompletion requests

  • Set file_count state when Files collection is not empty

Diagram Walkthrough

flowchart LR
 input["InstructMessageModel input"] -- "check Files collection" --> check{"Files not empty?"}
 check -- "yes" --> setState["SetState file_count"]
 check -- "no" --> skip["Skip state setting"]
 setState --> state["IConversationStateService"]
 skip --> state
Loading

File Walkthrough
Relevant files
Enhancement
InstructModeController.cs
Add file_count state tracking logic

src/Infrastructure/BotSharp.OpenAPI/Controllers/InstructModeController.cs

  • Added conditional check for non-empty Files collection in input
  • Set file_count state with the count of files when available
  • Inserted blank line for code readability
+2/-0

Copy link

qodo-merge-pro bot commented Oct 24, 2025 •
edited
Loading

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪
🎫 No ticket provided 
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->
</details></td></tr>
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
⚪
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label
Copy link

qodo-merge-pro bot commented Oct 24, 2025 •
edited
Loading

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion Impact
Possible issue
✅ (削除) Add missing state source parameter (削除ここまで)
Suggestion Impact:The commit updated the SetState call for "file_count" to include source: StateSource.External exactly as suggested.

code diff:

- if (!input.Files.IsNullOrEmpty()) state.SetState("file_count", input.Files.Count);
+ if (!input.Files.IsNullOrEmpty()) state.SetState("file_count", input.Files.Count, source: StateSource.External);

Add the missing source: StateSource.External parameter to the state.SetState
call to maintain consistency with other state modifications in the method.

src/Infrastructure/BotSharp.OpenAPI/Controllers/InstructModeController.cs [39] 

-if (!input.Files.IsNullOrEmpty()) state.SetState("file_count", input.Files.Count);
+if (!input.Files.IsNullOrEmpty()) state.SetState("file_count", input.Files.Count, source: StateSource.External);

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out an inconsistency with surrounding code, where all other SetState calls include the source parameter. Applying this change improves code consistency and robustness.

Low
  • Update

@iceljc iceljc merged commit fda443e into SciSharp:master Oct 29, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Oceania2018 Oceania2018 Oceania2018 approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /