feat: add CDM v10dev support and aggregation override

[k-rister]

Summary

• Add local-v10 OpenSearch instance for CDM v10dev, set as default index-to target while keeping v8dev and v9dev in query-from
• Add bash tab completions for the new --aggregation <sum|avg|max|min> flag on crucible get metric, which overrides a metric's default-aggregation at query time
• Document v10dev, the default-aggregation field, the four aggregation types (sum/avg/max/min), and the --aggregation query-time override in docs/how-cdm-works.md
• Add pr-review to the crucible-dev-tools skills list in CLAUDE.md

Related PRs:

• CDM PR#202 (merged) — core v10dev implementation
• rickshaw PR#859 (merged) — v10dev support in rickshaw-gen-docs

Test plan

• Verify crucible get metric --aggregation <TAB> completes with sum avg max min
• Verify crucible get metric --<TAB> includes --aggregation in the flag list
• Verify CDM server starts successfully with v10dev configured as index-to
• Run a benchmark and confirm results index into v10dev indices
• Query with --aggregation override and confirm different aggregation behavior

🤖 Generated with Claude Code

[k-rister]

PR Review: crucible#645 — feat: add CDM v10dev support and aggregation override

Summary: Adds a v10dev OpenSearch instance as the new index-to target, bash completions for --aggregation on get metric, and documents v10dev/default-aggregation in how-cdm-works.md.
Changed files: 4
Review dimensions: Correctness, API & Contracts, Build & Deploy, Documentation, Style

Issues

• [bin/_crucible_completions:434] opensearch add --cdmver completions missing v10dev — The --cdmver value completion for opensearch add (line 434) and opensearch update (line 459) lists v7dev v8dev v9dev but not v10dev. This same file adds a v10dev instance in the services.json change, so the completions are now inconsistent. A user doing crucible opensearch add --cdmver <TAB> won't see v10dev as an option. They can type it manually, but the completions should offer it.

Style

• [config/services.json:40-44] query-from ordering breaks newest-first convention — The existing list had ["local-v9", "local-v8"] (newest first). The PR appends local-v10 at the end: ["local-v9", "local-v8", "local-v10"]. If ordering is cosmetic this is minor, but the convention was newest-first.

File Coverage

• CLAUDE.md — No issues found
• bin/_crucible_completions — 1 issue (missing v10dev in --cdmver completions)
• config/services.json — 1 style (query-from ordering)
• docs/how-cdm-works.md — No issues found

Limitations

• Cannot verify that the CDM server correctly handles v10dev indices at runtime (requires a running OpenSearch instance)
• Cannot verify that --aggregation flag is properly parsed by the get metric command (implementation is in the CDM subproject, merged in PR#202)
• Cannot verify bash completion behavior in a live shell

Verdict

Approve with comments — The PR is functional and safe to merge. The missing v10dev in the opensearch --cdmver completions is the only substantive finding — it's a consistency gap in the same file being modified, but doesn't block anything.

[k-rister]

Review response

Both findings addressed:

1. --cdmver completions missing v10dev — Fixed. Added v10dev to the value completions for both opensearch add and opensearch update subcommands.

2. query-from ordering — Fixed. Reordered to ["local-v10", "local-v9", "local-v8"] to follow the newest-first convention.

Both fixes squashed into their respective original commits.

[k-rister]

PR Review: crucible#645 — feat: add CDM v10dev support and aggregation override

Summary: Adds a v10dev OpenSearch instance as the default index target, adds --aggregation bash completions for get metric, and documents v10dev / default-aggregation in the CDM guide.
Changed files: 4
Review dimensions: Correctness, API & Contracts, Build & Deploy, Documentation, Style

Documentation

• [bin/_help:29-39] --aggregation not listed in get metric optional arguments. The help text for get metric documents optional flags like --breakout and --filter, but does not mention the new --aggregation flag. Per the codebase convention ("update all three locations: bin/_help, bin/_crucible_completions, and the relevant command's own help text"), this should be added alongside the existing optional argument documentation. The completions and CDM-side help are already updated — only _help is missing.

• [docs/how-services-work.md:223-233] Services guide example still shows v9dev as the sole instance. The how-services-work.md opensearch config example shows only local-v9 with "index-to": "local-v9". Now that v10dev is the default index target in the shipped services.json, the example is no longer representative of what a fresh install looks like. Minor — the example illustrates structure, not policy — but it could mislead someone comparing their config to the docs.

• [schema/services.json:213] Schema description says 'v8dev', 'v9dev' — doesn't mention v10dev. Very minor, but the description string "(e.g., 'v8dev', 'v9dev')" could include v10dev for completeness.

File Coverage

• CLAUDE.md — No issues found
• bin/_crucible_completions — No issues found (completions correctly wired: --aggregation value completion with sum avg max min, flag added to _crucible_filter_used_flags, v10dev added to both add and update cdmver lists)
• config/services.json — No issues found (valid structure, matches schema, index-to/query-from correctly configured)
• docs/how-cdm-works.md — No issues found (accurate documentation of default-aggregation, latency class, v10dev versioning, and --aggregation override; version table v9dev description corrected from "additional document types" to "metric_def document type")

Limitations

• Cannot verify runtime behavior of v10dev indexing or --aggregation query-time override (requires running OpenSearch + CDM server)
• Cannot verify that the latency class value is accepted by CDM's metric_desc validation (implemented in CDM PR#202, not in this diff)

Verdict

Approve with comments — All code changes are correct and well-structured. The only findings are documentation gaps: bin/_help should mention --aggregation for consistency with how other optional get metric flags are documented, and two minor doc references could be updated. None are blocking.

[k-rister]

Review response

All three documentation findings addressed:

1. bin/_help missing --aggregation — Fixed. Added --aggregation <sum|avg|max|min> with description to the get metric optional arguments section, alongside --breakout and --filter.

2. docs/how-services-work.md example shows v9dev — Fixed. Updated the opensearch config example to show local-v10 with v10dev as the instance, matching the current default in services.json.

3. schema/services.json description missing v10dev — Fixed. Updated the cdmver description string to include v10dev in the examples list.

Each fix squashed into its respective original commit.

[atheurer]

Excellent, fully backwards-compatible schema migration. Appreciate adding the co-changes (help text, JSON schema, and guide docs) in subsequent commits. Approve!