Skip to content

feat(ce-strategy,ce-product-pulse): add PM skills for upstream anchor and outcome pulse#614

Merged
tmchow merged 1 commit into
mainfrom
marcus/pm-skills
May 1, 2026
Merged

feat(ce-strategy,ce-product-pulse): add PM skills for upstream anchor and outcome pulse#614
tmchow merged 1 commit into
mainfrom
marcus/pm-skills

Conversation

@imwm
Copy link
Copy Markdown
Collaborator

@imwm imwm commented Apr 20, 2026

Summary

Adds PM skills ce-strategy and ce-product-pulse:

  • /ce-strategy — Interviews the user to produce docs/strategy.md: a Rumeltian problem/approach statement, persona(s), key metrics, and work tracks. Short and structured on purpose; re-runnable to update individual sections. ce:ideate, ce:brainstorm, and ce:plan read it as grounding when present.
  • /ce-product-pulse — Generates a single-page, time-windowed report (usage, performance, errors, followups) saved to ~/pulse-reports/ as a browseable timeline. First-time run interviews the user to define metrics and wire data sources (MCP-preferred, read-only DB only). Adapted from Spiral's "pulse" command.

Both skills use the dash-form ce-* naming convention going forward; existing ce:* skills can be renamed in a separate sweep.

Upstream/downstream wiring

The two skills bookend the existing loop — strategy anchors it, pulse closes it:

        Strategy (docs/strategy.md)
           |
           v
Brainstorm -> Plan -> Work -> Review -> Compound -> Pulse -> Repeat
                                                      |
                                                      v
                                        (~/pulse-reports/ signal for
                                         next strategy + brainstorm)

Three existing skills now read docs/strategy.md when present:

  • ce:brainstorm — Constraint Check pulls in strategy alongside AGENTS.md.
  • ce:ideate — quick-context sub-agent summarizes strategy and returns approach + active tracks so ideation weights toward strategy-aligned directions.
  • ce:plan — planning context summary includes target problem/approach/tracks; research flags plan decisions that pull away from active tracks.

ce-product-pulse also seeds from strategy when available: product name and key metrics are carried into data-source setup so the interview wires up connections to actually measure the metrics strategy defined.

What this does not do

  • No backlog prioritization, no issue-tracker updates, no scheduling (product-pulse hands off to the schedule skill with explicit confirmation).
  • Pulse is read-only: refuses read-write DB credentials, persists no PII in saved reports.
  • Strategy is an anchor doc, not a plan — features belong in ce:brainstorm, schedules in the tracker.

Files changed

  • plugins/compound-engineering/skills/ce-strategy/ (new) — SKILL.md + references/interview.md + references/strategy-template.md
  • plugins/compound-engineering/skills/ce-product-pulse/ (new) — SKILL.md + references/interview.md + references/report-template.md
  • plugins/compound-engineering/skills/ce-{brainstorm,ideate,plan}/SKILL.md — strategy-as-grounding integration
  • README.md, plugins/compound-engineering/README.md — workflow diagram, command tables, skill count 42→44

Test plan

  • bun run release:validate passes (45 skills, 50 agents)
  • bun test passes
  • /ce-strategy first-run flow produces a valid docs/strategy.md and re-runs update a single section without clobbering the rest
  • /ce-product-pulse first-run interview writes docs/product-pulse.md and refuses read-write DB creds; subsequent run produces a 30-40-line report at ~/pulse-reports/YYYY-MM-DD_HH-MM.md
  • /ce:brainstorm, /ce:ideate, /ce:plan all read docs/strategy.md when present and run cleanly when it is absent
  • Converter output is clean: bun run convert for opencode/codex targets with a test plugin including both skills


# Product Pulse

`ce-product-pulse` queries the product's data sources for a given time window and produces a compact, single-page report covering usage, performance, errors, and followups. The report is saved to `~/pulse-reports/` and the key points are surfaced in chat.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imwm wondering justification to have this in home directory vs in repo and checked in for benefit ongoing? Or do you expect users to use this when not having the repo?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Moved to docs/pulse-reports/. In-repo gives us the diffable, PR-reviewable timeline you mentioned plus better team-shared memory than a per-user home dir. Updated everywhere it appeared (skill body, frontmatter, references, both READMEs).


`ce-product-pulse` queries the product's data sources for a given time window and produces a compact, single-page report covering usage, performance, errors, and followups. The report is saved to `~/pulse-reports/` and the key points are surfaced in chat.

This is a read-only skill. It reports on what happened; it does not fix anything, ship anything, or change the product.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can skip this and enforce with tools in front matter instead? that not only guarantees it but will also reduce context window usage by eliminating extraenous tools

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took half of this. Added allowed-tools: [Read, Write, Glob, Grep, Bash, AskUserQuestion] in frontmatter, which trims context. I had to leave a one-paragraph prose statement in place because "read-only" was actually a slight overclaim. The skill does Write its config (docs/product-pulse.md) and report files. The new prose is more precise: "no product/DB/external mutations; skill's only writes are its own config and report files; MCP and data-source tools must be invoked read-only." Frontmatter can't enumerate MCP tools (analytics/tracing/payments vary per user setup), so the prose still does that work.

Copy link
Copy Markdown
Collaborator

@tmchow tmchow Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're storing config in docs/product-pulse.md? as in skill settings/preferences? or is this saying it's just reflecting back what values were used (vs looking up a config)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put the config in docs/product-pulse.md, but I'll change that to be more in line with the repo's .compound-engineering/config.local.yaml convention.

Comment thread plugins/compound-engineering/skills/ce-strategy/SKILL.md Outdated
Comment thread plugins/compound-engineering/skills/ce-strategy/SKILL.md Outdated
Comment thread plugins/compound-engineering/skills/ce-strategy/SKILL.md Outdated
Comment thread plugins/compound-engineering/skills/ce-strategy/SKILL.md Outdated
Comment thread plugins/compound-engineering/README.md Outdated
Copy link
Copy Markdown
Collaborator

@tmchow tmchow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other comment right now is whether we should be adding a new ce-strategy-alignment-reviewer agent that is used to essentially test: “Does this conflict with the project’s written strategy?” could be used in various skills where it makes sense. This then allows us to get ongoing mileage and value from the strategy doc.

Comment thread plugins/compound-engineering/skills/ce-product-pulse/SKILL.md Outdated
imwm added a commit that referenced this pull request Apr 30, 2026
Merged origin/main, picking up the ce:* to ce- rename across the
plugin plus the README/workflow restructure.

Conflict resolution (README.md, plugins/compound-engineering/README.md):
- Adopted main's prose-first workflow structure
- Inserted strategy and product-pulse rows
- Applied reviewer-suggested intro rewrite to plugin README
- Updated skill count to 37

Review feedback addressed (PR #614, @tmchow):
- Swept all ce:* references in new skills to ce- dash form
- Stripped leading / from cross-skill references for Codex compatibility
- Moved pulse reports from ~/pulse-reports/ to docs/pulse-reports/ per reviewer
- Added allowed-tools frontmatter to ce-product-pulse to trim context
- Reframed "read-only" prose to be precise (skill writes own config and reports)
- Made scheduling handoff cross-harness aware (in-plugin schedule skill optional)
@imwm
Copy link
Copy Markdown
Collaborator Author

imwm commented Apr 30, 2026

Re: the ce-strategy-alignment-reviewer agent suggestion. Strong idea, and keeping it scoped out of this PR so we do not expand merging-blocker surface; deferring to you and Kieran on whether and where to slot it in next. Natural plug-in points to consider: ce-plan (flag plan decisions that pull away from active tracks) and ce-code-review (catch implementation drift from strategy). Once we have a strategy doc accumulating real edits, the alignment review becomes the mechanism that keeps it load-bearing instead of decorative.

@imwm imwm marked this pull request as ready for review April 30, 2026 07:18
@imwm imwm requested a review from tmchow April 30, 2026 07:18
imwm added a commit that referenced this pull request Apr 30, 2026
Picks up the ce:* to ce- rename across the plugin plus the README/
workflow restructure from main.

Conflict resolution (README.md, plugins/compound-engineering/README.md):
- Adopted main's prose-first workflow structure
- Inserted strategy and product-pulse rows
- Applied reviewer-suggested intro rewrite to plugin README
- Updated skill count to 37

Review feedback addressed (PR #614, @tmchow):
- Swept all ce:* references in new skills to ce- dash form
- Stripped leading / from cross-skill references for Codex compatibility
- Moved pulse reports from ~/pulse-reports/ to docs/pulse-reports/ per reviewer
- Added allowed-tools frontmatter to ce-product-pulse to trim context
- Reframed read-only prose to be precise (skill writes own config and reports)
- Made scheduling handoff cross-harness aware (in-plugin schedule skill optional)

# Conflicts:
#	README.md
#	plugins/compound-engineering/README.md
@imwm imwm force-pushed the marcus/pm-skills branch from 4776e1c to 9d4ecf1 Compare April 30, 2026 07:25
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4776e1c3f2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

- `7d`, `30d` - trailing days
- `1h` - short-window (useful during launches)

If the argument is empty, default to `24h`. If it is unparseable, ask the user to clarify.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use configured default lookback when argument is empty

The skill asks for a default lookback during setup and stores it in docs/product-pulse.md, but this rule hardcodes 24h whenever no argument is passed. In normal recurring use (e.g., user configured 7d and runs /ce-product-pulse with no args), the report will query the wrong window and silently ignore saved configuration. After Phase 2 reads docs/product-pulse.md, the no-argument path should use the configured default instead of always forcing 24h.

Useful? React with 👍 / 👎.

Comment on lines +50 to +56
Run the interview in the section order of the final document:

1. Target problem
2. Our approach
3. Who it's for
4. Key metrics
5. Tracks
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Capture product name before drafting strategy document

The first-run interview flow never collects a product name, but references/strategy-template.md requires {{product_name}} for the document title. On repos where the product name is not obvious from prior context, this forces the agent to guess or leave placeholders, reducing the accuracy of the core artifact generated by the skill. Add an explicit product-name prompt in the first-run interview before filling the template.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0ef57af6bf

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +249 to +250
- **Strategy metrics carried forward**: surfaced in the report, not stored as config — they live in `docs/strategy.md` and are re-read each run from there.
- **Per-source connection details** (URLs, API keys, query specifics): live with the user's MCP configuration, not in this config.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Persist metric-to-source mapping for recurring pulse runs

The interview flow gathers source/query mapping for each strategy metric, but this section explicitly says strategy metrics are not persisted in config. That breaks recurring accuracy: later runs are still expected to render each strategy metric with a current value and delta, yet after the initial session there is no durable mapping for where/how to query those metrics, so the agent must guess or silently degrade to no data. Persist at least metric names plus canonical source/query hints so subsequent pulse reports are reproducible.

Useful? React with 👍 / 👎.

First-run setup already offered scheduling (see Phase 1.1 end). Phase 3 is a lighter re-surface for ad-hoc runs:

- If the argument was a known schedule keyword (`daily`, `hourly`, `weekly`), note that this run is ad-hoc and suggest scheduling via the harness's available primitive (the in-plugin `schedule` skill where present; otherwise a platform-native option) for recurring runs.
- If no schedule is on file and this is the third or later pulse run the user has done, mention once that scheduling is available. Don't nag on every run.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist schedule state before using third-run reminder gating

Phase 3 gates scheduling reminders on whether a schedule is already on file and whether this is the third-or-later run, but the skill does not persist schedule state or run count in its config contract. In practice this condition cannot be evaluated reliably across invocations, so the reminder will be inconsistent (either repeated too often or skipped unexpectedly). Add a durable schedule/run-state key or remove the conditional gate.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31868b8822

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


### Phase 2: Run the Pulse

Use the `pulse_*` settings already extracted from `.compound-engineering/config.local.yaml` in Phase 0. Apply hard defaults for any unset settings (see Phase 0 "Config keys").
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reload config before first-run pulse execution

The first-run path goes Phase 0 -> Phase 1 -> Phase 2, but Phase 2 is instructed to use settings "already extracted" in Phase 0. On first run, Phase 0 had no pulse_* settings yet, so Phase 2 can run with missing/default values even after Phase 1 writes the new config. This can make the very first generated pulse query the wrong window/sources or omit configured signals unless the model happens to re-read manually.

Useful? React with 👍 / 👎.


#### 2.4 Write the Report

Save to `docs/pulse-reports/YYYY-MM-DD_HH-MM.md` using the local time of the run. Create `docs/pulse-reports/` if it does not exist.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid minute-only filenames for pulse report writes

Report filenames are only precise to the minute (YYYY-MM-DD_HH-MM.md), so two runs in the same minute overwrite each other. This is especially likely right after setup or when rerunning to validate queries, and it silently loses prior pulse history in the directory that is meant to be a timeline.

Useful? React with 👍 / 👎.


After each unique source is named, check MCP coverage:

1. Call `search_mcp_registry` with the tool name to see if an official or community MCP exists. Do not guess from memory.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not require MCP registry tools outside allowed-tools

The interview requires calling search_mcp_registry (and later offers suggest_connectors), but this skill's frontmatter only allows Read, Write, Glob, Grep, Bash, and AskUserQuestion. In restricted runs, that MCP-discovery step cannot execute as written, so source-setup guidance degrades or stalls unless a fallback path is added.

Useful? React with 👍 / 👎.

imwm added a commit that referenced this pull request Apr 30, 2026
Adds pulse_metric_sources CSV key (metric=source pairs) so multi-source
setups stay reproducible across runs. Without it, the agent gathered
the per-metric source during the interview but had no durable record;
subsequent runs had to guess or silently degrade to no data.

Single-source setups (one analytics tool, one tracing tool) keep
working unchanged: metrics not listed in pulse_metric_sources fall back
to pulse_analytics_source and render with a (default source) marker.

Multi-source setups (e.g., retention in PostHog, NPS in Delighted) now
persist the override in pulse_metric_sources during the interview and
the report-template rendering rule honors it.

Resolves chatgpt-codex-connector P1 review on PR #614.

Updates:
- config-template.yaml + .compound-engineering/config.local.example.yaml
  add the new key
- SKILL.md Phase 0 Config keys list documents the override semantics
- references/interview.md section 6.0 instructs the agent to populate
  the mapping for any strategy metric whose source differs from the
  class default
- references/interview.md Config File Shape extends the schema
- references/report-template.md strategy-metric rendering rule looks up
  pulse_metric_sources first, falls back to pulse_analytics_source with
  a visible marker
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f3fa2e949a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


## Interaction Method

Use the platform's blocking question tool when available (`AskUserQuestion` in Claude Code, `request_user_input` in Codex, `ask_user` in Gemini). Otherwise, present numbered options in chat and wait for the user's reply before proceeding.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Load AskUserQuestion via ToolSearch before fallback path

The interaction block skips the deferred-tool load step, so in Claude Code sessions where AskUserQuestion is not yet loaded the skill will immediately degrade to chat-numbered prompts instead of using the blocking question UI. plugins/compound-engineering/AGENTS.md ("Cross-Platform User Interaction") explicitly requires trying ToolSearch with select:AskUserQuestion before falling back, because schema-not-loaded is not a valid fallback trigger. Without that step, first-run interviews can run in the wrong interaction mode even when the tool is available.

Useful? React with 👍 / 👎.


If no tracing tool was named in section 6, ask: "What tool do you use for application tracing and errors? (e.g., Datadog, Sentry, Honeycomb, New Relic.) Skip if you don't have one - I'll omit the system performance section from the report." Skipping is fine - the pulse will just report usage and followups in that case.

**Capture:** tool name (or "none"), top-error count (default 5), latency opt (default on).
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist system-performance preferences gathered in setup

This capture step records a customizable top-error count and latency toggle, but those settings are never represented in the documented pulse_* config keys for recurring runs, so user choices are lost after setup. The same commit’s report instructions later expect a "configured count," which becomes unreproducible across runs; users who chose "top 3" or "no latency" will silently drift back to default output. Either persist these fields in config and read them in Phase 2, or remove the customization branch.

Useful? React with 👍 / 👎.

imwm added a commit that referenced this pull request Apr 30, 2026
…tion and reload config in Phase 2

Two prose corrections from automated review on PR #614.

ToolSearch step in Interaction Method (both skills):
- Previous prose said 'use the platform's blocking question tool when
  available, otherwise present numbered options' — which omits the
  ToolSearch with select:AskUserQuestion step that AGENTS.md explicitly
  requires for Claude Code when the schema is not yet loaded.
- Without that step, Claude Code sessions where AskUserQuestion is
  deferred would incorrectly fall back to chat-numbered prompts.
- Replaced both Interaction Method blocks with the canonical AGENTS.md
  prose, including the ToolSearch instruction and explicit guidance
  that schema-not-loaded is not a valid fallback trigger.

Phase 2 config reload (ce-product-pulse):
- Phase 2 previously instructed agents to use settings 'already
  extracted from config.local.yaml in Phase 0', but on first run
  Phase 0 had no pulse_* settings yet (config did not exist or had
  no pulse_* keys). Phase 1 then writes the config and accepts an
  edit round.
- Rewrote Phase 2 opening: if Phase 1 ran (first run or setup arg),
  re-read config.local.yaml to pick up any edits; otherwise use the
  values extracted in Phase 0 unchanged.
… and outcome pulse

Adds two new skills to the compound-engineering plugin: one upstream of the
core engineering loop (strategy), one downstream (pulse). Both follow the
existing skill conventions and integrate with ce-ideate, ce-brainstorm,
and ce-plan.

ce-strategy
- Interview-driven creator and maintainer of docs/strategy.md.
- Captures a Rumeltian problem statement, approach, persona(s), key
  metrics, and 2-4 work tracks.
- Re-runnable: subsequent runs target a single section without
  clobbering the rest.
- Anti-pattern pushback codified in references/interview.md so the agent
  challenges weak strategy answers (vanity metrics, fluff approaches,
  feature lists in place of guiding choices).

ce-product-pulse
- Time-windowed pulse report on usage, performance, errors, and
  followups. Saves to docs/pulse-reports/ as a browseable timeline.
- First-time setup interviews the user to define metrics and wire data
  sources. MCP-preferred. Read-only DB access only -- write credentials
  are explicitly refused.
- Configuration lives as flat pulse_* keys in the unified
  .compound-engineering/config.local.yaml alongside work_delegate_*,
  matching the existing convention for plugin-local settings. Schema
  defined in ce-setup's config-template.yaml and the rendered example.
- pulse_metric_sources gives per-strategy-metric source overrides for
  multi-source setups; metrics not listed fall back to
  pulse_analytics_source with a (default source) marker.
- pulse_pending_metrics and pulse_excluded_metrics give explicit
  dispositions for un-instrumented strategy metrics.
- Phase 0 uses the worktree-aware bash pre-resolution pattern
  (matches ce-work-beta) to read config at skill load time. Phase 2
  re-reads config after first-run interviews to pick up edits.

Integration touches
- ce-brainstorm Constraint Check, ce-ideate quick-context sub-agent,
  and ce-plan planning context summary all read docs/strategy.md as
  grounding when present.
- Cross-platform Interaction Method blocks in both new skills follow
  the AGENTS.md convention: ToolSearch with select:AskUserQuestion is
  tried first in Claude Code before falling back to numbered chat
  options. Schema-not-loaded is not a valid fallback trigger.

README updates
- Workflow diagram and Core Workflow tables surface both new skills
  with the upstream/downstream framing.
- Skill count 36 -> 37.

Tested locally with /ce-strategy and /ce-product-pulse first-run and
subsequent-run flows. release:validate passes; frontmatter tests pass.
@imwm imwm force-pushed the marcus/pm-skills branch from af1c686 to 8c3df11 Compare April 30, 2026 23:37
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8c3df11b4c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Run these in **parallel** (different tools, no shared load):

- Product analytics query (primary event count, value-realization count, completions, conversion ratios) over the window
- Application tracing query (error counts by category, latency distribution, top error signatures) over the window
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Make tracing query conditional on tracing source

Phase 1 explicitly allows users to skip tracing, but Phase 2.1 always dispatches an application tracing query. In setups without a tracing tool configured, this creates an impossible step and can block or degrade the pulse run before report assembly. Guard this query behind a configured tracing source (similar to the payments conditional) so "no tracing" flows still complete.

Useful? React with 👍 / 👎.


Run these **serially**, after the parallel batch:

- Read-only database queries. One at a time. Tight, scoped queries only. Never full-table scans on large tables. If a DB query would be expensive, skip it and note "DB query skipped (estimated cost too high)".
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip DB query stage when database is not enabled

The interview permits pulse_db_enabled: false, but Phase 2.1 still instructs serial database queries unconditionally. On configurations that intentionally omitted DB access, this step causes unnecessary failures or wasted retries against a non-existent connection. The DB query stage should run only when DB usage was explicitly enabled.

Useful? React with 👍 / 👎.

@tmchow tmchow merged commit cb8f9b3 into main May 1, 2026
2 checks passed
@github-actions github-actions Bot mentioned this pull request Apr 30, 2026
@madebymlai
Copy link
Copy Markdown

i dont get what this strategy thing even does...

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.

3 participants