Skip to content

Revert: Implement literature cleaning and exporting utilities (#177) #178

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.

Sign up for GitHub

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
xinquiry merged 1 commit into from
Jan 18, 2026
Merged

Revert: Implement literature cleaning and exporting utilities (#177) #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 13 additions & 53 deletions service/app/agents/mcp_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,60 +164,20 @@ async def execute_tool_call(
f"Checking tool '{tool_name}' for injection. Schema props: {list(input_schema.get('properties', {}).keys())}"
)

schema_props = input_schema.get("properties", {})

# Inject knowledge_set_id when tool schema supports it.
if "knowledge_set_id" in schema_props:
logger.info(f"Tool '{tool_name}' supports knowledge_set_id injection.")
effective_ks_id = None
# Prefer session-level override if available.
if session_id:
try:
from app.repos.session import SessionRepository

session_repo = SessionRepository(db)
session_obj = await session_repo.get_session_by_id(session_id)
if session_obj and getattr(session_obj, "knowledge_set_id", None):
effective_ks_id = session_obj.knowledge_set_id
logger.info(f"Using session knowledge_set_id override: {effective_ks_id}")
except Exception as e:
logger.warning(f"Failed to load session knowledge_set_id override: {e}")

if effective_ks_id is None and agent:
effective_ks_id = getattr(agent, "knowledge_set_id", None)
logger.info(f"Agent Knowledge Set ID: {effective_ks_id}")

if effective_ks_id:
args_dict["knowledge_set_id"] = str(effective_ks_id)
logger.info(f"Injected knowledge_set_id: {effective_ks_id} into args")
if "knowledge_set_id" in input_schema.get("properties", {}):
logger.info(f"Tool '{tool_name}' requires knowledge_set_id.")
if agent:
ks_id = getattr(agent, "knowledge_set_id", None)
logger.info(f"Agent found. Knowledge Set ID: {ks_id}")
if ks_id:
args_dict["knowledge_set_id"] = str(ks_id)
logger.info(f"Injected knowledge_set_id: {ks_id} into args")
else:
logger.warning(f"Agent {agent.id} has NO knowledge_set_id bound!")
Comment on lines +167 to +176
Copy link
Contributor

@sourcery-ai sourcery-ai bot Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): 请评估移除会话级 knowledge_set_id 覆盖逻辑，对所有现有调用方是否都是可接受的。

之前的行为是优先使用会话级的 knowledge_set_id（通过 SessionRepository），仅在缺失时才回退到 agent。现在的新行为完全依赖 agent。任何使用每会话覆盖（例如在共享 agent 下按对话切换知识库）的流程，都会失去这种行为，可能会查询到错误的知识库，或者导致依赖特定 ID 的工具调用失败。如果这项变更是有意的，请验证所有依赖会话级 knowledge_set_id 的调用方，或考虑保留 session 级的回退逻辑。

Original comment in English

issue (bug_risk): Consider whether removing the session-level knowledge_set_id override is acceptable for all existing callers.

The previous behavior preferred a session-level knowledge_set_id (via SessionRepository) and only fell back to the agent. The new behavior relies solely on the agent. Any flows that used per-session overrides (e.g., per-conversation knowledge set switching with a shared agent) would now lose that behavior and might query the wrong knowledge set or break tool calls expecting a specific ID. If this change is intentional, please verify all callers that rely on session-level knowledge_set_id, or consider retaining the session fallback.

Comment on lines +168 to +176
Copy link
Contributor

@sourcery-ai sourcery-ai bot Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: 如果 knowledge_set_id 在设计上是可选或允许缺失的，那么新的日志文案和日志级别可能会产生误导或噪音。

由于在很多 schema 中，knowledge_set_id 可能是可选字段，把该字段描述为工具“requires”的是不准确的，而且在其缺失时发出警告（包括没有 agent 上下文的情况）会让合法流程也产生日志噪音。建议弱化措辞（例如改为 “supports knowledge_set_id”），并且将日志级别降低到 info/debug，或者仅在该字段确实在 schema 的 required 列表中时才使用 warning。

Original comment in English

suggestion: The new log wording and levels may be misleading/noisy if knowledge_set_id is optional or missing by design.

Since knowledge_set_id may be optional in many schemas, saying the tool "requires" it is inaccurate, and warning when it’s absent (including when there’s no agent context) will be noisy for valid flows. Consider softening the wording (e.g., "supports knowledge_set_id") and/or lowering the log level to info/debug, or only using a warning when the field is actually required per the schema’s required list.

else:
logger.warning("No knowledge_set_id available for injection")

# Inject user_id when tool schema supports it.
if "user_id" in schema_props:
logger.info(f"Tool '{tool_name}' supports user_id injection.")
effective_user_id = None
if session_id:
try:
from app.repos.session import SessionRepository

session_repo = SessionRepository(db)
session_obj = await session_repo.get_session_by_id(session_id)
if session_obj and getattr(session_obj, "user_id", None):
effective_user_id = session_obj.user_id
logger.info(f"Using session user_id: {effective_user_id}")
except Exception as e:
logger.warning(f"Failed to load session user_id: {e}")

if effective_user_id is None and agent:
effective_user_id = getattr(agent, "user_id", None)
logger.info(f"Agent user_id: {effective_user_id}")

if effective_user_id:
args_dict["user_id"] = str(effective_user_id)
logger.info("Injected user_id into args")
else:
logger.warning("No user_id available for injection")
logger.warning("No agent context available for injection!")
else:
logger.info(f"Tool '{tool_name}' does NOT require knowledge_set_id.")

try:
result = await call_mcp_tool(refreshed_server, tool_name, args_dict)
Expand Down
Loading
Loading