fix(pi): classify native skill loads as Skill, not Read (#588) (#590)

Pi and OMP have no dedicated skill tool: a native skill load is emitted as
an ordinary `read` tool call whose path points at the skill's SKILL.md (or a
`skill://<name>` URI in newer OMP builds). The parser mapped every read to the
Read tool and never populated `skills`, so Pi/OMP sessions over-counted Reads
AND always showed an empty "Skills & Agents" breakdown.

Detect these reads (basename === 'SKILL.md', or a skill:// URI), extract the
skill name (parent directory, or the URI segment), and surface the call as the
`Skill` tool with the name recorded in `skills` -- exactly how the Claude
parser represents a skill invocation. That both removes the Read over-count and
lets the shared classifier tag the turn `general` so the Skills & Agents
breakdown picks it up (populating a field the dashboard never received before).
The path is read from arguments.path with a defensive arguments.file_path
fallback.

Tests cover SKILL.md / skill:// / file_path detection, a non-skill read staying
a Read, and an end-to-end check that a parsed skill load reaches the classifier
subCategory that feeds skillBreakdown.

docs/providers/pi.md

@@ -27,6 +27,7 @@ Per `<provider>:<path>:<responseId>` when a response ID is present, falling back
 - Undefined token fields in `message.usage` are coerced to `0` (`pi.ts:156-159`); never `undefined`.
 - The provider name is taken from `source.provider` (`pi.ts:182`), not hard-coded. This matters because `pi.ts` is the parser for **both** Pi and OMP; see [`omp.md`](omp.md).
 - Tool-call content type is extracted from the message envelope (`pi.ts:169-176`).
+- Pi/OMP have no dedicated skill tool: a native skill load is a `read` whose path points at a skill's `SKILL.md` (or a `skill://<name>` URI in newer OMP builds). The parser surfaces these as the `Skill` tool and records the name in `skills` (mirroring the Claude parser) instead of counting a `Read`, so the shared classifier tags the turn `general` and the Skills & Agents breakdown picks it up (`skillLoadName`, issue #588).
 
 ## When fixing a bug here
 

src/providers/pi.ts

@@ -35,6 +35,34 @@ const toolNameMap: Record<string, string> = {
 // Pre-sorted by key length descending so longer/more-specific keys match first
 const modelDisplayEntries = Object.entries(modelDisplayNames).sort((a, b) => b[0].length - a[0].length)
 
+// Pi/OMP have no dedicated skill tool the way Claude Code does. A native skill
+// load is emitted as an ordinary `read` tool call whose path points at the
+// skill's `SKILL.md` (Pi resolves skills from many roots: ~/.pi/agent/skills,
+// project .pi/skills, .agents/skills, package skills/, --skill <path>), or, in
+// newer OMP builds, at a `skill://<name>` URI. Left untouched these inflate the
+// Read tool count and leave the Skills dimension empty (issue #588). Return the
+// skill name when a read is really a skill load, else null so it stays a Read.
+function skillLoadName(name: string | undefined, args: Record<string, unknown> | undefined): string | null {
+  if (name !== 'read') return null
+  const raw = args?.['path'] ?? args?.['file_path']
+  if (typeof raw !== 'string') return null
+  const path = raw.trim()
+  if (path.length === 0) return null
+
+  if (path.startsWith('skill://')) {
+    const rest = path.slice('skill://'.length).replace(/^\/+/, '')
+    const first = rest.split(/[/?#]/)[0]?.trim() ?? ''
+    return first.length > 0 ? first : null
+  }
+
+  // Match on the SKILL.md basename, not a directory prefix, because skill roots
+  // live in many locations. Split on both separators so Windows paths work.
+  const segments = path.split(/[\\/]/).filter(Boolean)
+  if (segments[segments.length - 1] !== 'SKILL.md') return null
+  const parent = segments[segments.length - 2]?.trim()
+  return parent && parent.length > 0 ? parent : null
+}
+
 type PiEntry = {
   type: string
   id?: string
@@ -169,7 +197,25 @@ function createParser(source: SessionSource, seenKeys: Set<string>): SessionPars
         seenKeys.add(dedupKey)
 
         const toolCalls = normalizeContentBlocks(msg.content).filter(c => c.type === 'toolCall' && c.name)
-        const tools = toolCalls.map(c => toolNameMap[c.name!] ?? c.name!)
+
+        // A SKILL.md-loading read is surfaced as the `Skill` tool (not `Read`)
+        // and its name is recorded in `skills`. This mirrors how the Claude
+        // parser represents a skill invocation, so the shared classifier tags
+        // the turn `general` and the "Skills & Agents" breakdown picks it up,
+        // instead of over-counting a Read and leaving Skills empty (#588).
+        // Every other call stays a normal tool.
+        const tools: string[] = []
+        const skills: string[] = []
+        for (const c of toolCalls) {
+          const skill = skillLoadName(c.name, c.arguments)
+          if (skill !== null) {
+            skills.push(skill)
+            tools.push('Skill')
+            continue
+          }
+          tools.push(toolNameMap[c.name!] ?? c.name!)
+        }
+
         const bashCommands = toolCalls
           .filter(c => c.name === 'bash')
           .flatMap(c => {
@@ -193,6 +239,7 @@ function createParser(source: SessionSource, seenKeys: Set<string>): SessionPars
           costUSD,
           tools,
           bashCommands,
+          skills,
           timestamp,
           speed: 'standard',
           deduplicationKey: dedupKey,

tests/providers/pi.test.ts

@@ -5,6 +5,39 @@ import { tmpdir } from 'os'
 
 import { createPiProvider } from '../../src/providers/pi.js'
 import type { ParsedProviderCall } from '../../src/providers/types.js'
+import { classifyTurn } from '../../src/classifier.js'
+import type { ParsedApiCall, ParsedTurn } from '../../src/types.js'
+
+// Mirrors src/parser.ts providerCallToTurn so we can assert that a Pi call's
+// skills survive the classifier into `subCategory`, which is the sole input the
+// session summary reads to build the "Skills & Agents" breakdown (#588).
+function turnFromPiCall(call: ParsedProviderCall, userMessage = ''): ParsedTurn {
+  const apiCall: ParsedApiCall = {
+    provider: call.provider,
+    model: call.model,
+    usage: {
+      inputTokens: call.inputTokens,
+      outputTokens: call.outputTokens,
+      cacheCreationInputTokens: call.cacheCreationInputTokens,
+      cacheReadInputTokens: call.cacheReadInputTokens,
+      cachedInputTokens: call.cachedInputTokens,
+      reasoningTokens: call.reasoningTokens,
+      webSearchRequests: call.webSearchRequests,
+    },
+    costUSD: call.costUSD,
+    tools: call.tools,
+    mcpTools: call.tools.filter(t => t.startsWith('mcp__')),
+    skills: call.skills ?? [],
+    subagentTypes: call.subagentTypes ?? [],
+    hasAgentSpawn: call.tools.includes('Agent'),
+    hasPlanMode: call.tools.includes('EnterPlanMode'),
+    speed: call.speed,
+    timestamp: call.timestamp,
+    bashCommands: call.bashCommands,
+    deduplicationKey: call.deduplicationKey,
+  }
+  return { userMessage, assistantCalls: [apiCall], timestamp: call.timestamp, sessionId: call.sessionId }
+}
 
 let tmpDir: string
 
@@ -48,14 +81,20 @@ function assistantMessage(opts: {
   output?: number
   cacheRead?: number
   cacheWrite?: number
-  tools?: Array<{ name: string; command?: string }>
+  tools?: Array<{ name: string; command?: string; path?: string; filePath?: string }>
 }) {
-  const content = (opts.tools ?? []).map(t => ({
-    type: 'toolCall',
-    id: `call-${t.name}`,
-    name: t.name,
-    arguments: t.command !== undefined ? { command: t.command } : {},
-  }))
+  const content = (opts.tools ?? []).map(t => {
+    const args: Record<string, unknown> = {}
+    if (t.command !== undefined) args['command'] = t.command
+    if (t.path !== undefined) args['path'] = t.path
+    if (t.filePath !== undefined) args['file_path'] = t.filePath
+    return {
+      type: 'toolCall',
+      id: `call-${t.name}`,
+      name: t.name,
+      arguments: args,
+    }
+  })
 
   return JSON.stringify({
     type: 'message',
@@ -263,6 +302,115 @@ describe('pi provider - JSONL parsing', () => {
     expect(calls[0]!.bashCommands).toEqual(['git', 'bun'])
   })
 
+  it('classifies a SKILL.md read as a skill load, not a Read (#588)', async () => {
+    const projectDir = join(tmpDir, '--Users-test-myproject--')
+    const filePath = await writeSession(projectDir, 'session.jsonl', [
+      sessionMeta(),
+      assistantMessage({
+        tools: [
+          { name: 'read', path: '/Volumes/T7/repos/cuneiform/.pi/skills/bmad-create-story/SKILL.md' },
+          { name: 'read', path: '/Volumes/T7/repos/cuneiform/workflow.md' },
+          { name: 'edit', path: '/Volumes/T7/repos/cuneiform/workflow.md' },
+        ],
+      }),
+    ])
+
+    const provider = createPiProvider(tmpDir)
+    const source = { path: filePath, project: 'myproject', provider: 'pi' }
+    const calls: ParsedProviderCall[] = []
+    for await (const call of provider.createSessionParser(source, new Set()).parse()) {
+      calls.push(call)
+    }
+
+    // The SKILL.md read is surfaced as the Skill tool (not Read); the plain
+    // read stays a Read.
+    expect(calls[0]!.skills).toEqual(['bmad-create-story'])
+    expect(calls[0]!.tools).toEqual(['Skill', 'Read', 'Edit'])
+  })
+
+  it('classifies a skill:// read as a skill load (OMP-style URI)', async () => {
+    const projectDir = join(tmpDir, '--Users-test-myproject--')
+    const filePath = await writeSession(projectDir, 'session.jsonl', [
+      sessionMeta(),
+      assistantMessage({
+        tools: [{ name: 'read', path: 'skill://commit-workflow' }],
+      }),
+    ])
+
+    const provider = createPiProvider(tmpDir)
+    const source = { path: filePath, project: 'myproject', provider: 'pi' }
+    const calls: ParsedProviderCall[] = []
+    for await (const call of provider.createSessionParser(source, new Set()).parse()) {
+      calls.push(call)
+    }
+
+    expect(calls[0]!.skills).toEqual(['commit-workflow'])
+    expect(calls[0]!.tools).toEqual(['Skill'])
+  })
+
+  it('reads the file_path key as a fallback for skill loads', async () => {
+    const projectDir = join(tmpDir, '--Users-test-myproject--')
+    const filePath = await writeSession(projectDir, 'session.jsonl', [
+      sessionMeta(),
+      assistantMessage({
+        tools: [{ name: 'read', filePath: '/home/u/.agents/skills/deep-research/SKILL.md' }],
+      }),
+    ])
+
+    const provider = createPiProvider(tmpDir)
+    const source = { path: filePath, project: 'myproject', provider: 'pi' }
+    const calls: ParsedProviderCall[] = []
+    for await (const call of provider.createSessionParser(source, new Set()).parse()) {
+      calls.push(call)
+    }
+
+    expect(calls[0]!.skills).toEqual(['deep-research'])
+    expect(calls[0]!.tools).toEqual(['Skill'])
+  })
+
+  it('leaves a normal read (no SKILL.md) as a Read with no skills', async () => {
+    const projectDir = join(tmpDir, '--Users-test-myproject--')
+    const filePath = await writeSession(projectDir, 'session.jsonl', [
+      sessionMeta(),
+      assistantMessage({
+        tools: [{ name: 'read', path: '/home/u/project/src/skill-loader.ts' }],
+      }),
+    ])
+
+    const provider = createPiProvider(tmpDir)
+    const source = { path: filePath, project: 'myproject', provider: 'pi' }
+    const calls: ParsedProviderCall[] = []
+    for await (const call of provider.createSessionParser(source, new Set()).parse()) {
+      calls.push(call)
+    }
+
+    expect(calls[0]!.skills).toEqual([])
+    expect(calls[0]!.tools).toEqual(['Read'])
+  })
+
+  it('a pure skill-load turn classifies as general with the skill as subCategory (feeds the Skills breakdown, #588)', async () => {
+    const projectDir = join(tmpDir, '--Users-test-myproject--')
+    const filePath = await writeSession(projectDir, 'session.jsonl', [
+      sessionMeta(),
+      assistantMessage({
+        tools: [{ name: 'read', path: '/home/u/.pi/agent/skills/systematic-debugging/SKILL.md' }],
+      }),
+    ])
+
+    const provider = createPiProvider(tmpDir)
+    const source = { path: filePath, project: 'myproject', provider: 'pi' }
+    const calls: ParsedProviderCall[] = []
+    for await (const call of provider.createSessionParser(source, new Set()).parse()) {
+      calls.push(call)
+    }
+
+    // End to end: the parsed skill load must reach `subCategory`, or the
+    // Skills & Agents breakdown stays empty (the second half of #588).
+    const classified = classifyTurn(turnFromPiCall(calls[0]!))
+    expect(classified.category).toBe('general')
+    expect(classified.subCategory).toBe('systematic-debugging')
+  })
+
   it('skips assistant messages with zero tokens', async () => {
     const projectDir = join(tmpDir, '--Users-test-myproject--')
     const filePath = await writeSession(projectDir, 'session.jsonl', [