-
Notifications
You must be signed in to change notification settings - Fork 81
chore(mcp): enable model capture and conversation correlation by default - #944
Open
lucasheriques wants to merge 2 commits into
Open
chore(mcp): enable model capture and conversation correlation by default #944lucasheriques wants to merge 2 commits into
lucasheriques wants to merge 2 commits into
Conversation
Enable both existing instrumentation features and model capture on PostHogMCP, with explicit opt-outs. Missing-capability reporting and feedback stay disabled. Resolve model argument ownership from bounded raw catalog lookups on fresh low-level instances. Do not emit synthetic tools/list events, alter application model arguments, or block dispatch when catalog resolution fails. Validation: 486 MCP v1 tests passed (1 skipped), 433 MCP v2 tests passed (21 skipped); Ruff lint/format, mypy baseline for 237 files, public API snapshot, warning-as-error import and wheel build passed. CodeScene gate passed. One feedback cursor deprecation warning remains outside the requested scope. Include a Sampo minor changeset and document changed defaults and opt-outs.
Contributor
Prompt To Fix All With AI
### Issue 1 posthog/mcp/_tool_schema.py:23-25 **Ownership lookup is not cached** The fallback returns the resolved ownership without storing it. On a cold low-level server, each direct tool call therefore invokes the original `tools/list` handler again, potentially traversing 16 pages, repeating handler side effects, and adding up to 250 ms of latency. Cache the result in `data.tool_model_parameter_injected` before returning it. ```suggestion owns_model = await asyncio.wait_for( _find_model_ownership(name, list_page), timeout=0.25 ) data.tool_model_parameter_injected[name] = owns_model return owns_model ``` --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Reviews (1): Last reviewed commit: "chore(mcp): enable model capture and con..." | Re-trigger Greptile
Comment on lines
+23
to
+25
return await asyncio.wait_for(
_find_model_ownership(name, list_page), timeout=0.25
)
Contributor
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.
P2 Ownership lookup is not cached
The fallback returns the resolved ownership without storing it. On a cold low-level server, each direct tool call therefore invokes the original tools/list handler again, potentially traversing 16 pages, repeating handler side effects, and adding up to 250 ms of latency. Cache the result in data.tool_model_parameter_injected before returning it.
Suggested change
return await asyncio.wait_for(
_find_model_ownership(name, list_page), timeout=0.25
)
owns_model = await asyncio.wait_for(
_find_model_ownership(name, list_page), timeout=0.25
)
data.tool_model_parameter_injected[name] = owns_model
return owns_model
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review. Path: posthog/mcp/_tool_schema.py Line: 23-25 Comment: **Ownership lookup is not cached** The fallback returns the resolved ownership without storing it. On a cold low-level server, each direct tool call therefore invokes the original `tools/list` handler again, potentially traversing 16 pages, repeating handler side effects, and adding up to 250 ms of latency. Cache the result in `data.tool_model_parameter_injected` before returning it. ```suggestion owns_model = await asyncio.wait_for( _find_model_ownership(name, list_page), timeout=0.25 ) data.tool_model_parameter_injected[name] = owns_model return owns_model ``` **Knowledge Base Used:** - [MCP analytics instrumentation](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-python/-/docs/mcp-analytics-instrumentation.md) - [MCP framework instrumentation](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-python/-/docs/mcp-framework-instrumentation.md) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
posthog-python Compliance ReportDate: 2026年09月11日 19:01:20 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Remove the duplicate custom-dispatcher default and opt-out test. The existing parameterized prepare-and-capture test already covers both. Keep the fresh-instance regressions and bounded catalog lookup coverage. Validation: MCP v1: 485 passed, 1 skipped. MCP v2: 432 passed, 21 skipped. Ruff lint/format and CodeScene pre-commit checks passed. The existing feedback nextCursor deprecation warning remains outside this change. Runtime behavior is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
Motivation and changes
Model capture and conversation correlation currently require separate opt-ins. This enables both existing features by default, preserves explicit opt-outs, and leaves missing-capability reporting and feedback collection disabled for Gesh's work.
Fresh low-level server instances previously advertised analytics arguments but lacked the schema ownership information needed to capture them. A bounded lookup of the original tool catalog now resolves ownership before dispatch, follows up to 16 pages, and stops waiting after 250 ms. Lookup failures leave the tool's arguments intact. The lookup does not inject virtual tools or emit a synthetic
tools/listevent. Catalog handlers can therefore be invoked during a cold tool call; high-level registries and existing listing metadata avoid this fallback.Conversation IDs add a handle to eligible tool responses. Clients must echo it to correlate later calls; callers that want existing transport session grouping and unchanged result content can explicitly disable conversation correlation. Model metadata remains unverified analytics attribution.
capture_modelis also enabled onPostHogMCP; custom dispatchers continue supplying their own session IDs. Both MCP major versions resolve model ownership on fresh low-level instances. Standalone FastMCP keeps its existing version-aware registry path. Includes documentation, the updated public API snapshot, and a Sampo minor changeset.Validation
One feedback test still reports the existing
nextCursordeprecation warning; feedback behavior is outside this change. No live model-compliance or production-ingestion test was performed.Release
.sampo/changesets/mcp-analytics-defaults.mdrequests a minor release. Review and merge, then approve the normal release workflow; no package has been published from this branch.Agent context
Autonomy: Human-driven (agent-assisted), directed by Lucas Faria.
Implemented with Codex using local shell, GitHub CLI, and CodeScene MCP. Fresh-instance failures were reproduced before implementation. Defaults changed on the existing API; no new option was introduced. Human review required.