Skip to content

Fix what the review of the MCP and format work found - #49

Merged
wu-sheng merged 1 commit into
mainfrom
fix-review-edges
Sep 28, 2026
Merged

wu-sheng merged 1 commit into
mainfrom
fix-review-edges

Conversation

@wu-sheng

Copy link
Copy Markdown
Member

A review of the latest changes (#44 to #48) against the OAP and Horizon found these. Each one holds on the code.

Changes

  • MCP names. A Claude Code MCP tool name splits into a server and a tool only when exactly one __ follows mcp__, counting the ones that overlap. mcp__foo___bar can be foo and _bar or foo_ and bar, so it is left whole; it was split as the first. The split feeds the landed parts, the step's attrs, the metric labels and, through them, the OAP's endpoint names.
  • Order of nodes on one record. A node on the whole record comes before the nodes on its parts, which follow their 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, and asz-view.md now says so.
  • Where record times stop. The page read each record's time from its raw line, so it kept reading past a line that does not decode, although Session Data says a reader stops there, as the OAP does. It now decodes each record. On the largest real session, 142 MB, a conversation read takes about 0.25 s more (2.3 s to 2.55 s).
  • Times at the edges. A file's and a node's first and last time keep a time before 1970; the last one used to stay at 0 and be dropped, where the OAP keeps it. A record at exactly 1970-01-01T00:00:00Z counts as none, as 0 already reads, so a file's first time no longer depends on the order Go gives a map.
  • Docs. asz-view.md said the document carries no RFC 3339 strings, but a workspace change and a tool execution keep their record's time as one. session-data.md and otlp.md list execution among the file kinds. docs/en/changes/changes.md records each change.

Checked

  • New tests: the MCP split for ten names, the order of three nodes on one record in all six input orders, and the page's times over a landed file with two records before 1970, a line whose off is a string, and a record after it. Each fails with its fix reverted.
  • The documents of the 94 real conversations measured (65 Claude Code, 8 with MCP calls, 21 LangChain) are byte-identical before and after.
  • make check passes.

- 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.
@wu-sheng wu-sheng added this to the 0.6.0 milestone Sep 28, 2026
@wu-sheng
wu-sheng merged commit 6dbd2ba into main Sep 28, 2026
24 checks passed
@wu-sheng
wu-sheng deleted the fix-review-edges branch September 28, 2026 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant