InstructCompletion add file_urls state

[yileicn]

PR Type

Enhancement

---

Description

• Add ToString() method to InstructFileModel for better string representation

• Introduce file_urls state in conversation for file tracking

• Optimize file_count state assignment using null-coalescing operator

• Improve code readability with consistent formatting

---

Diagram Walkthrough

flowchart LR
  A["InstructFileModel"] -->|"adds ToString()"| B["String Representation"]
  C["InstructCompletion"] -->|"sets file_urls state"| D["Conversation State"]
  C -->|"optimizes file_count"| D
  B -->|"used by"| D

Loading

File Walkthrough

Relevant files

Enhancement

InstructFileModel.cs Add ToString method for file representation                            src/Infrastructure/BotSharp.Abstraction/Files/Models/InstructFileModel.cs Add ToString() override method that returns FileUrl if available Falls back to truncated FileData (max 20 chars) if FileUrl is empty Returns empty string if both properties are null or empty +13/-0    InstructModeController.cs Add file_urls state and optimize file_count                            src/Infrastructure/BotSharp.OpenAPI/Controllers/Instruct/InstructModeController.cs Add new file_urls state using input.Files?.Select(p => p.ToString()) Optimize file_count state to use null-coalescing operator instead of ternary Improve code formatting with consistent spacing +3/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Sensitive data exposure Description: The new file_urls conversation state stores input.Files?.Select(p => p.ToString()), and since InstructFileModel.ToString() falls back to FileData.SubstringMax(20), this can unintentionally persist and later expose snippets of sensitive file contents (e.g., secrets/PII) through downstream logging, telemetry, prompts, or any feature that reads conversation state. InstructModeController.cs [37-38] Referred Code .SetState("file_count", input.Files?.Count, source: StateSource.External) .SetState("file_urls", input.Files?.Select(p => p.ToString()), source: StateSource.External);
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: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed 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: Passed 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: Null element crash: The new input.Files?.Select(p => p.ToString()) will throw a NullReferenceException if input.Files contains any null elements because p.ToString() is invoked without a null check. Referred Code .SetState("file_urls", input.Files?.Select(p => p.ToString()), source: StateSource.External); 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 logging: The new conversation state writes for file_count and file_urls add potentially security-relevant tracking data without any visible audit log entries containing actor, timestamp, action, and outcome. Referred Code .SetState("file_count", input.Files?.Count, source: StateSource.External) .SetState("file_urls", input.Files?.Select(p => p.ToString()), source: StateSource.External); 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: Unvalidated file metadata: The PR persists external input-derived file_urls (via ToString() which may fall back to FileData) into conversation state without visible validation/sanitization to ensure it does not store unsafe or sensitive content. Referred Code .SetState("file_count", input.Files?.Count, source: StateSource.External) .SetState("file_urls", input.Files?.Select(p => p.ToString()), source: StateSource.External); Learn more about managing compliance generic rules or creating your own custom rules

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	State key is misleading The state key file_urls is inaccurate as it can hold truncated file data, not just URLs. It should be renamed to file_references to avoid confusion for consumers of the state. Examples: src/Infrastructure/BotSharp.OpenAPI/Controllers/Instruct/InstructModeController.cs [38] .SetState("file_urls", input.Files?.Select(p => p.ToString()), source: StateSource.External); src/Infrastructure/BotSharp.Abstraction/Files/Models/InstructFileModel.cs [26-37] public override string ToString() { if (!string.IsNullOrEmpty(FileUrl)) { return FileUrl; } else if (!string.IsNullOrEmpty(FileData)) { return FileData.SubstringMax(20); } ... (clipped 2 lines) Solution Walkthrough: Before: // InstructFileModel.cs public override string ToString() { if (!string.IsNullOrEmpty(FileUrl)) { return FileUrl; } else if (!string.IsNullOrEmpty(FileData)) { return FileData.SubstringMax(20); } return string.Empty; } // InstructModeController.cs state.SetState("file_urls", input.Files?.Select(p => p.ToString()), ...); After: // InstructFileModel.cs public override string ToString() { if (!string.IsNullOrEmpty(FileUrl)) { return FileUrl; } else if (!string.IsNullOrEmpty(FileData)) { return FileData.SubstringMax(20); } return string.Empty; } // InstructModeController.cs state.SetState("file_references", input.Files?.Select(p => p.ToString()), ...); Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the state key file_urls is misleading because its value can contain truncated file data, not just URLs, which could cause downstream issues.	Medium
Possible issue	Add null check for files Add a Where(f => f != null) clause before Select to filter out potential null elements in the input.Files collection, preventing a NullReferenceException. src/Infrastructure/BotSharp.OpenAPI/Controllers/Instruct/InstructModeController.cs [38] -.SetState("file_urls", input.Files?.Select(p => p.ToString()), source: StateSource.External); +.SetState("file_urls", input.Files?.Where(f => f != null).Select(f => f.ToString()), source: StateSource.External); Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential NullReferenceException if the input.Files list contains null elements and provides a simple, effective fix to make the code more robust against malformed input.	Medium
More