Skip to content

prompt-event: add tool/model/timing/text fields#1017

Open
shortdiv wants to merge 1 commit intofeat/prompt-event-metricfrom
feat/prompt-event-fields
Open

prompt-event: add tool/model/timing/text fields#1017
shortdiv wants to merge 1 commit intofeat/prompt-event-metricfrom
feat/prompt-event-fields

Conversation

@shortdiv
Copy link
Copy Markdown
Contributor

@shortdiv shortdiv commented Apr 7, 2026

Summary

  • Extends PromptEventValues with sparse positional fields needed for the Sessions UI: tool_call_name, model, error, start_ts, reported_duration_ms, observed_duration_ms, input_hash, input_text.
  • Stacked on top of Add PromptEvent metric for tracking prompt events #852 (feat/prompt-event-metric) — targets that branch, not main.

Why

The base PR introduces the PromptEvent metric (event id 5) but only carries the core identification fields (kind, event_id, parent). The downstream Sessions dashboard in the monorepo needs:

  • model on AiMessage / ThinkingMessage to attribute turns
  • tool_call_name + error on ToolCall for tool usage / friction analysis
  • start_ts / *_duration_ms for session timing
  • input_text (truncated) on HumanMessage / AiMessage for friction regex + session titles
  • input_hash for dedupe across re-emits

Test plan

  • cargo check / cargo test on the git-ai crate
  • napi build picks up the new fields and round-trips them through parseMetricsBatch
  • End-to-end: emit a real prompt with claude-code, verify all new fields populate in the local ClickHouse prompt_events table

🤖 Generated with Claude Code


Open with Devin

Extends PromptEventValues with fields needed by the sessions UI:
tool_call_name, model, error, start_ts, reported_duration_ms,
observed_duration_ms, input_hash, input_text. Adds SkillInvocation
and McpCall kinds. Wires the Claude transcript parser to populate
the new fields where they're available; fallback hook paths set
input_hash + tool_call_name. Existing fields are unchanged so this
is back-compat with PR 852.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@shortdiv
Copy link
Copy Markdown
Contributor Author

shortdiv commented Apr 8, 2026

Proposal: add skill file metadata to SkillInvocation events

Context: we're building a skills leaderboard in the monorepo Sessions UI and the three things we can't derive from invocation data alone are "when was this skill last edited?", "when was it first authored?", and "did two sessions invoke the same version of the skill?". All three are cheap to get if the emitter stats the SKILL.md file at invocation time.

I'd like to add three sparse fields to PromptEventValues, populated only when kind = SkillInvocation:

pub mod prompt_event_pos {
    // ... existing 0..11 ...
    pub const SKILL_FILE_PATH: usize = 12;   // String  - absolute path to SKILL.md
    pub const SKILL_FILE_MTIME: usize = 13;  // u64     - file mtime (unix ms)
    pub const SKILL_FILE_HASH: usize = 14;   // String  - sha256[..16] of file contents
}

Why denormalized on the event, not a separate skills table

The invocation log already is the usage data. The leaderboard query becomes:

SELECT
  skill_file_path                AS path,
  anyLast(tool_call_name)        AS name,
  MIN(start_ts)                  AS first_seen,
  MAX(skill_file_mtime)          AS last_edited,
  countDistinct(prompt_id)       AS sessions,
  countDistinct(human_author)    AS distinct_users,
  countDistinct(skill_file_hash) AS distinct_versions
FROM prompt_events
WHERE kind = 'SkillInvocation'
  AND org_id = ?
GROUP BY skill_file_path

No second table, no upsert logic, no GC. A few hundred redundant bytes per invocation — negligible at our scale. If we later need `creator` (from `git blame`) or `description` (from frontmatter), that's when a cached `skills` entity table earns its keep. Invocation-level data shouldn't go there.

Where the path comes from

In `parse_claude_transcript`, the skill case currently lands in the `tool_use` branch once `kind_for_tool` returns `SKILL_INVOCATION`. The tool_use payload for Claude's `Skill` / `SlashCommand` tools contains the skill name in `item["input"]`. We resolve that to a filesystem path by searching the standard Claude skill locations in order:

  1. `/.claude/skills//SKILL.md`
  2. `/.claude/skills/.md`
  3. `$HOME/.claude/skills//SKILL.md`
  4. `$HOME/.claude/skills/.md`

