-
Notifications
You must be signed in to change notification settings - Fork 53
✨ adding v3 version of hints gen #826
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
✨ adding v3 version of hints gen #826
Conversation
Signed-off-by: Savitha Raghunathan <[email protected]>
WalkthroughA new asynchronous function, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant LanguageModel
Client->>Server: Accept file (triggers accept_file)
Server->>Server: Query accepted solutions for client
alt Solutions found
loop For each solution
Server->>Server: Build prompt with AST diffs and guidelines
Server->>LanguageModel: Send prompt asynchronously
LanguageModel-->>Server: Return structured hint
Server->>Server: Store hint linked to solution and violations
end
Server->>Server: Commit session
else No solutions found
Server->>Server: Log and exit
end
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: Savitha Raghunathan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
kai_mcp_solution_server/src/kai_mcp_solution_server/server.py (2)
557-573: Improve prompt structure and validation.The structured prompt format is a good improvement, but consider adding validation to ensure the LLM response follows the expected format.
Add response validation:
+def validate_hint_response(response_content: str) -> bool: + """Validate that the response follows the expected format.""" + content = str(response_content).strip() + return ( + "SUMMARY:" in content + and "HINT:" in content + and content.count("---") >= 2 + ) async def generate_hint_v3( kai_ctx: KaiSolutionServerContext, client_id: str, ) -> None: # ... existing code ... response = await kai_ctx.model.ainvoke(prompt) + + if not validate_hint_response(response.content): + print( + f"Warning: Generated hint for client {client_id} does not follow expected format", + file=sys.stderr, + )
536-554: Consider removing or deprecating older hint generation versions.With the introduction of v3, consider whether v1 and v2 are still needed. If not, they should be deprecated or removed to reduce code maintenance burden.
Add deprecation warnings or remove unused versions:
+import warnings + async def generate_hint_v1( kai_ctx: KaiSolutionServerContext, client_id: str, ) -> None: + warnings.warn( + "generate_hint_v1 is deprecated. Use generate_hint_v3 instead.", + DeprecationWarning, + stacklevel=2 + ) # ... existing code ... async def generate_hint_v2( kai_ctx: KaiSolutionServerContext, client_id: str, ) -> None: + warnings.warn( + "generate_hint_v2 is deprecated. Use generate_hint_v3 instead.", + DeprecationWarning, + stacklevel=2 + ) # ... existing code ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
kai_mcp_solution_server/src/kai_mcp_solution_server/server.py(2 hunks)
🔇 Additional comments (1)
kai_mcp_solution_server/src/kai_mcp_solution_server/server.py (1)
868-868: LGTM! Function call correctly updated to use v3.The function call update is appropriate and consistent with the introduction of the new hint generation version.
fabianvf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me
|
LGTM |
Summary by CodeRabbit