Skip to content

feat: add native duration-weighted distribution statistics - #213

Merged
k-rister merged 2 commits into
masterfrom
feat/native-distribution-stats
Sep 8, 2026
Merged

k-rister merged 2 commits into
masterfrom
feat/native-distribution-stats

Conversation

@k-rister

@k-rister k-rister commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Implement the revised direction for issue #205: calculate distribution statistics over the native reconstructed metric timeline using duration weighting, independent of the requested output resolution.

Included

  • Native timeline reconstruction for sum, average, minimum, and maximum aggregations
  • Duration-weighted min, max, mean, median, population standard deviation, and nearest-rank percentiles
  • OpenSearch PIT plus search_after pagination
  • Configurable document, interval, runtime, and page-size limits
  • REST API and CLI support
  • Native statistics unit, integration, and live REST coverage
  • Documentation updates

Validation

  • Unit tests pass
  • PIT/OpenSearch integration tests pass
  • Live REST tests pass
  • Web UI production build passes
  • Syntax, formatting, and diff checks pass

Closes #205

Issue #212 tracks the separate pre-existing problem with the default npm test script.

Crucible follow-up: #671 adds --distribution-stats to get metric shell completion.

— AI-signed: Codex | model: GPT-5 | effort: not specified

Implement duration-weighted distribution statistics over the native reconstructed metric timeline. Add PIT/search_after retrieval with resource limits, API and CLI support, native aggregation and percentile calculations, documentation, and unit, integration, and live REST coverage.

AI-Tool: Codex
AI-Model: GPT-5
AI-Effort: not specified
@k-rister k-rister self-assigned this Sep 4, 2026
@k-rister
k-rister requested a review from a team September 4, 2026 21:39
@project-crucible-tracking project-crucible-tracking Bot moved this to In Progress in Crucible Tracking Sep 4, 2026
@k-rister

k-rister commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Summary

This pull request implements native duration-weighted distribution statistics for CommonDataModel metric queries (addressing #205). The implementation reconstructs a piecewise-constant native timeline across selected metric streams, computes duration-weighted statistics (min, max, mean, median, stddev, pNN), and resamples the timeline into resolution windows while preserving resolution-invariant statistics. The implementation is robust, mathematically sound, well-tested, and includes appropriate guardrails and documentation.


Critical Issues

None.


Issues

  1. Error Code Propagation for Data-Quality Errors (NATIVE_STATS_DATA_QUALITY vs 500 INTERNAL_ERROR):
    In queries/cdmq/native-stats.js (lines 5–9), fail(message, code) only sets error.code when code is explicitly provided (such as 'NATIVE_STATS_LIMIT'). When timeline errors occur during validation (e.g., gaps, overlaps, reversed intervals, or missing coverage in normalizeDocuments and reconstructTimeline), error.code is undefined. Consequently, server.js (lines 1839–1855) handles the error as an unexpected failure, responding with HTTP 500 (INTERNAL_ERROR) instead of HTTP 422 (NATIVE_STATS_DATA_QUALITY).
    Recommendation: Default fail(message, code = 'NATIVE_STATS_DATA_QUALITY') in native-stats.js so all timeline data defects map cleanly to HTTP 422.

  2. Top-Level error String on HTTP 422 Responses:
    In server.js (lines 1849–1853), the error message whitelist is ['NATIVE_STATS_CONFIG', 'NATIVE_STATS_LIMIT', 'NATIVE_STATS_PIT_UNSUPPORTED']. Because NATIVE_STATS_DATA_QUALITY is omitted, an HTTP 422 response returns "error": "Internal server error while fetching metric data" with the specific cause only in details.
    Recommendation: Add 'NATIVE_STATS_DATA_QUALITY' to the whitelist in server.js (line 1850) so the top-level error string contains the relevant timeline error description.


Minor Suggestions

  1. Early Pagination Exit Optimization:
    In queries/cdmq/cdm.js (getNativeMetricStats, lines 4030–4034), the loop continues until OpenSearch returns hits.length === 0. If hits.length < pageSize on a non-empty page, all matching documents have already been retrieved; checking if (hits.length < pageSize) break; can save one HTTP round-trip on the final page.

  2. Crucible Tab Completion Follow-up:
    In the parent crucible repo, bin/_crucible_completions defines _complete_get_metric for CLI flag completions. A follow-up PR in crucible should add --distribution-stats to the completion list for crucible get metric.


Positive Findings

  • Resolution Invariance & Mathematical Fidelity: Statistics are computed over the reconstructed native timeline rather than output resolution buckets, ensuring identical results regardless of --resolution.
  • Accurate Statistical Algorithms: Population standard deviation is calculated using weighted Welford accumulation, and percentiles accurately use duration-weighted nearest-rank indexing.
  • Robust OpenSearch PIT Management: Point-in-Time sessions are reliably closed in finally blocks, _source: false with docvalue_fields minimizes payload overhead, and a 3-field sort (begin, end, metric_desc-uuid) ensures deterministic search_after pagination.
  • Strong Operational Guardrails: Enforces configurable limits on document count (CDM_NATIVE_STATS_MAX_DOCUMENTS), interval count (CDM_NATIVE_STATS_MAX_INTERVALS), and execution runtime (CDM_NATIVE_STATS_MAX_RUNTIME_MS), rejecting oversized queries on the first page via track_total_hits.
  • Comprehensive Test Suite & Documentation: Pure unit tests, mocked OpenSearch PIT streaming tests, and REST integration tests are all provided, and documentation across queries/cdmq/README.md, web-ui-requirements.md, ARCHITECTURE.md, and DESIGN.md is updated.

Verification

Ran test suites in queries/cdmq:

npm run test:native-stats
# 9/9 tests passed (0 skipped, 0 failed)

npm run test:native-stats:integration
# 5/5 tests passed (0 skipped, 0 failed)

Verdict

APPROVE

The core functionality, data integrity guarantees, and integration points are solid, thoroughly tested, and ready for merge. The error-code classification items can be addressed as quick follow-ups or minor polish.

— AI-signed: Antigravity | model: Gemini 3.7 Flash | effort: High

Return native timeline data-quality failures as HTTP 422 responses with actionable error messages, and avoid an unnecessary final OpenSearch page request when pagination reaches a short page. Add regression coverage for both behaviors.

AI-Tool: Codex
AI-Model: GPT-5
AI-Effort: not specified
@k-rister

k-rister commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review feedback in commit a25d819:

  • Timeline validation failures now carry NATIVE_STATS_DATA_QUALITY and return HTTP 422 with the actionable message.
  • Added regression coverage for data-quality error classification.
  • Added the short-page pagination optimization and regression coverage; the final empty-page request is avoided when the last page is shorter than the configured page size.
  • Unit tests, integration tests, syntax checks, Prettier, and diff checks pass.

The Crucible tab-completion suggestion remains a separate follow-up in the parent repository.

— AI-signed: Codex | model: GPT-5 | effort: not specified

@atheurer atheurer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the native duration-weighted statistics implementation, PIT pagination, API/CLI integration, limits, error handling, and tests. The previously identified data-quality classification and short-page pagination issues are addressed, and reported CI checks are green. Approve.

@k-rister
k-rister merged commit 5555b3e into master Sep 8, 2026
38 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Crucible Tracking Sep 8, 2026
@k-rister
k-rister deleted the feat/native-distribution-stats branch September 8, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

feat: temporal distribution stats across resolution windows (stddev, median, percentile)

2 participants