First hit wins. If none exist (shouldn't happen in practice, but), we emit the event with the other fields and leave all three new fields null — sparse encoding handles it.

Wiring sketch

// new helper in src/commands/prompt_event.rs
struct SkillFileMeta {
    path: String,
    mtime_ms: u64,
    hash: String,
}

fn resolve_skill_file(skill_name: &str, cwd: &Path) -> Option<SkillFileMeta> {
    let candidates = [
        cwd.join(\".claude/skills\").join(skill_name).join(\"SKILL.md\"),
        cwd.join(\".claude/skills\").join(format!(\"{skill_name}.md\")),
        dirs::home_dir()?.join(\".claude/skills\").join(skill_name).join(\"SKILL.md\"),
        dirs::home_dir()?.join(\".claude/skills\").join(format!(\"{skill_name}.md\")),
    ];
    for path in candidates {
        let Ok(meta) = std::fs::metadata(&path) else { continue };
        let Ok(mtime) = meta.modified() else { continue };
        let mtime_ms = mtime
            .duration_since(std::time::UNIX_EPOCH)
            .ok()?
            .as_millis() as u64;
        let bytes = std::fs::read(&path).ok()?;
        let mut hasher = Sha256::new();
        hasher.update(&bytes);
        let hash = format!(\"{:x}\", hasher.finalize())[..16].to_string();
        return Some(SkillFileMeta {
            path: path.to_string_lossy().into_owned(),
            mtime_ms,
            hash,
        });
    }
    None
}

Then at the `tool_use` arm in `parse_claude_transcript` (and symmetrically in `emit_single_event_from_hook` for the PreToolUse path):

let skill_file = if kind == prompt_event_kind::SKILL_INVOCATION {
    item[\"input\"][\"skill_name\"]
        .as_str()
        .or_else(|| item[\"input\"][\"command\"].as_str())
        .and_then(|name| resolve_skill_file(name.trim_start_matches('/'), &cwd))
} else {
    None
};

And on the `PromptEventValues` builder:

if let Some(meta) = &evt.skill_file {
    values = values
        .skill_file_path(&meta.path)
        .skill_file_mtime(meta.mtime_ms)
        .skill_file_hash(&meta.hash);
}

Scope note

This is Claude-only in the first cut. Cursor / Codex / etc. handle skills (or their equivalents) differently — some don't have a SKILL.md-on-disk model at all. For those agents we'd leave the fields null and the leaderboard would just show them as "unresolved." I think that's the right default: don't pretend we have data we don't, and the UI can render "—" for unresolved rows.

Happy to open a follow-up PR on top of this one if you agree with the shape.

@shortdiv
Copy link
Copy Markdown
Contributor Author

shortdiv commented Apr 8, 2026

Addendum: daemon telemetry context

After digging into the daemon code I wanted to add context I didn't have when writing the proposal above — it answers open question #1 and tightens the server-side reaper story.

How telemetry flows today:

  • Wrapper hooks are fire-and-forget over a Unix control socket to the long-lived daemon (ensure_daemon_running in src/commands/daemon.rs:195, daemon_is_up at :237).
  • The daemon buffers envelopes in memory and flushes every 3 seconds via telemetry_flush_loop (src/daemon/telemetry_worker.rs:19, FLUSH_INTERVAL).
  • PromptEvent (metric id 5) rides the same path → POST /worker/metrics/upload, batches ≤250, SQLite fallback on failure (src/api/metrics.rs:48).
  • The only periodic daemon work is daemon_update_check_loop at src/daemon.rs:7409, which runs hourly and does GC + version check + uptime cap (DAEMON_MAX_UPTIME_SECS = 24h30m). It emits nothing session-shaped.
  • The closest thing to session state today is WrapperPreState / WrapperPostState (src/daemon.rs:6826), keyed by invocation_id, scoped to a single git command invocation (not an AI prompt session), GC'd after ~1h.

There is no current daemon heartbeat. I grepped the same terms and confirmed. Socket reachability is the only liveness notion, and it only gates wrapper → daemon RPC, not session → server.

What this means for the proposal:

The good news — the 3s flush interval means we don't need a dedicated heartbeat. Any HumanMessage / ToolCall / PreToolUse already reaches the server within ~3s of being emitted, so max(event_ts) per session_id on the server side is a tight enough liveness proxy for the 60s / 10min thresholds I proposed. The "any existing event implicitly updates last_seen_at" line in the proposal is load-bearing and the 3s flush makes it actually true.

The bad news — hard wrapper death is invisible to the daemon too. If the wrapper process is kill -9'd, terminal closed, laptop lid slammed, OOM killed, etc., the SIGTERM/SIGINT handler I proposed never runs. Nothing reaches the socket. From the daemon's perspective there's just… silence, and the daemon itself has no per-session state to notice the silence from (remember, WrapperPreState is keyed by git invocation, not prompt session). This means:

  • Client-side session_end(user_quit) is best-effort — we should not treat its absence as a signal of anything.
  • The server-side reaper is the only reliable source of truth for termination. It must always run, even if the daemon is "well-behaved," because the daemon fundamentally can't see its own death.
  • The 10min reap threshold in the proposal is doing real work, not just catching edge cases.

Small refinement to the proposal:

The daemon could cheaply help the reaper by adding a second in-flight map, parallel to WrapperPreState, keyed by session_id, updated on every outbound telemetry batch. Then in the hourly GC loop (or a faster 60s one — we'd need to add it), any session_id whose last_flush_ts is older than the idle threshold gets an session_end(idle_timeout) synthesized and emitted on the next flush. This gives us a second source of end events that doesn't require the wrapper to be alive — just the daemon. The server-side reaper stays as the final backstop for when the daemon itself dies.

So the layered story becomes:

  1. Wrapper alive, clean exit → SIGTERM handler emits session_end(user_quit). Arrives within 3s.
  2. Wrapper dies hard, daemon alive → daemon's own idle sweep synthesizes session_end(idle_timeout) after 30min. Arrives on next flush.
  3. Daemon also dead → server-side reaper synthesizes session_end(signal) after 10min of silence.

Three independent mechanisms, each catching what the layer above can't. The daemon-side sweep is optional for the MVP (rollout step 1 still works with just 1 and 3), but worth designing space for now.

Refined answer to open question #3 ("what counts as a user-facing event for the idle timer"): given the 3s flush, we can afford to be stricter than I initially thought. HumanMessage and ToolCall clearly yes. AiMessage I'd now say no — if the agent is talking to itself with no human input for 30min, that is an idle session from the user's perspective, regardless of AI activity. This matches the intuition that "idle" = "human walked away," not "nothing is happening."

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.

2 participants