feat(#52): runtime attribution instrumentation (Slice 1) - #68
Conversation
Extend the small-tier baseline with lifecycle phase labels, product-side observation diagnostics, and a machine-readable attribution report so #52 can name dominant costs before any architecture changes. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Document measured PID refusal causes from #60 attribution and analytically eliminate alternative families to select ≤2 bounded spikes for Slice 3. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
…ice 4) Finalize practical runtime architecture decision (D+A topology, retain ADR-012 hybrid dense), remove Node backend and seedGraph skip from default baseline path, and confine spike repro helpers to run-da-spike.mjs. Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6370538279
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const environment = Object.fromEntries(entries | ||
| .filter(([name]) => !isGraphdExecutionHook(name, platform) | ||
| && (platform !== 'win32' || name.toLowerCase() !== 'path'))); | ||
| Object.assign(environment, graphdThreadEnvironment(inheritedEnvironment)); |
There was a problem hiding this comment.
When graphdEnvironmentFor receives either a normal environment or one with LAMINA_RUNTIME_BOUNDED_TOPOLOGY=1, it passes the raw environment object to graphdThreadEnvironment, which expects a parsed budget containing graphd_threads. Consequently threadLimitEnvironment(undefined) emits values such as OMP_NUM_THREADS='NaN' and UV_THREADPOOL_SIZE='NaN'; bounded runs therefore do not apply the requested graphd cap and can still hit the PID ceiling this feature is intended to prevent. Pass runtimeBudgetFromEnvironment(inheritedEnvironment) instead, allowing the helper to return an empty object when the feature is disabled.
Useful? React with 👍 / 👎.
| } : null, | ||
| mode: live ? 'live' : invalidate ? 'rebuild' : discover ? 'discover' : 'observe', | ||
| subprocess_launches: { | ||
| cocoindex_worker: workerDiagnostics.filter((item) => item.ok).length, |
There was a problem hiding this comment.
When bounded topology selects the Node backend, runWorker calls observeNode in-process and still appends a successful diagnostic, so this expression reports one cocoindex_worker subprocess per attempt even though no CocoIndex process was launched. This already makes the bounded-topology attribution claim a worker launch that did not occur; gate the count on backend === COCOINDEX_BACKEND so the experiment's process-topology evidence remains accurate.
Useful? React with 👍 / 👎.
| }); | ||
|
|
||
| const ROLE_ALIASES = Object.freeze({ | ||
| graphd: 'graphd_startup', |
There was a problem hiding this comment.
The launch accumulator declares a graphd bucket, but this alias converts every graphd event to graphd_startup, which is absent from that accumulator. Both direct recording and merged product attribution therefore fall through to other, causing the top-level attribution report to show zero graphd launches and inflated unknown launches even when the product attribution reports graphd startup.
Useful? React with 👍 / 👎.
| mode: live ? 'live' : invalidate ? 'rebuild' : discover ? 'discover' : 'observe', | ||
| subprocess_launches: { | ||
| cocoindex_worker: workerDiagnostics.filter((item) => item.ok).length, | ||
| graphd: compatibilityRecovery ? 2 : 1, |
There was a problem hiding this comment.
For observations performed after seedGraph or against any already-running compatible daemon, ensureGraphd reuses that daemon, but the attribution unconditionally records one graphd subprocess (and two after compatibility recovery). Once later baseline scenarios are unblocked, this overcounts graphd startups throughout the report; derive the count from whether the initial ensureGraphd call actually spawned a daemon rather than assuming every observation starts one.
Useful? React with 👍 / 👎.
1 participant
Summary
Phase 1 of #52 / epic #49: runtime attribution, root-cause elimination, bounded spikes, and ADR-015 accepted.
Slice 1 — Attribution instrumentation
observe.mjssuccess responses.benchmarks/runtime-baseline-v1/attribution/small.json.Slice 2 — Root-cause elimination matrix
Slice 3 — Bounded spikes (≤2)
initial-observationvalid atpids.max=64, peak 47 tasks — evidencebenchmarks/runtime-baseline-v1/spikes/da-bounded-topology.jsonbenchmarks/runtime-baseline-v1/spikes/b-lexical-first.jsonpackages/cli/lib/runtime-budget.mjsSlice 4 — ADR finalization + issue rewrite
docs/decisions/015-practical-runtime-architecture.mdspikes/run-da-spike.mjs#52 handoff to Phase 2
Start Phase 2 with #69 (topology/memory ownership) — must complete small
initial-observationunder 64 tasks without spike-only Node backend.Test plan
npm run test:runtime-baselinenpm run test:safe-runnernpm run test:semantic-oraclenpm run test:real-repository-oraclenode tests/runtime_budget_test.mjsnode tests/observation_diagnostics_test.mjsCloses #52. Phase 1 of epic #49 complete; Phase 2 begins at #69.
Made with Cursor