feat(e001-s003): tracing store, /trace/:id route, planner events, met…

[Dumidu1212]

Add completed tests

Summary by CodeRabbit

• New Features

  • Enhanced observability with metrics for total traces created and total trace events recorded.
  • Application initialization now supports injecting a tracing store via configuration.

• Bug Fixes

  • More reliable trace cleanup by using a single, consistent deletion path to keep state in sync.

• Tests

  • End-to-end tests now initialize the app with tracing enabled to validate routes and plugins that depend on trace data.

[coderabbitai]

Walkthrough

Adds metrics instrumentation to TraceStore for trace creation and event recording, adjusts prune to use delete(id), and updates E2E tests to inject a shared TraceStore via buildApp’s options.

Changes

Cohort / File(s)	Summary
Tracing metrics integration src/tracing/traceStore.ts	Import traceCreatedTotal and traceEventsTotal. Increment counters in create() and record(). Update prune() to evict via delete(id) instead of direct map deletion.
E2E app wiring for tracing tests/e2e/plan.e2e.test.ts, tests/e2e/tools.e2e.test.ts	Instantiate a shared TraceStore in beforeAll and pass it to buildApp({ registry, planner, traces }). Reflects an expanded buildApp options signature to accept traces.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor T as Test Runner
  participant B as buildApp
  participant A as App
  participant S as TraceStore
  participant M as Metrics

  T->>B: buildApp({ registry, planner, traces: S })
  B->>A: Initialize App with injected TraceStore
  Note over A,S: App uses shared TraceStore instance

  A->>S: create(trace)
  S->>M: increment traceCreatedTotal

  A->>S: record(event)
  S->>M: increment traceEventsTotal

  alt prune capacity reached
    A->>S: prune()
    S->>S: delete(oldestId)
  end

  Note over M: Metrics reflect created traces and events

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Possibly related PRs

• feat(e001-s003): trace store with TTL/capacity and tracing metrics #8 — Introduces TraceStore and metrics; closely aligned with this PR’s metric increments and prune behavior adjustments.

Poem

A hop, a trace, a counter’s tick—
I sniff the logs and count them quick.
Create, record, prune old to new,
The carrots add to metrics too.
With shared stores wired end-to-end,
Our traces bounce—and never bend. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title touches on real changes such as adding a TraceStore, tracing metrics, and planner event recording, but it also mentions a /trace/:id route that isn't implemented in this PR and the trailing ellipsis reduces clarity. Although it lists relevant features, it is overly long and tries to cover too many details at once. Under our guidelines, referencing actual parts of the changeset qualifies it as partially related and thus acceptable.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/e001-s003-tracing

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a729cc3 and 001e59c.

📒 Files selected for processing (3)

• src/tracing/traceStore.ts (4 hunks)
• tests/e2e/plan.e2e.test.ts (1 hunks)
• tests/e2e/tools.e2e.test.ts (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

tests/e2e/tools.e2e.test.ts (2)

src/tracing/traceStore.ts (1)

• TraceStore (26-93)

src/app.ts (1)

• buildApp (14-31)

src/tracing/traceStore.ts (1)

src/metrics/metrics.ts (2)

• traceCreatedTotal (43-46)
• traceEventsTotal (47-50)

tests/e2e/plan.e2e.test.ts (4)

src/tracing/traceStore.ts (1)

• TraceStore (26-93)

src/registry/service.ts (1)

• IRegistryService (4-4)

src/planner/planner.ts (1)

• Planner (53-215)

src/app.ts (1)

• buildApp (14-31)

🔇 Additional comments (5)

src/tracing/traceStore.ts (3)

1-1: LGTM!

The metrics imports are correctly added to support observability of trace operations.

---

43-43: LGTM!

Incrementing traceCreatedTotal on trace creation provides valuable observability into trace lifecycle.

---

79-80: LGTM!

Using this.delete(oldest) instead of directly manipulating this.traces ensures consistent state updates for both the traces Map and the order array, preventing potential inconsistencies.

tests/e2e/tools.e2e.test.ts (1)

1-1: LGTM!

The TraceStore wiring is correctly implemented. The test properly instantiates a TraceStore instance and injects it into buildApp, enabling route/plugin initialization that depends on tracing data.

Also applies to: 27-27, 30-32

tests/e2e/plan.e2e.test.ts (1)

38-38: Verify: Two separate TraceStore instances in test.

Line 46 instantiates the Planner with its own TraceStore, while Line 47 passes a different traces instance to buildApp. This means:

• Traces created by the Planner (via /plan endpoint) will be stored in the Planner's internal TraceStore
• The /trace/:id route will query the separate traces instance passed to buildApp
• Traces created by planning operations won't be retrievable via trace routes

If this separation is intentional for testing isolation, consider adding a comment to clarify. Otherwise, pass the same traces instance to both the Planner and buildApp:

-    const planner = new Planner(registry, new SimpleScorer(), new StubExec(), new TraceStore());
-    app = buildApp({ registry, planner, traces });
+    const planner = new Planner(registry, new SimpleScorer(), new StubExec(), traces);
+    app = buildApp({ registry, planner, traces });

Also applies to: 41-41, 46-47

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Critical: Double-counting trace events.

The traceEventsTotal counter is incremented both here in TraceStore.record() and in Planner.rec() (which calls this.traces.record()). Every event will be counted twice.

Remove the duplicate increment from one of these locations. Since TraceStore is the source of truth, keep the increment here and remove it from Planner.rec():

In src/planner/planner.ts, update the rec() method:

private rec(traceId: string, type: string, data: unknown): void {
  this.traces.record(traceId, type, data);
  // Remove: traceEventsTotal.inc();
}

🤖 Prompt for AI Agents

In src/tracing/traceStore.ts around line 52, TraceStore currently increments
traceEventsTotal (traceEventsTotal.inc()); the duplicate increment lives in
src/planner/planner.ts inside the rec() method. Keep the increment here
(TraceStore) as the single source of truth and remove the traceEventsTotal.inc()
call from Planner.rec(); update src/planner/planner.ts::rec to only call
this.traces.record(traceId, type, data) and delete the extra counter increment
so each event is counted once.