feat: add metric name validation

[michaeldwan]

Summary

Add validation for metric names in record_metric() and delete operations to ensure metric names are valid for downstream systems (OTel, Prometheus, JSON).

Validation Rules

• Each segment must start with a letter (a-z, A-Z) and end with a letter or digit
• Segments can contain letters, digits, and underscores (_)
• Segments cannot start or end with underscores
• Segments cannot contain consecutive underscores (__)
• Use dots (.) to create nested objects (e.g., timing.inference)
• Maximum 128 characters total
• Maximum 4 dot-separated segments
• Cannot be predict_time (reserved by runtime)
• Cannot start with cog. (reserved for system metrics)

Changes

• crates/coglet-python/src/metric_scope.rs: Added validate_metric_name() and validate_metric_name_for_delete() with 18 unit tests
• docs/python.md: Added "Naming rules" section under Metrics
• python/cog/predictor.py: Updated docstring for record_metric()
• integration-tests/tests/coglet_metrics.txtar: Removed predict_time user-override test
• integration-tests/tests/coglet_metrics_validation.txtar: New integration test for validation errors
• crates/coglet-python/coglet/_sdk/__init__.pyi: Updated docstrings

Breaking Change

Users can no longer set predict_time or use the cog. prefix in metric names. Previously, predict_time was silently overridden by the runtime. Now it raises a ValueError with a clear message.

[ask-bonk]

The metric name validation logic looks solid with good test coverage. However, there's a significant code duplication issue between validate_metric_name() and validate_metric_name_for_delete() - they share ~90 lines of identical validation logic.

I recommend refactoring to extract the common validation logic into a shared helper function, with only the reserved name checks varying between the two public functions.

The duplication spans lines 407-498 (validate_metric_name_for_delete) which is nearly identical to lines 293-396 (validate_metric_name). The only meaningful difference is that validate_metric_name checks reserved names (predict_time and cog.* prefix) while validate_metric_name_for_delete skips those checks.

[ask-bonk]

Posted review identifying the ~90 line code duplication between validate_metric_name() and validate_metric_name_for_delete() in metric_scope.rs. The PR adds solid validation logic with good test coverage, but should refactor to extract common validation into a shared helper function.

github run

[ask-bonk]

The code duplication issue has been addressed. The author extracted the common validation logic into a shared helper function validate_metric_name_structure() (lines 293-385), which is now called by both validate_metric_name() and validate_metric_name_for_delete(). This eliminates the ~90 line duplication that was previously identified.

LGTM

github run

[ask-bonk]

LGTM

github run

[ask-bonk]

LGTM

github run