Repository navigation
Fix what the review of the MCP and format work found - #49
Merged
Merged
Conversation
- An MCP tool name splits into a server and a tool only when exactly one "__" follows "mcp__", counting the ones that overlap. mcp__foo___bar is foo and _bar, or foo_ and bar, so it is left whole. It was split as the first. - On one record, a node on the whole record comes before the nodes on its parts, which follow their block. The two were decided by id, which made a cycle, so a sort could return any order. The OAP already orders them this way. - The page reads record times by decoding each record, so they end where Session Data says a reader stops: at a line that does not decode. On a 142 MB session this takes about 0.25 s more. A file's and a node's first and last time keep a time before 1970, and no longer depend on map order when a record is at exactly 1970-01-01T00:00:00Z, which counts as none. - asz.view no longer says the document carries no RFC 3339 strings: a workspace change and a tool execution keep their record's time as one. It says how siblings on one record are ordered. Session Data and the OpenTelemetry export list the execution kind. The documents of the 94 real conversations measured are 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.
A review of the latest changes (#44 to #48) against the OAP and Horizon found these. Each one holds on the code.
Changes
__followsmcp__, counting the ones that overlap.mcp__foo___barcan befooand_barorfoo_andbar, so it is left whole; it was split as the first. The split feeds the landed parts, the step'sattrs, the metric labels and, through them, the OAP's endpoint names.block, then the id. The whole record and a part were decided by id, which made a cycle (block 1 before the whole record, the whole record before block 0, block 0 before block 1), so a sort could return any of them. The OAP already orders them this way, andasz-view.mdnow says so.asz-view.mdsaid the document carries no RFC 3339 strings, but a workspace change and a tool execution keep their record'stimeas one.session-data.mdandotlp.mdlistexecutionamong the file kinds.docs/en/changes/changes.mdrecords each change.Checked
offis a string, and a record after it. Each fails with its fix reverted.make checkpasses.