Skip to content

Allowing the evaluation to not call commit_tool#2041

Merged
yeshion23333 merged 2 commits into
volcengine:mainfrom
fujiajie666:feat/eval_with_no_commit_tool
May 14, 2026
Merged

Allowing the evaluation to not call commit_tool#2041
yeshion23333 merged 2 commits into
volcengine:mainfrom
fujiajie666:feat/eval_with_no_commit_tool

Conversation

@fujiajie666
Copy link
Copy Markdown
Collaborator

Description

When calling vikingbot for evaluation, the bot invokes the commit tool, which can corrupt memories. This should be prevented.

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 70
🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes

Sub-PR theme: Add disabled_tools functionality to avoid commit_tool during evaluation

Relevant files:

  • bot/vikingbot/agent/loop.py
  • bot/vikingbot/agent/tools/registry.py
  • bot/vikingbot/channels/openapi.py
  • bot/vikingbot/channels/openapi_models.py

Sub-PR theme: Add token_usage to ChatResponse

Relevant files:

  • bot/vikingbot/channels/openapi.py
  • bot/vikingbot/channels/openapi_models.py

⚡ Recommended focus areas for review

Missing OutboundMessage token_usage Attribute

The code attempts to access msg.token_usage from OutboundMessage, but the PR does not modify OutboundMessage to include this field, nor does it set token_usage when sending the RESPONSE event. This will cause an AttributeError at runtime when processing responses.

        pending.token_usage = msg.token_usage
        await pending.add_event(
            "response",
            {"content": msg.content or "", "response_id": msg.response_id},
        )
        pending.set_final(msg.content or "")
        await pending.close_stream()
    elif msg.event_type == OutboundEventType.REASONING:
        await pending.add_event("reasoning", msg.content)
    elif msg.event_type == OutboundEventType.TOOL_CALL:
        await pending.add_event("tool_call", msg.content)
    elif msg.event_type == OutboundEventType.TOOL_RESULT:
        await pending.add_event("tool_result", msg.content)
    return

# Handle as normal OpenAPIChannel message
session_id = msg.session_key.chat_id
pending = self._pending.get(session_id)

if not pending:
    # No pending request for this session, ignore
    return

if msg.event_type == OutboundEventType.RESPONSE:
    # Final response - add to stream first
    pending.set_response_id(msg.response_id)
    pending.token_usage = msg.token_usage

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@yeshion23333 yeshion23333 merged commit 8b04fd6 into volcengine:main May 14, 2026
4 of 5 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project May 14, 2026
@fujiajie666 fujiajie666 deleted the feat/eval_with_no_commit_tool branch May 18, 2026 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants