Add workflow discovery and CLI command (v0.3)#30
Conversation
…w candidates Extends the composability analysis with positive interaction detection (synergy scores), ordered skill sequence extraction from usage timestamps, and automatic workflow candidate flagging. Backwards compatible — v1 function and tests unchanged, CLI falls back to v1 when no usage log exists. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implements multi-skill workflow support: discovers workflow patterns from existing telemetry, displays them via `selftune workflows`, and codifies them to SKILL.md via `selftune workflows save`. Includes 48 tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR adds composability v2 analysis and end-to-end workflow discovery: new pure functions analyzeComposabilityV2 and discoverWorkflows, SKILL.md workflow parsing/writing utilities, CLI wiring (new workflows command and composability v2 fallback), types/docation, and comprehensive tests. V2 uses usage logs to detect synergy, ordered sequences, and workflow candidates. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI (selftune workflows)"
participant FS as "FS (telemetry / usage / SKILL.md)"
participant Engine as "discoverWorkflows / analyzeComposabilityV2"
participant Formatter as "formatWorkflows / skill-md-writer"
CLI->>FS: read telemetry.jsonl + skill_usage_log.jsonl
CLI->>Engine: invoke discoverWorkflows(telemetry, usage, opts)
Engine-->>CLI: WorkflowDiscoveryReport / ComposabilityReportV2
CLI->>Formatter: format JSON or human report
alt save subcommand
CLI->>Formatter: appendWorkflow(SKILL.md, CodifiedWorkflow)
Formatter-->>FS: write SKILL.md
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e965a4078d
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/selftune/eval/composability-v2.ts`:
- Around line 21-26: Extract the duplicate clamp implementation into a shared
helper module (e.g., create a new exported function clamp in
cli/selftune/utils/math.ts) and replace the local clamp definitions in
composability-v2.ts and discover.ts with imports from that new module; ensure
the exported signature matches function clamp(value: number, min: number, max:
number): number and update both files to import { clamp } from the shared utils,
removing the duplicate implementations.
In `@cli/selftune/index.ts`:
- Around line 209-213: The untyped result from readJsonl(logPath) forces later
type assertions; call readJsonl with an explicit type parameter (e.g., create or
reuse an interface like TelemetryEntry/TelemetryRecord and pass it to readJsonl)
so telemetry is strongly typed, then remove the manual type assertions where
telemetry elements are accessed (the current casts after assignment). Update the
telemetry declaration to use readJsonl<YourTelemetryType>(logPath) and ensure
any downstream code that relied on assertions compiles with the new type.
In `@cli/selftune/workflows/discover.ts`:
- Around line 240-244: The callback passed to workflows.filter uses a non-null
assertion options.skill! which violates the Biome lint rule; instead, before the
filter call capture the skill into a local const (e.g., const skill =
options?.skill) after checking options?.skill and then use that const inside the
predicate (e.g., w.skills.includes(skill)) so the nullability is resolved
without using the non-null assertion; update the block that assigns filtered
(and any references to options.skill in that scope) to use the local const.
In `@cli/selftune/workflows/skill-md-writer.ts`:
- Around line 42-57: The inline complex type for the variable current is hurting
readability; replace the anonymous inline type with a named local interface or
type (e.g., WorkflowBuilder) or reuse CodifiedWorkflow with a builder-style
narrower type, then update the declaration of current and its initializations to
use that named type (refer to current, WorkflowBuilder, and CodifiedWorkflow in
your changes) so the parsing loop and object construction remain identical but
the type is clearer.
- Around line 113-117: The writer outputs raw synergy_score which can have many
decimals; update the block that builds the source line (the code that reads
workflow.discovered_from and pushes `- **Source:** Discovered from
${occurrence_count} sessions (synergy: ${synergy_score})`) to format
synergy_score to two decimal places (e.g., parse/convert the value and use
toFixed(2) or Intl.NumberFormat) so the output matches the expected `(synergy:
X.XX)` pattern used by the parser.
- Around line 85-89: The if-branch that sets current.source = "authored"
contains an unnecessary continue which should be removed; locate the loop
handling `current` in skill-md-writer.ts (the if-block that assigns
`current.source = "authored"`) and delete the trailing `continue` so control
naturally proceeds to the next iteration without redundant flow control.
In `@cli/selftune/workflows/workflows.ts`:
- Around line 77-80: The parsed integers for minOccurrences and window (produced
via Number.parseInt in workflows.ts) must be validated against NaN and negative
values before being passed to the discovery engine: after parsing
values["min-occurrences"] and values.window, check Number.isFinite(parsed) (or
!Number.isNaN(parsed)) and ensure parsed >= 0; if invalid, either set the
variable to undefined or throw a descriptive CLI error so the discovery engine
doesn't get NaN/negative inputs; update the handling around the minOccurrences
and window variables to perform these checks and produce a clear error message
or fallback to undefined.
In `@docs/design-docs/composability-v2.md`:
- Around line 139-150: Update the fenced code block that documents the algorithm
by adding a language specifier so it reads ```text instead of ```; specifically
modify the algorithm block that starts with "Algorithm:" in composability-v2.md
so the opening fence includes "text" to ensure the block is treated as plain
text.
- Around line 117-126: The fenced code block containing the pseudo-code for
computing synergy_score (symbols: synergy_score, clamp, avg_errors_alone,
avg_errors_together) is missing a language specifier; update the opening fence
to include a plain-text language such as ```text or ```plaintext so static
analyzers and renderers treat this as non-executable pseudocode (e.g., replace
the leading ``` with ```text).
- Around line 227-240: The markdown ordered list numbering is inconsistent
(starts at 1 then jumps to 3/4/5); update the list so numbering is
consistent—either make every item use "1." (letting the renderer auto-number) or
renumber sequentially starting at "1." for the entries referencing the
--min-occurrences flag, the Synergy score table, the "Detected Sequences"
section, the "Workflow Candidates" section, and the link to the selftune
workflows command; ensure the same numbering style is applied to the entire list
block so linters no longer flag it.
In `@docs/design-docs/workflow-support.md`:
- Around line 40-49: The fenced code block(s) under "Discovery Pipeline" (e.g.,
the block containing "Discovery Pipeline:" and "skill_usage_log.jsonl") and the
other unlabeled blocks should include language specifiers for proper syntax
highlighting; update those fences to use `text` for plain prose blocks, `bash`
for CLI snippets, and `typescript` for type definitions as appropriate so each
triple-backtick fence is like ```text, ```bash, or ```typescript around the
corresponding block.
In `@tests/eval/composability-v2.test.ts`:
- Around line 75-82: Replace the non-null assertions used after
expect(...).toBeDefined() by adding and using a type guard helper (e.g.
assertDefined<T>(value): asserts value is T) and call it right after locating
the value (the variable pair from report.pairs.find(...)), so the compiler knows
pair is defined and you can access pair.synergy_score, pair.avg_errors_together
and pair.avg_errors_alone without using `!`; apply the same pattern to the other
find calls in this test file to remove all non-null assertions.
In `@tests/workflows/discover.test.ts`:
- Around line 262-272: The test uses non-null assertions on seqAB and seqBA;
replace them with explicit type guards: assert presence with
expect(seqAB).toBeDefined() / expect(seqBA).toBeDefined() (or early
returns/throws if undefined) then use the variables without '!'; alternatively
use if (!seqAB) fail('seqAB missing') (and same for seqBA) before asserting
occurrence_count and sequence_consistency so the runtime type is guaranteed and
the non-null assertions are removed from seqAB and seqBA.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bb334068-74c1-4ab2-8fef-f08d949d5bbd
📒 Files selected for processing (12)
cli/selftune/eval/composability-v2.tscli/selftune/index.tscli/selftune/types.tscli/selftune/workflows/discover.tscli/selftune/workflows/skill-md-writer.tscli/selftune/workflows/workflows.tsdocs/design-docs/composability-v2.mddocs/design-docs/workflow-support.mdtests/eval/composability-v2.test.tstests/workflows/discover.test.tstests/workflows/skill-md-writer.test.tstests/workflows/workflows.test.ts
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/selftune/eval/composability-v2.ts`:
- Around line 179-187: sequenceCounts currently stores only the first-seen
seq.firstQuery, making representative_query order-dependent; change the value
stored in sequenceCounts (used around sequenceCounts, sessionSequences, and when
constructing SkillSequence) to include a per-query frequency map (e.g., queries:
Map<string, number>) and increment the count for seq.firstQuery on each
occurrence, then when you build each SkillSequence choose the query with the
highest frequency as the representative_query; apply the same change to the
second occurrence referenced (the block at the later use around lines 228-233)
so both places pick the most-frequent initiating query instead of the first-seen
one.
- Around line 92-101: The co-skill baseline currently uses coSkillAloneSessions
= sessions.filter(...) which selects any session that contains coSkill but not
skillName, allowing multi-skill sessions; change the filter used to compute
coSkillAloneSessions so it only selects true solo sessions where
skills_triggered contains coSkill and no other skills (e.g.,
skills_triggered.length === 1 or equivalent check), then recompute
errorsCoSkillAlone and avgErrorsAlone the same way (falling back to errorsAlone
when no true-solo coSkill sessions exist).
In `@cli/selftune/index.ts`:
- Around line 210-213: The parsed values for windowSize and minOccurrences
(variables windowSize and minOccurrences) are not validated before being passed
to analyzeComposabilityV2; update the parsing logic to check Number.parseInt
results for NaN and ensure they are non-negative (and >0 if required by
analyzeComposabilityV2), and if invalid, return or throw a clear error (or print
a user-facing message and exit) rather than passing undefined/NaN through;
mirror the validation pattern used in workflows.ts (lines that validate parsed
integers) and apply it where the values are constructed so
analyzeComposabilityV2 always receives validated integers.
In `@cli/selftune/workflows/discover.ts`:
- Around line 176-184: Explain in a brief inline comment above the
avgErrorsIndividual and synergyScore calculation that this workflow
intentionally uses the max of per-skill solo error rates (computed via
getSkillSoloErrorRate and stored in avgErrorsIndividual) as the denominator for
the synergy formula, unlike composability-v2 which uses the target skill's solo
rate; note why this choice (multi-skill conservative normalization) was made and
reference the functions/variables synergyScore and clamp so future readers
understand the divergence and rationale.
- Around line 150-154: getSkillSoloErrorRate currently returns 0 for missing
data which conflates "no data" with a zero error rate; change
getSkillSoloErrorRate(skillName) to return undefined (or NaN) when
skillSoloErrors.get(skillName) is missing or count === 0, and then update the
code that computes avgErrorsIndividual and the synergy score to explicitly
handle undefined (e.g., skip skills with undefined solo rate when averaging or
bail out/assign a neutral synergy score when any required solo data is missing)
so the synergy calculation does not produce negative/invalid scores; look for
references to getSkillSoloErrorRate, skillSoloErrors, avgErrorsIndividual, and
the synergy formula to make the coordinated changes.
In `@cli/selftune/workflows/skill-md-writer.ts`:
- Around line 79-90: The regex in the discovered-source parsing only accepts
positive synergy numbers, so negative synergy like "-0.45" gets ignored; update
the pattern used in the discoveredMatch call (the /^Discovered from (\d+)
sessions? \(synergy: ([\d.]+)\)$/ regex) to accept a leading negative sign and
proper decimals (e.g. /^Discovered from (\d+) sessions? \(synergy:
(-?\d+(?:\.\d+)?)\)$/) so discoveredMatch correctly captures negative synergy
and parseFloat(synergy) on current.discovered_from.synergy_score continues to
work.
In `@docs/design-docs/composability-v2.md`:
- Around line 130-133: Add a blank line immediately before the fenced code block
that contains the Typescript example so the block is separated from the
preceding sentence (Markdown lint MD031). Locate the fenced block that wraps the
line "conflict_detected = synergy_score < -0.3" and insert a single empty line
just above the opening ```typescript fence so the code block is properly
delimited.
- Around line 152-172: The markdown example has the JSON fenced code block
immediately after "Example output:" without a separating blank line; add a
single blank line between the "Example output:" line and the ```json fenced
block so the code block renders correctly—update the section containing the
"Example output:" heading and the following JSON example to insert one empty
line before the ```json block.
In `@docs/design-docs/workflow-support.md`:
- Around line 187-203: The markdown fenced code blocks (the "Output:" block
showing "Discovered Workflows..." and the other two fenced blocks referenced
around lines 282 and 298) are not surrounded by blank lines, triggering MD031;
fix by inserting a blank line immediately before each opening ``` and a blank
line immediately after each closing ```, ensuring the fenced blocks (e.g., the
"Output:" block) have one empty line separating them from surrounding paragraphs
so markdownlint passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e99a8106-504b-414c-a07c-00ddba5c3206
📒 Files selected for processing (15)
biome.jsoncli/selftune/eval/composability-v2.tscli/selftune/index.tscli/selftune/types.tscli/selftune/utils/math.tscli/selftune/workflows/discover.tscli/selftune/workflows/skill-md-writer.tscli/selftune/workflows/workflows.tsdocs/design-docs/composability-v2.mddocs/design-docs/workflow-support.mdpackage.jsontests/eval/composability-v2.test.tstests/workflows/discover.test.tstests/workflows/skill-md-writer.test.tstests/workflows/workflows.test.ts
Implements multi-skill workflow discovery across all skills in telemetry. Discovers patterns automatically, displays via
selftune workflows, and codifies viaselftune workflows save. Includes workflow type system, discovery engine with synergy/consistency scoring, SKILL.md parser/writer, CLI formatter, and 48 comprehensive tests.Parallel agents implemented types+discovery, SKILL.md writer, and CLI registration. Full test suite passes (1098 tests). Supports --min-occurrences, --window, --skill filters and JSON output.
Generated with Claude Code