feat(mcpserver): Slack URL 取得成功時は文書検索をスキップして referenced_slack_urls を優先 - #68
Conversation
…earch When a Slack permalink resolves, skip hybrid document search and return referenced_slack_urls with thread replies first so MCP clients prioritize the referenced conversation over unrelated document hits.
|
@codex review |
VSA ValidationOK。
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de2c6dd4de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if len(slackURLMessages) > 0 { | ||
| mcpResponse := convertSlackURLDirectResponse(searchRequest, slackURLMessages) | ||
| responseJSON, marshalErr := json.MarshalIndent(mcpResponse, "", " ") | ||
| if marshalErr != nil { | ||
| errorMsg := fmt.Sprintf("Failed to serialize response: %v", marshalErr) | ||
| return CreateToolCallErrorResult(errorMsg), marshalErr | ||
| } | ||
|
|
||
| hsta.logger.Printf("Slack URL direct fetch completed successfully - found %d results", mcpResponse.Total) | ||
| sendProgress(1.0, 1.0, "Slack URL direct fetch completed") | ||
| return CreateToolCallResult(string(responseJSON)), nil |
There was a problem hiding this comment.
When the MCP server runs with --export-eval and a Slack permalink resolves, this early-return path exits before the existing hsta.evalWriter.WriteRecord block later in HandleToolCallWithProgress. The same permalink requests used to fall through to hybrid search and be written to the eval JSONL, so direct-fetch traffic now disappears from evaluation exports; write an eval record before returning here.
Useful? React with 👍 / 👎.
| if len(slackURLMessages) > 0 { | ||
| mcpResponse := convertSlackURLDirectResponse(searchRequest, slackURLMessages) |
There was a problem hiding this comment.
When a caller sets enable_slack_search (or uses an explicit Slack-search directive) and the permalink fetch succeeds, this branch returns before the later slackService.Search(...) block runs. In a request like “Slack検索を利用して、このURLに関連する会話も探して ”, the tool now returns only the referenced message and silently ignores the requested workspace Slack search; only short-circuit document search, or gate this return on Slack search not being requested.
Useful? React with 👍 / 👎.
| if len(slackURLMessages) > 0 { | ||
| mcpResponse := convertSlackURLDirectResponse(searchRequest, slackURLMessages) |
There was a problem hiding this comment.
When a query contains multiple Slack permalinks and FetchByURLs resolves at least one but returns errors for others, fetchReferencedSlackMessages drops those errors and this len(slackURLMessages) > 0 branch returns only the fetched subset with no warning or fallback to the normal URL/document search path. Users asking about several linked messages can get an apparently complete response that silently omits inaccessible or failed URLs; only short-circuit when all detected URLs were fetched or surface the partial failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b76286a0cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| f.logger.Printf("MessageFetcher: failed to fetch thread replies: %v", err) | ||
| } else { | ||
| enriched.ThreadMessages = threadMessages | ||
| return enriched, fmt.Errorf("failed to fetch thread replies: %w", err) |
There was a problem hiding this comment.
When a threaded Slack permalink's primary message is fetched but fetchThreadReplies fails, this returns a partial enriched message together with an error. In the MCP direct-fetch path, fetchReferencedSlackMessages treats any response.Errors as incomplete, so a URL-only request falls through to OpenSearch instead of returning the resolved primary message; if OpenSearch is unavailable, the user gets a search error even though the linked Slack message was fetched. Treat reply-context failures as warnings for the direct URL fetch, or otherwise count the primary URL as fetched.
Useful? React with 👍 / 👎.
| } | ||
| allSlackURLsFetched := slackURLFetchComplete && len(slackURLMessages) > 0 && len(slackURLMessages) == len(slackURLs) | ||
| workspaceSlackSearchRequested := searchRequest.EnableSlackSearch && hsta.slackService != nil | ||
| if allSlackURLsFetched && !workspaceSlackSearchRequested { |
There was a problem hiding this comment.
When mcpClient is configured, a query that contains a resolvable Slack permalink and another MCP-backed reference now returns here before the later QueryWithRetry and attachMCPResultsToHybridResponse path runs. In that scenario, with workspace Slack search not requested, the response silently omits mcp_results even though the server wires MCP enrichment into the hybrid adapter; keep the direct Slack response, but still run/attach MCP tool results before returning.
Useful? React with 👍 / 👎.
1 participant
Pull Request
Summary
When a Slack permalink in the MCP
hybrid_searchquery resolves successfully, RAGent now short-circuits document hybrid search and returns the fetched message (including thread replies) inreferenced_slack_urlsfirst. This prevents MCP clients from answering from unrelated knowledge-base hits when the user clearly referenced a Slack conversation.Type of Change
Changes Made
search_method=slack_url_direct_fetch, emptyresults, sources["slack_urls"]thread_repliesonHybridSearchSlackResult(and OpenAPI) so thread context is available to MCP clientsHybridSearchResponseJSON soreferenced_slack_urlsappears beforeresultsreferenced_slack_urlsMotivation and Context
Previously, even after a Slack permalink was fetched, the tool continued hybrid document search and returned both document hits and the referenced Slack message. LLM clients often over-weighted document
resultsand underused the explicitly referenced Slack conversation. Operators asking “この URL の内容を教えて” expect the linked message (and thread) to be the primary answer source.This change makes successful Slack URL resolution the authoritative path: skip BM25/vector search, avoid unnecessary OpenSearch/Bedrock cost, and surface thread replies so answers can include the full conversation context.
How Has This Been Tested?
go test ./internal/mcpserver/)Test Configuration
Commands and Results
Impact Analysis
Components Affected
cmd/)internal/vectorizer/)internal/opensearch/)internal/s3vector/)internal/slackbot/)internal/embedding/)internal/config/)internal/mcpserver/) — hybrid_search tool behavior and response schemaopenapi.yaml) — Slack result schemaAWS Resources Impact
Breaking Changes
Migration Guide
Behavioral change for MCP
hybrid_searchwhen a Slack permalink resolves:search_method: "slack_url_direct_fetch",results: [], and populatedreferenced_slack_urls.HybridSearchSlackResultmay include optionalthread_replies.Clients that assumed hybrid document results always accompany URL fetch should read
referenced_slack_urlsfirst (tool description now states this). Failed URL fetch still falls through to normal hybrid search.Dependencies
Documentation
openapi.yaml)Checklist
go fmt/gofmtandgo vet)go mod tidyto clean up dependencies (no module changes)Performance Considerations
Successful Slack URL resolution skips OpenSearch health check, embedding, BM25, and vector search for that request.
File-by-File Changes
internal/mcpserver/hybrid_search_tool.goconvertSlackURLDirectResponse, thread reply mapping, tool description updateinternal/mcpserver/types.goreferenced_slack_urlsbeforeresults; addThreadRepliesonHybridSearchSlackResultinternal/mcpserver/command.goreferenced_slack_urlsinternal/mcpserver/hybrid_search_tool_slack_url_test.goopenapi.yamlthread_replieson Slack result schemaPULL_REQUEST.mdAdditional Notes
PULL_REQUEST.mdis committed only to satisfy PR workflow requirements; it may be ignored after merge.Screenshots/Logs
N/A
プルリクエスト（日本語版）
概要
MCP
hybrid_searchでクエリ内の Slack permalink が取得できた場合、文書ハイブリッド検索を行わずreferenced_slack_urls（スレッド返信含む）を最優先で返すようにした。ユーザーが明示的に参照した会話を、無関係なドキュメントヒットより優先させる。変更の種類
実装された変更
search_method=slack_url_direct_fetch）thread_repliesを hybrid_search の Slack 結果に追加referenced_slack_urlsをresultsより前に配置動機と背景
Slack URL を取得できても従来は文書検索も並行して走り、LLM がドキュメント結果を優先してしまうことがあった。「この URL の内容を教えて」系の問い合わせでは、参照メッセージとスレッドを一次情報にすべき。関連 Issue なし。
変更される挙動
referenced_slack_urlsのみ返却thread_repliesに返信を含めるテスト方法
go test -v -count=1 ./internal/mcpserver/ -timeout 120s→ PASS）gofmt/go vet ./internal/mcpserver/→ PASSテスト設定
影響分析
影響を受けるコンポーネント
internal/mcpserver/）openapi.yaml）AWSリソースへの影響
破壊的変更
Slack permalink 取得成功時は文書検索結果が空になる。
referenced_slack_urlsを読むクライアント実装が必要（ツール説明でも指示済み）。取得失敗時は従来パスにフォールバック。依存関係
ドキュメント
openapi.yaml）チェックリスト
gofmtとgo vet）パフォーマンスに関する考慮事項
懸念
追加ノート