From 88597bd8d2a285b90d8b5598634820fe36a7cf6a Mon Sep 17 00:00:00 2001 From: blizhan Date: Tue, 16 Jun 2026 15:22:37 +0800 Subject: [PATCH] feat: enhance query and trace commands with default expressions and duration tracking - Updated query commands to use a default expression when omitted, improving usability. - Added duration tracking for run parameters, including status and source information. - Enhanced output formats to include duration in rich, plain, and JSON representations. - Modified help documentation to reflect changes in command usage and default behaviors. - Expanded tests to ensure correct functionality of new features and maintain existing behavior. --- .gitignore | 5 +- AGENTS.md | 2 + README.md | 46 ++-- .../checklists/requirements.md | 37 +++ .../contracts/cli-output.md | 146 ++++++++++ specs/006-default-query-runtime/data-model.md | 114 ++++++++ specs/006-default-query-runtime/plan.md | 168 ++++++++++++ specs/006-default-query-runtime/quickstart.md | 103 ++++++++ specs/006-default-query-runtime/research.md | 115 ++++++++ specs/006-default-query-runtime/spec.md | 223 ++++++++++++++++ specs/006-default-query-runtime/tasks.md | 249 ++++++++++++++++++ src/aimx/aim_bridge/run_params.py | 46 +++- src/aimx/commands/help.py | 14 +- src/aimx/commands/query.py | 15 +- src/aimx/commands/trace.py | 35 ++- src/aimx/rendering/params_views.py | 36 ++- tests/contract/test_query_contract.py | 99 ++++++- tests/contract/test_trace_contract.py | 107 +++++++- tests/integration/test_query_command.py | 107 +++++++- tests/integration/test_trace_command.py | 47 +++- tests/unit/test_query_helpers.py | 64 +++++ tests/unit/test_run_params.py | 175 +++++++++++- tests/unit/test_trace_helpers.py | 92 ++++++- 23 files changed, 1972 insertions(+), 73 deletions(-) create mode 100644 specs/006-default-query-runtime/checklists/requirements.md create mode 100644 specs/006-default-query-runtime/contracts/cli-output.md create mode 100644 specs/006-default-query-runtime/data-model.md create mode 100644 specs/006-default-query-runtime/plan.md create mode 100644 specs/006-default-query-runtime/quickstart.md create mode 100644 specs/006-default-query-runtime/research.md create mode 100644 specs/006-default-query-runtime/spec.md create mode 100644 specs/006-default-query-runtime/tasks.md diff --git a/.gitignore b/.gitignore index 5d98a29..607cf78 100644 --- a/.gitignore +++ b/.gitignore @@ -188,7 +188,10 @@ cython_debug/ # that can be found at https://github.com/github/gitignore/blob/main/Global/VisualStudioCode.gitignore # and can be added to the global gitignore or merged into this file. However, if you prefer, # you could uncomment the following to ignore the entire vscode folder -# .vscode/ +.vscode/ + +# Generic IDE metadata +.idea/ # OS / editor .DS_Store diff --git a/AGENTS.md b/AGENTS.md index 34854fb..f653761 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,6 +83,8 @@ Why `3.12`: - Existing local Aim repositories on disk (read-only); run params are read from Aim run metadata attributes under `.aim` (004-run-params-query) - Python 3.12 for development, runtime support `>=3.10,<3.13` + Python standard library, `numpy>=1.24`, `rich>=13.7`, `plotext>=5.3`, existing Aim SDK usage for owned trace commands; no new runtime dependency planned (005-distribution-trace-visual) - Existing local Aim repositories on disk, read-only; distribution histogram points are read from Aim sequence data under `.aim` (005-distribution-trace-visual) +- Python 3.12 for development, runtime support `>=3.10,<3.13` + Python standard library, `numpy>=1.24`, `rich>=13.7`, `plotext>=5.3`, `textual-image>=0.12.0`, existing Aim SDK usage for owned query and trace commands; no new dependency planned (007-default-query-runtime) +- Existing local Aim repositories on disk, read-only; query/trace data and run timing metadata are read from `.aim` repositories without modification (007-default-query-runtime) ## Recent Changes - 001-aim-command-passthrough: Added Python 3.12 for development, runtime support `>=3.10,<3.13` + Python standard library, native Aim CLI (external runtime prerequisite for delegated commands), pytest for test automation diff --git a/README.md b/README.md index dbc96bf..b2aa27d 100644 --- a/README.md +++ b/README.md @@ -64,17 +64,17 @@ When provided, `--repo` accepts either the repository root, such as `data`, or the metadata directory itself, such as `data/.aim`. ```bash -# Summarize matching metrics -aimx query metrics "metric.name == 'loss'" --repo data +# Summarize metrics from all runs +aimx query metrics --repo data -# Preview matching images in supported terminals -aimx query images "images" --repo data +# Preview images in supported terminals +aimx query images --repo data -# Compare run parameters across matching runs -aimx query params "run.hash != ''" --repo data +# Compare run parameters and durations +aimx query params --repo data -# Plot a metric time series -aimx trace "metric.name == 'loss'" --repo data +# Plot metric time series from all runs +aimx trace --repo data ``` ## Features @@ -106,8 +106,9 @@ aimx trace "metric.name == 'loss'" --repo data | `aimx query params` | Compare run-level parameters across matching runs. | | `aimx trace` | Plot, tabulate, or export metric time series. | -Both `aimx query` and `aimx trace` accept **AimQL** expressions as their filter -argument. AimQL is Aim's native Python-like query language. +Both `aimx query` and `aimx trace` accept optional **AimQL** expressions as +their filter argument. When the expression is omitted or blank, `aimx` uses +`run.hash != ''`. AimQL is Aim's native Python-like query language. ```bash aimx query metrics "metric.name == 'loss' and run.hparams.learning_rate > 0.001" @@ -126,6 +127,9 @@ statistics such as step count, last value, min value, and max value. ```bash # Rich table, colored in terminals by default +aimx query metrics --repo data + +# Filter with AimQL aimx query metrics "metric.name == 'loss'" --repo data # Short run hashes are transparently expanded to full hashes @@ -156,11 +160,11 @@ images directly in the terminal. ```bash # Inline preview in modern terminals -aimx query images "images" --repo data +aimx query images --repo data # Metadata output only -aimx query images "images" --repo data --plain -aimx query images "images" --repo data --json +aimx query images --repo data --plain +aimx query images --repo data --json # Filter and sample matching image rows aimx query images "images" --repo data --epochs 10:50 --head 10 @@ -193,7 +197,7 @@ matching runs. ```bash # Compare discovered params across all matching runs -aimx query params "run.hash != ''" --repo data +aimx query params --repo data # Select specific params aimx query params "run.experiment == 'cloud-segmentation'" --repo data \ @@ -209,7 +213,9 @@ aimx query params "run.hparam.lr == 0.0001" --repo data --param hparam.lr ``` Missing selected params are displayed as `-` in terminal/plain output and listed -under `missing_params` in JSON. +under `missing_params` in JSON. Params output also includes run duration as a +table column, a plain-output field after run name, and a JSON `duration` object +with `seconds`, `status`, and `source`. ### Trace metrics @@ -219,6 +225,9 @@ overlaid on the same plot. ```bash # Plot all matching loss curves +aimx trace --repo data + +# Filter with AimQL aimx trace "metric.name == 'loss'" --repo data # Plot one run by short hash @@ -252,7 +261,7 @@ current-step histogram and step-by-bin heatmap. Use `--table`, `--csv`, or ```bash # Show a web-like terminal visual for the first matched distribution -aimx trace distribution "distribution.name != ''" --repo data +aimx trace distribution --repo data # Inspect a specific training step; nearest tracked step is used if needed aimx trace distribution "distribution.name != ''" --repo data --step 12300 @@ -325,6 +334,7 @@ Useful local checks: uv run aimx --help uv run aimx version uv run aimx doctor -uv run aimx query metrics "metric.name == 'loss'" --repo data -uv run aimx query images "images" --repo data/.aim --json +uv run aimx query metrics --repo data +uv run aimx query images --repo data/.aim --json +uv run aimx query params --repo data --json ``` diff --git a/specs/006-default-query-runtime/checklists/requirements.md b/specs/006-default-query-runtime/checklists/requirements.md new file mode 100644 index 0000000..cf5faab --- /dev/null +++ b/specs/006-default-query-runtime/checklists/requirements.md @@ -0,0 +1,37 @@ +# Specification Quality Checklist: Default Query Expression And Run Duration + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-06-14 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- Validation iteration 1 passed. The spec uses user-facing command behavior and output contracts, includes no [NEEDS CLARIFICATION] markers, and bounds scope to read-only default-expression handling plus run duration in params results. +- Implementation verification on 2026-06-15 passed manual quickstart commands, + focused query/trace/params tests, passthrough regression tests, and full + suite validation (`347 passed, 15 skipped`). diff --git a/specs/006-default-query-runtime/contracts/cli-output.md b/specs/006-default-query-runtime/contracts/cli-output.md new file mode 100644 index 0000000..a4884f1 --- /dev/null +++ b/specs/006-default-query-runtime/contracts/cli-output.md @@ -0,0 +1,146 @@ +# CLI Output Contract: Default Query Expression And Run Duration + +**Feature**: `006-default-query-runtime` + +This contract defines observable CLI behavior for omitted query-language input +and run duration in `aimx query params`. + +## Affected Command Shapes + +The following owned command forms accept an omitted expression and use +`run.hash != ''` as the effective expression: + +```text +aimx query metrics [] [--repo ] [--json] [--oneline | --plain] + [--steps start:end | --epochs start:end] + [--head N] [--tail N] [--every K] + +aimx query images [] [--repo ] [--json] [--oneline | --plain] + [--steps start:end | --epochs start:end] + [--head N] [--tail N] [--every K] [--max-images N] + +aimx query params [] [--repo ] [--json] [--oneline | --plain] + [--param ]... + +aimx trace [] [--repo ] [--table | --csv | --json] + [--steps start:end] [--head N] [--tail N] [--every K] + [--width W] [--height H] [--no-color] + +aimx trace distribution [] [--repo ] + [--steps start:end] + [--head N] [--tail N] [--every K] + [--step N] + [--width W] [--height H] [--no-color] + [--table | --csv | --json] +``` + +The command target is still required for `aimx query`. The `distribution` +subtarget is still required for distribution trace. + +## Effective Expression Rules + +| Input Shape | Effective Expression | +|-------------|----------------------| +| Expression omitted | `run.hash != ''` | +| Expression token is empty or whitespace-only | `run.hash != ''` | +| First token after target is an option such as `--repo` | `run.hash != ''`; option parsing starts at that token | +| Non-empty explicit expression | The explicit expression exactly as supplied | + +Examples: + +```bash +aimx query metrics --repo data +aimx query images --repo data --plain +aimx query params --repo data --json +aimx trace --repo data +aimx trace --json --repo data +aimx trace distribution --repo data --head 2 +``` + +Each example above is equivalent to the same command with +`"run.hash != ''"` supplied as the expression. + +## Query Params Duration Output + +`aimx query params` includes run duration in every output mode. + +### Human-Readable Output + +The default params table includes a duration column near the run identity +columns: + +```text +RUN EXPERIMENT NAME DURATION hparam.lr ... +eca37394 cloud-segmentation ucloudnet-pre-0503 1h20m13s 0.0001 +``` + +If duration cannot be determined, the cell is an explicit marker such as +`unavailable` or `running`; it must not be blank or `0s` unless duration is +actually zero. + +### Plain Output + +Plain output emits one tab-separated row per matched run: + +```text + ... +``` + +Missing duration uses the same explicit marker semantics as the rich output. + +### JSON Output + +JSON output keeps the existing top-level params envelope and adds a stable +`duration` object for every run: + +```json +{ + "target": "params", + "repo": "data", + "expression": "run.hash != ''", + "runs_count": 1, + "param_keys": ["hparam.lr"], + "runs": [ + { + "hash": "eca37394eeb84f48a5d2d736", + "experiment": "cloud-segmentation", + "name": "ucloudnet-pre-0503", + "duration": { + "seconds": 4813.352312088013, + "status": "available", + "source": "duration" + }, + "params": { + "hparam.lr": 0.0001 + }, + "missing_params": [] + } + ] +} +``` + +When duration cannot be calculated, `duration.seconds` is `null` and +`duration.status` is `unavailable` or `running`. + +## Exit Status + +| Condition | Exit Status | Output | +|-----------|-------------|--------| +| Omitted expression with valid repo and matches | `0` | Same mode-specific output as explicit `run.hash != ''` | +| Omitted expression with valid repo and zero matches | `0` | Existing no-results or empty structured output behavior | +| Explicit valid expression | `0` | Same output behavior as before this feature | +| Explicit invalid expression | `2` | Existing actionable evaluation error on stderr | +| Missing repository path | `2` | Existing actionable repository error on stderr | +| Missing option value | `2` | Existing actionable parser error on stderr | +| Missing duration metadata on a params run | `0` | Run remains visible with duration status marker | + +## Non-Regression Requirements + +- Explicit query and trace expressions keep existing match semantics. +- `query images` inline rendering and `--max-images` behavior remain unchanged. +- `query params --param` selection and missing-param behavior remain unchanged. +- Metric trace and distribution trace `--table`, `--csv`, and `--json` modes + remain parseable with their existing data shapes, except where an existing + output already reports an expression and must now report the effective + default. +- Commands outside owned `aimx` surfaces continue to delegate to native `aim`. diff --git a/specs/006-default-query-runtime/data-model.md b/specs/006-default-query-runtime/data-model.md new file mode 100644 index 0000000..17ab2f6 --- /dev/null +++ b/specs/006-default-query-runtime/data-model.md @@ -0,0 +1,114 @@ +# Phase 1 Data Model: Default Query Expression And Run Duration + +## Default Query Expression + +Represents the query-language expression used when the user does not provide a +meaningful expression. + +### Fields + +- `value`: Always `run.hash != ''`. +- `source`: `default` when supplied by `aimx`, `explicit` when provided by the + user. + +### Validation Rules + +- The default is used only when expression input is absent, empty, or + whitespace-only. +- A non-empty explicit expression always takes precedence. +- Option tokens must remain options; they must not be consumed as expressions + when expression input is omitted. + +## Query Or Trace Invocation + +Represents one parsed owned command request. + +### Fields + +- `target`: Query target (`metrics`, `images`, `params`) or trace target + (`metrics`, `distribution`). +- `repo_path`: User-provided local repository path, normalized later to a repo + root when `.aim` is supplied. +- `expression`: Effective expression after applying omitted-expression default. +- `expression_source`: Whether the effective expression came from user input or + the default. +- `output_mode`: Rich/default, plain, table, CSV, or JSON depending on command + surface. +- `filters`: Existing target-specific filters such as step range, epoch range, + sampling, image cap, selected params, dimensions, or selected distribution + step. + +### Validation Rules + +- Query target remains required. +- Metric trace target may be implicit; distribution trace target remains + explicit through `trace distribution`. +- Existing option validation remains unchanged for missing values, unsupported + options, mutually exclusive filters, and invalid numeric values. +- Invalid non-empty expressions are not repaired during parsing; they fail + through the existing evaluation path. + +## Run Identity + +Represents the run identity shown in query and params results. + +### Fields + +- `hash`: Stable full run hash. +- `experiment`: Optional experiment label. +- `name`: Optional run display name. +- `creation_time`: Optional creation timestamp already used by existing run + metadata helpers. + +### Relationships + +- Query metrics, query images, trace metrics, trace distributions, and query + params all attach results to run identity. +- Params rows add run duration metadata next to the same identity. + +## Run Duration + +Represents elapsed runtime for a returned params run. + +### Fields + +- `seconds`: Numeric elapsed seconds when available; `null` when not + calculated. +- `status`: `available`, `running`, or `unavailable`. +- `source`: `duration`, `end_time_minus_creation_time`, or `missing_metadata`. + +### Validation Rules + +- If Aim exposes a valid non-negative `duration`, use it. +- If `duration` is missing but `end_time` and `creation_time` are valid, use + `end_time - creation_time`. +- Negative or non-numeric values are treated as unavailable. +- Runs with unavailable duration remain in output. +- A missing duration is never rendered as zero. + +## Params Result Row + +Represents one returned run in `aimx query params`. + +### Fields + +- `run`: Run identity. +- `duration`: Run duration metadata. +- `params`: Flattened selected or default parameter values. +- `selected_keys`: User-requested flattened parameter keys, if any. +- `missing_keys`: User-requested keys absent from this run. + +### Relationships + +- A params query result contains zero or more params result rows. +- Each row belongs to exactly one run identity. +- Duration is independent from the `params` map and does not affect parameter + selection or missing-key tracking. + +### Validation Rules + +- Rows with no params remain visible. +- Rows with missing selected params remain visible and list missing keys. +- JSON output includes duration for every row. +- Rich and plain outputs display duration for every row as a value or explicit + unavailable/running marker. diff --git a/specs/006-default-query-runtime/plan.md b/specs/006-default-query-runtime/plan.md new file mode 100644 index 0000000..13f08e6 --- /dev/null +++ b/specs/006-default-query-runtime/plan.md @@ -0,0 +1,168 @@ +# Implementation Plan: Default Query Expression And Run Duration + +**Branch**: `007-default-query-runtime` | **Date**: 2026-06-14 | **Spec**: [spec.md](/Users/blizhan/data/code/github/aimx/specs/006-default-query-runtime/spec.md) +**Input**: Feature specification from `/Users/blizhan/data/code/github/aimx/specs/006-default-query-runtime/spec.md` + +## Summary + +Make the existing `aimx`-owned query and trace commands easier to use by +defaulting omitted or blank query-language input to `run.hash != ''`. Apply the +same effective-expression behavior to `aimx query metrics`, `aimx query images`, +`aimx query params`, `aimx trace`, and `aimx trace distribution`, while +preserving explicit expressions and existing output modes. Extend the params +query result with read-only run duration metadata so users can compare run +configuration alongside elapsed runtime. The implementation stays within +current parser, bridge, and renderer boundaries; it adds no new runtime +dependency and does not mutate Aim repositories or native Aim passthrough. + +## Technical Context + +**Language/Version**: Python 3.12 for development, runtime support `>=3.10,<3.13` +**Primary Dependencies**: Python standard library, `numpy>=1.24`, `rich>=13.7`, `plotext>=5.3`, `textual-image>=0.12.0`, existing Aim SDK usage for owned query and trace commands; no new dependency planned +**Storage**: Existing local Aim repositories on disk, read-only; query/trace data and run timing metadata are read from `.aim` repositories without modification +**Testing**: pytest unit, integration, and contract suites; sample Aim repository rooted at `/Users/blizhan/data/code/github/aimx/data` for end-to-end validation +**Target Platform**: Terminal-first CLI for local shells, SSH sessions, scripts, and CI on Python-supported platforms +**Project Type**: Single-project Python CLI application +**Performance Goals**: Omitted-expression commands should add no extra repository scans beyond the same commands run explicitly with `run.hash != ''`; params duration extraction should be O(1) per returned run and avoid metric/image/blob loading +**Constraints**: Read-only; preserve native Aim passthrough behavior; preserve explicit-expression semantics; preserve existing image rendering caps, params selection, trace export modes, and stable machine-readable output parseability +**Scale/Scope**: Five owned command forms, one shared default expression, params duration extraction/rendering, help/README updates, and focused unit/integration/contract tests + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +- [x] Safe coexistence: default-expression handling and params duration display + only read existing local Aim data; no normal-path change modifies the + installed `aim` package, replaces the native `aim` executable, or mutates + `.aim` repo data. +- [x] Ownership boundary: all behavior changes are limited to `aimx`-owned + command paths: `query metrics`, `query images`, `query params`, `trace`, + and `trace distribution`. Native Aim passthrough remains unchanged. +- [x] Read-only default: the feature inspects query results and run metadata + only and does not introduce write, repair, migration, or sync behavior. +- [x] CLI-first contract: default rich/plain/JSON/export workflows remain + non-interactive and scriptable; params JSON gains a stable duration field + while existing explicit-expression workflows remain test-covered. +- [x] Compatibility plan: design reuses current repo normalization, short-hash + expansion, parser error paths, query/trace collection helpers, and pytest + suites; tests compare omitted-expression behavior against the explicit + `run.hash != ''` baseline. + +## Project Structure + +### Documentation (this feature) + +```text +/Users/blizhan/data/code/github/aimx/specs/006-default-query-runtime/ +├── plan.md +├── research.md +├── data-model.md +├── quickstart.md +├── contracts/ +│ └── cli-output.md +├── checklists/ +│ └── requirements.md +└── tasks.md # created later by /speckit.tasks +``` + +### Source Code (repository root) + +```text +/Users/blizhan/data/code/github/aimx/ +├── README.md +├── src/aimx/ +│ ├── commands/ +│ │ ├── help.py # document omitted-expression defaults +│ │ ├── query.py # default expression parsing for query targets +│ │ └── trace.py # default expression parsing for trace targets +│ ├── aim_bridge/ +│ │ └── run_params.py # add run duration metadata to params rows +│ └── rendering/ +│ └── params_views.py # show duration in rich/plain/JSON params output +└── tests/ + ├── contract/ + │ ├── test_query_contract.py # omitted-expression and params-duration contracts + │ └── test_trace_contract.py # omitted-expression trace contracts + ├── integration/ + │ ├── test_query_command.py # sample-repo omitted-expression query coverage + │ └── test_trace_command.py # sample-repo omitted-expression trace coverage + └── unit/ + ├── test_query_helpers.py # parser defaults and option ordering + ├── test_trace_helpers.py # parser defaults for trace/distribution + └── test_run_params.py # duration extraction and unavailable status +``` + +**Structure Decision**: Keep the existing single-project CLI layout. Put +default-expression parsing in the command modules because that is where user +arguments become invocations. Keep duration extraction inside the params bridge +and duration formatting inside params renderers so metrics, images, trace data, +and native passthrough stay untouched. + +## Phase 0: Research Summary + +Phase 0 decisions are captured in [research.md](/Users/blizhan/data/code/github/aimx/specs/006-default-query-runtime/research.md). Key outcomes: + +- Use the literal shared default expression `run.hash != ''` when query-language + input is omitted or blank. +- Treat option-looking tokens such as `--repo` or `--json` as the start of + options, not as expressions, so `aimx query params --repo data` and + `aimx trace --repo data` are valid default-expression forms. +- Preserve all non-empty explicit expressions exactly as today, including + invalid expressions that should still fail through the existing Aim evaluator. +- Use Aim run timing metadata already exposed on local run objects. Prefer + `run.duration` when available, fall back to `run.end_time - run.creation_time`, + and mark duration as unavailable/still-running when timing metadata is + incomplete. +- Keep duration as first-class run metadata in params output, not as a synthetic + parameter key, so selected param comparisons remain stable. + +## Phase 1: Design Summary + +- Add a shared constant such as `DEFAULT_QUERY_EXPRESSION = "run.hash != ''"` + in the command parsing layer or a small shared helper, then use it from query + and trace parsers. +- Update `parse_query_invocation` so the target remains required, but the + expression becomes optional. If the next token is absent, blank, or starts + with `-`, set the expression to the default and parse remaining tokens as + options. If the next token is non-empty and does not start with `-`, preserve + it as the explicit expression. +- Update `parse_trace_invocation` for both metric and distribution forms: + `aimx trace --repo data`, `aimx trace --json --repo data`, and + `aimx trace distribution --repo data` all use the default expression, while + non-empty explicit expressions continue to win. +- Keep current error handling for unsupported options, missing option values, + invalid repositories, and invalid non-empty expressions. +- Extend params data with a duration model that carries `seconds: float | None` + and `status` (`available`, `unavailable`, or `running`). Derive it during + `collect_run_params` while the raw Aim run object is still available. +- Update params renderers: + - Rich table adds a `DURATION` column after run identity columns. + - Plain output adds a duration cell after run name. + - JSON output adds a stable `duration` object per run, separate from `params`. +- Update README/help examples to show omitted-expression forms and explain that + `run.hash != ''` is the default when no expression is supplied. +- Validate with parser unit tests, params duration unit tests, query/trace + contract tests, integration tests against `data`, and existing passthrough + tests. + +## Post-Design Constitution Check + +- [x] Safe coexistence: design reads existing query results and run timing + attributes only; it does not modify the installed Aim package, + executable, or repository data. +- [x] Ownership boundary: behavior is contained to existing `aimx`-owned query + and trace command paths plus their docs/tests; native Aim passthrough is + not intercepted or extended. +- [x] Read-only default: default expressions broaden inspection convenience but + still invoke read-only Aim query APIs and in-memory renderers. +- [x] CLI-first contract: [contracts/cli-output.md](/Users/blizhan/data/code/github/aimx/specs/006-default-query-runtime/contracts/cli-output.md) + defines command shapes, output expectations, JSON duration shape, and + exit statuses for automation. +- [x] Compatibility: explicit-expression behavior, output parseability, image + rendering controls, trace modes, params selection, and passthrough tests + remain part of the validation set. + +## Complexity Tracking + +No constitution violations; no exceptional complexity requires justification. +The feature uses existing command, bridge, renderer, and test boundaries. diff --git a/specs/006-default-query-runtime/quickstart.md b/specs/006-default-query-runtime/quickstart.md new file mode 100644 index 0000000..02e0885 --- /dev/null +++ b/specs/006-default-query-runtime/quickstart.md @@ -0,0 +1,103 @@ +# Quickstart: Default Query Expression And Run Duration + +This quickstart validates the feature against the local Aim test repository at +`/Users/blizhan/data/code/github/aimx/data`. + +## 1. Sync The Environment + +```bash +uv sync --group dev +``` + +## 2. Query Without Expressions + +Each command should behave as though `"run.hash != ''"` was supplied. + +```bash +uv run aimx query metrics --repo data +uv run aimx query images --repo data --plain +uv run aimx query params --repo data +``` + +Expected: + +- all commands exit with status `0` +- query output reports or behaves according to the effective expression + `run.hash != ''` +- explicit output controls such as `--plain` keep their existing shape + +## 3. Trace Without Expressions + +```bash +uv run aimx trace --repo data --head 5 +uv run aimx trace --repo data --json --head 1 +uv run aimx trace distribution --repo data --head 2 --no-color +``` + +Expected: + +- metric trace uses all runs as the default match set +- JSON export remains parseable +- distribution trace prints its default visual output when the repository has + distribution data, or the existing no-matches message when it does not + +## 4. Confirm Explicit Expressions Still Win + +```bash +uv run aimx query metrics "metric.name == 'loss'" --repo data --json +uv run aimx trace "metric.name == 'loss'" --repo data --table --head 2 +uv run aimx trace distribution "distribution.name != ''" --repo data --json --head 1 +``` + +Expected: + +- explicit expressions are preserved +- known invalid expressions still fail with exit status `2` +- structured outputs remain parseable + +## 5. Check Params Duration + +```bash +uv run aimx query params --repo data +uv run aimx query params --repo data --plain +uv run aimx query params --repo data --json +``` + +Expected: + +- rich output includes a duration column +- plain output includes a duration field after run name +- JSON output includes `duration.seconds`, `duration.status`, and + `duration.source` for every returned run +- runs without complete timing metadata remain visible with an explicit + unavailable or running duration status + +## 6. Focused Regression Checks + +```bash +uv run pytest tests/unit/test_query_helpers.py tests/unit/test_trace_helpers.py tests/unit/test_run_params.py -q +uv run pytest tests/contract/test_query_contract.py tests/contract/test_trace_contract.py -q +uv run pytest tests/integration/test_query_command.py tests/integration/test_trace_command.py -q +uv run pytest tests/integration/test_passthrough_behavior.py -q +``` + +Expected: + +- parser defaults are covered for query and trace +- params duration contract is covered in rich/plain/JSON paths +- omitted-expression behavior matches explicit `run.hash != ''` +- native Aim passthrough behavior remains unchanged + +## 7. Implementation Verification Notes + +Verified on 2026-06-15 against the local `data` Aim repository. + +- Sections 2-5 manual CLI commands exited with status `0`. +- Distribution quickstart commands exited with status `0`; the local sample + repository currently reports the existing no-matches path for distribution + data in some pytest scenarios, so those tests keep their skip behavior. +- Focused unit checks passed: `144 passed`. +- Focused contract checks passed: `39 passed, 8 skipped`. +- Focused integration checks passed: `40 passed, 7 skipped`. +- Passthrough regression checks passed: `10 passed`. +- Full suite passed: `347 passed, 15 skipped`. diff --git a/specs/006-default-query-runtime/research.md b/specs/006-default-query-runtime/research.md new file mode 100644 index 0000000..1767804 --- /dev/null +++ b/specs/006-default-query-runtime/research.md @@ -0,0 +1,115 @@ +# Phase 0 Research: Default Query Expression And Run Duration + +## Decision: Use A Single Shared Default Expression + +Use the literal expression `run.hash != ''` whenever query-language input is +omitted or blank for the owned query and trace surfaces. + +**Rationale**: This is the expression requested in the feature description and +matches existing examples already used for "all runs" params queries. Sharing +one value across command parsers keeps behavior consistent and makes output +contracts easy to verify. + +**Alternatives considered**: + +- Target-specific defaults such as `metric.name != ''`, `images`, or + `distribution.name != ''`: rejected because the request explicitly asks for + `run.hash != ''` and consistency matters more than target-specific narrowing. +- Leaving trace commands expression-required: rejected because the feature + explicitly includes `trace` and `trace distribution`. +- Empty string forwarded to Aim: rejected because Aim expression behavior would + be less explicit and less stable for users and tests. + +## Decision: Treat Option-Looking Tokens As Omitted Expression + +When the parser sees an option token where an expression could appear, treat the +expression as omitted and parse the token as the first option. + +**Rationale**: Users should be able to run concise commands like +`aimx query params --repo data`, `aimx trace --repo data`, and +`aimx trace distribution --repo data`. Treating `--repo` as an expression would +be surprising and would break normal command-line conventions. + +**Alternatives considered**: + +- Require a `--` delimiter before options when omitting expressions: rejected + as too awkward for the main usability feature. +- Require a new `--all-runs` flag: rejected because the request asks for a + default when no query language input is present, not another explicit option. +- Accept only fully omitted expressions but not blank strings: rejected because + whitespace-only input is semantically empty and the spec requires the same + default behavior. + +## Decision: Preserve Explicit Expression Semantics Exactly + +Any non-empty, non-option expression token supplied by the user remains the +effective expression, even if the expression is syntactically invalid. + +**Rationale**: Existing explicit-expression behavior is a compatibility +contract. Invalid expressions should still be forwarded to the existing +evaluation path and fail with the same actionable error pattern, while valid +expressions should produce the same result set as before. + +**Alternatives considered**: + +- Validate expression syntax in the parser: rejected because Aim's evaluator is + already the source of truth and the project currently keeps parser validation + limited to command shape and options. +- Auto-repair invalid expressions to the default: rejected because that would + hide user mistakes and change failure semantics. + +## Decision: Keep Duration As Params Run Metadata + +Add run duration to `query params` as first-class run metadata, separate from +the parameter map and missing-parameter tracking. + +**Rationale**: Duration describes the run, not a user-recorded hyperparameter. +Keeping it separate prevents accidental collision with user param keys, +preserves selected-parameter behavior, and gives JSON consumers a stable place +to read duration for every run. + +**Alternatives considered**: + +- Add duration as a synthetic param key such as `run.duration`: rejected because + it would mix system metadata with user params and affect default/selected + param key ordering. +- Show duration only in rich output: rejected because scripts also need a stable + machine-readable field. +- Add duration filtering or sorting in this feature: rejected as additional + scope not requested by the user. + +## Decision: Derive Duration From Existing Aim Run Timing Metadata + +Prefer Aim's exposed `duration` value when present. If it is missing, calculate +duration from `end_time - creation_time` when both values exist. If timing is +incomplete, keep the run visible and mark duration as unavailable or running. + +**Rationale**: Local inspection of the sample Aim repository shows run objects +already expose `duration`, `end_time`, `creation_time`, and `created_at`. +Reading these attributes is cheap, read-only, and avoids deeper coupling to +stored Aim internals. The fallback keeps older or partially recorded runs +usable. + +**Alternatives considered**: + +- Calculate duration from metric timestamps: rejected because it would load + unrelated sequence data and could be inaccurate for runs without metrics. +- Require both `creation_time` and `end_time`: rejected because `duration` may + already be directly available and should be used when present. +- Drop runs with missing duration: rejected because params comparison should not + lose runs due to incomplete metadata. + +## Decision: No New Runtime Dependency + +Use existing Python and Aim SDK access paths only. + +**Rationale**: The repository already depends on the libraries needed for query, +trace, rendering, and tests. Default-expression parsing and duration extraction +do not require additional packages. + +**Alternatives considered**: + +- Add a duration-formatting dependency: rejected because simple elapsed-time + formatting is small and should stay in the existing renderer layer. +- Add a CLI parser framework: rejected because the current parsers are small, + tested, and project-local. diff --git a/specs/006-default-query-runtime/spec.md b/specs/006-default-query-runtime/spec.md new file mode 100644 index 0000000..4ed2b2c --- /dev/null +++ b/specs/006-default-query-runtime/spec.md @@ -0,0 +1,223 @@ +# Feature Specification: Default Query Expression And Run Duration + +**Feature Branch**: `007-default-query-runtime` +**Created**: 2026-06-14 +**Status**: Draft +**Input**: User description: "feat - 新增默认在没有 Query language的cli输入的情况下，默认使用\"run.hash != ''\"，需要支持query metrics / query images/ query params / trace / trace distribution - query params 增加 运行时间" + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Query Runs Without Typing The Default Expression (Priority: P1) + +As an Aim user exploring a local repository from the terminal, I want the owned +`aimx query` commands to work when I omit the query-language expression, so I can +quickly inspect all recorded runs without repeatedly typing `run.hash != ''`. + +**Why this priority**: This is the central requested behavior for the query +surface. It removes boilerplate from the most common "show me what is in this +repo" workflow while keeping the same default match set users already type +manually. + +**Independent Test**: Run query metrics, query images, and query params against +the local test repository without a query-language expression and confirm that +each command behaves as though `run.hash != ''` had been supplied. + +**Acceptance Scenarios**: + +1. **Given** a local repository contains at least one run with metrics, **When** + the user runs `aimx query metrics --repo data`, **Then** the command uses + `run.hash != ''` as the effective expression and returns matching metric + results. +2. **Given** a local repository contains at least one run with images, **When** + the user runs `aimx query images --repo data`, **Then** the command uses + `run.hash != ''` as the effective expression and returns matching image + results. +3. **Given** a local repository contains runs with params, **When** the user runs + `aimx query params --repo data`, **Then** the command uses `run.hash != ''` as + the effective expression and returns matching params results. +4. **Given** the user supplies an explicit non-empty expression, **When** any of + the query commands run, **Then** that explicit expression is used instead of + the default. + +--- + +### User Story 2 - Trace Runs Without Typing The Default Expression (Priority: P1) + +As an Aim user inspecting time series or distributions, I want the owned trace +commands to use the same default expression when I omit the query-language +input, so trace exploration starts from all runs consistently with query +exploration. + +**Why this priority**: The feature request explicitly includes both trace +surfaces. Applying the same default across query and trace avoids a split mental +model where some commands require boilerplate and others do not. + +**Independent Test**: Run the metric trace command and distribution trace +command against a local repository without a query-language expression and +confirm both behave as though `run.hash != ''` had been supplied. + +**Acceptance Scenarios**: + +1. **Given** a local repository contains metric trace data, **When** the user + runs `aimx trace --repo data`, **Then** the command uses `run.hash != ''` as + the effective expression and returns matching trace output. +2. **Given** a local repository contains distribution trace data, **When** the + user runs `aimx trace distribution --repo data`, **Then** the command uses + `run.hash != ''` as the effective expression and returns matching + distribution trace output. +3. **Given** the user supplies an explicit non-empty expression, **When** either + trace command runs, **Then** that explicit expression is used instead of the + default. + +--- + +### User Story 3 - Compare Params With Run Duration Visible (Priority: P2) + +As a user comparing experiment parameters across runs, I want `aimx query +params` results to include each run's recorded run duration, so I can compare +configuration choices together with how long each run took. + +**Why this priority**: Duration adds important experiment-comparison context but +depends on the params query result surface already being available. It is +valuable as an independent enhancement once params rows are returned. + +**Independent Test**: Run a params query against a repository containing runs +with recorded start/end timing and confirm that each returned run shows a run +duration in human-readable and machine-readable output modes. + +**Acceptance Scenarios**: + +1. **Given** a matching run has recorded timing information, **When** the user + runs `aimx query params --repo data`, **Then** the run's duration is visible + alongside its run identity, experiment label, and parameter values. +2. **Given** the user requests machine-readable params output, **When** the + command succeeds, **Then** each returned run includes a stable duration field + that automation can distinguish from parameter values. +3. **Given** a matching run lacks enough timing information to calculate a + duration, **When** params results are rendered, **Then** that run remains in + the results and its duration is clearly marked as unavailable rather than + failing the command. + +### Edge Cases + +- The query-language expression is omitted entirely after the command target. +- The query-language expression is provided as an empty or whitespace-only + argument. +- The user supplies a non-empty explicit expression that matches zero runs. +- The user supplies a syntactically invalid non-empty expression. +- Repository paths may point to either a repository root or an Aim metadata + directory. +- The default expression matches many runs, metrics, images, params, or trace + series, so existing output controls and limits must continue to apply. +- A run is still in progress or has incomplete timing metadata. +- A run has duration metadata but no recorded params, or has params but no + duration metadata. +- Machine-readable output is consumed by scripts expecting stable top-level + expression and result metadata. + +## Constitution Alignment *(mandatory)* + +- **CA-001 Safety & Mutability**: This feature is read-only. It changes default + command interpretation and params output only; it MUST NOT modify the + installed Aim package, `.aim` repository data, run records, images, metrics, + params, or distribution data. +- **CA-002 Ownership Boundary**: `aimx` owns the default-expression behavior for + `aimx query metrics`, `aimx query images`, `aimx query params`, `aimx trace`, + and `aimx trace distribution`. Native Aim passthrough remains unchanged for + every command path not explicitly owned by `aimx`. +- **CA-003 CLI & Output Contract**: The affected commands remain terminal-first + and scriptable in local shells, SSH sessions, captured logs, and CI. Human + output must show useful defaults clearly, and machine-readable params output + must expose run duration without breaking existing result interpretation. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: When the user omits the query-language expression, the system MUST + use `run.hash != ''` as the effective expression. +- **FR-002**: The default expression behavior MUST apply to `aimx query metrics`, + `aimx query images`, and `aimx query params`. +- **FR-003**: The default expression behavior MUST apply to `aimx trace` and + `aimx trace distribution`. +- **FR-004**: Empty or whitespace-only query-language input MUST be treated the + same as omitted input for the command surfaces listed in FR-002 and FR-003. +- **FR-005**: A non-empty explicit expression supplied by the user MUST take + precedence over the default expression and preserve the command's existing + expression semantics. +- **FR-006**: Any human-readable or machine-readable output that reports the + effective expression MUST report `run.hash != ''` when that default was used. +- **FR-007**: Expected no-result cases under the default expression MUST + complete as clear, non-destructive outcomes without traceback-style failures. +- **FR-008**: Invalid repository paths, invalid non-empty expressions, and + unsupported option combinations MUST continue to fail clearly with actionable + messages. +- **FR-009**: `aimx query params` MUST include a run-duration value for each + returned run when recorded timing metadata is sufficient to determine it. +- **FR-010**: `aimx query params` MUST keep runs in the result set when duration + cannot be determined and MUST mark duration as unavailable or still-running in + a way users can distinguish from a zero-length run. +- **FR-011**: `aimx query params` human-readable and plain-text outputs MUST + display run duration alongside existing run identity, experiment label, run + name, and parameter information without removing any existing comparison + fields. +- **FR-012**: `aimx query params` machine-readable output MUST include a stable + duration field separate from the params map for every returned run. +- **FR-013**: Adding default expressions and run duration MUST NOT change image + rendering limits, params selection behavior, trace visual/export modes, or + native Aim passthrough behavior except where explicitly described above. +- **FR-014**: User-facing help or adjacent project documentation MUST show that + the listed query and trace commands can be run without an expression and will + default to `run.hash != ''`. + +### Key Entities + +- **Default Query Expression**: The literal expression `run.hash != ''`, used + only when the user does not provide meaningful query-language input. +- **Query/Trace Invocation**: A single command request, including command target, + repository path, optional user expression, effective expression, output mode, + and any target-specific filters. +- **Run Identity**: The stable run hash and optional display metadata that + identify a run in query and trace results. +- **Run Duration**: The elapsed runtime associated with a run, derived from + recorded run timing metadata when available and represented distinctly from + user-recorded params. +- **Params Result Row**: One returned run in `query params`, including run + identity, experiment label, run name, duration status, and selected or default + parameter values. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: In acceptance testing, all five affected command forms can be run + without a query-language expression and use `run.hash != ''` as the effective + expression. +- **SC-002**: For each affected command, an explicit non-empty expression + produces the same matched result set as before this feature for the same + repository and output mode. +- **SC-003**: In params-query acceptance testing, 100% of returned runs show + either a calculated run duration or an explicit unavailable/still-running + duration status. +- **SC-004**: Machine-readable params output remains parseable and includes the + effective expression, match count, run identity, params data, and run-duration + field for 100% of returned rows. +- **SC-005**: Omitted-expression, zero-match, invalid-expression, missing-repo, + and missing-duration cases complete without repository mutation and without + traceback-style output for expected user-facing conditions. +- **SC-006**: Existing documented image-rendering, params-selection, trace + visual/export, and passthrough workflows remain available after this feature. + +## Assumptions + +- "No Query language CLI input" means the query-language expression is omitted + or is provided only as blank whitespace; it does not include non-empty invalid + expressions. +- The default expression is exactly `run.hash != ''` across all affected + command surfaces. +- "query params 增加运行时间" refers to each returned Aim run's recorded duration, + not the wall-clock time spent executing the `aimx` command. +- If a run is still active or lacks enough timing metadata, showing an explicit + unavailable/still-running status is preferable to hiding the run. +- This feature does not add duration filtering, duration sorting, repository + mutation, data repair, or any new write-capable behavior. diff --git a/specs/006-default-query-runtime/tasks.md b/specs/006-default-query-runtime/tasks.md new file mode 100644 index 0000000..43c64ab --- /dev/null +++ b/specs/006-default-query-runtime/tasks.md @@ -0,0 +1,249 @@ +# Tasks: Default Query Expression And Run Duration + +**Input**: Design documents from `/Users/blizhan/data/code/github/aimx/specs/006-default-query-runtime/` +**Prerequisites**: plan.md, spec.md, research.md, data-model.md, contracts/cli-output.md, quickstart.md + +**Tests**: Test tasks are included because this feature changes owned CLI +defaults and params output contracts, and the constitution requires validation +for scriptable output, safe failure modes, read-only behavior, and native Aim +passthrough non-regression. + +**Organization**: Tasks are grouped by user story so each story can be +implemented and tested as an independently useful increment. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel because it touches different files or depends + only on completed foundation work +- **[Story]**: Maps task to a user story (`US1`, `US2`, `US3`) +- Every task includes exact repository-relative file paths + +## Phase 1: Setup (Shared Infrastructure) + +**Purpose**: Verify the existing CLI, bridge, renderer, docs, and test touch +points before changing behavior. + +- [X] T001 Review current query parser, dispatch, and params rendering touch points in `src/aimx/commands/query.py`, `src/aimx/aim_bridge/run_params.py`, and `src/aimx/rendering/params_views.py` +- [X] T002 [P] Review current trace parser and dispatch touch points in `src/aimx/commands/trace.py` and `src/aimx/rendering/trace_views.py` +- [X] T003 [P] Review existing CLI documentation examples that mention required expressions in `README.md` and `src/aimx/commands/help.py` +- [X] T004 [P] Review current parser, contract, and integration test coverage in `tests/unit/test_query_helpers.py`, `tests/unit/test_trace_helpers.py`, `tests/contract/test_query_contract.py`, `tests/contract/test_trace_contract.py`, `tests/integration/test_query_command.py`, and `tests/integration/test_trace_command.py` + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Add shared parser defaults and run-duration primitives required by +all user stories. + +**Critical**: No user story work should begin until this phase is complete. + +- [X] T005 [P] Add query parser unit tests for omitted expression, option-first expression, blank expression, explicit expression precedence, and missing target behavior in `tests/unit/test_query_helpers.py` +- [X] T006 [P] Add trace parser unit tests for omitted metric expression, option-first metric expression, omitted distribution expression, option-first distribution expression, and explicit expression precedence in `tests/unit/test_trace_helpers.py` +- [X] T007 [P] Add run-duration unit tests for `duration`, `end_time - creation_time`, negative values, non-numeric values, missing metadata, and running status in `tests/unit/test_run_params.py` +- [X] T008 Define shared `DEFAULT_QUERY_EXPRESSION = "run.hash != ''"` and update `parse_query_invocation()` to support optional expressions without changing explicit-expression behavior in `src/aimx/commands/query.py` +- [X] T009 Import the shared default expression and update `parse_trace_invocation()` to support optional metric and distribution expressions without changing explicit-expression behavior in `src/aimx/commands/trace.py` +- [X] T010 Add `RunDuration` metadata and duration extraction helpers for Aim run objects in `src/aimx/aim_bridge/run_params.py` +- [X] T011 Run `uv run pytest tests/unit/test_query_helpers.py tests/unit/test_trace_helpers.py tests/unit/test_run_params.py -q` and fix foundational failures in `src/aimx/commands/query.py`, `src/aimx/commands/trace.py`, and `src/aimx/aim_bridge/run_params.py` + +**Checkpoint**: Parser defaults and duration primitives are ready; user story implementation can start. + +--- + +## Phase 3: User Story 1 - Query Runs Without Typing The Default Expression (Priority: P1) MVP + +**Goal**: Users can run `aimx query metrics`, `aimx query images`, and +`aimx query params` without a query-language expression, and each command uses +`run.hash != ''` as the effective expression. + +**Independent Test**: Run `uv run aimx query metrics --repo data`, +`uv run aimx query images --repo data --plain`, and +`uv run aimx query params --repo data`; confirm each behaves as though +`"run.hash != ''"` had been supplied. + +### Tests for User Story 1 + +- [X] T012 [P] [US1] Add contract tests for omitted-expression `query metrics`, `query images`, and `query params` JSON/rich output effective expression in `tests/contract/test_query_contract.py` +- [X] T013 [P] [US1] Add integration tests comparing omitted-expression query commands to explicit `run.hash != ''` commands against `data` and `data/.aim` in `tests/integration/test_query_command.py` +- [X] T014 [P] [US1] Add unit tests proving option-first query invocations preserve existing option parsing for `--repo`, `--json`, `--plain`, `--steps`, `--epochs`, `--head`, `--tail`, `--every`, `--max-images`, and `--param` in `tests/unit/test_query_helpers.py` + +### Implementation for User Story 1 + +- [X] T015 [US1] Update query usage and validation messages so expressions are optional but query target remains required in `src/aimx/commands/query.py` +- [X] T016 [US1] Ensure `run_query_command()` reports the effective default expression in query headers and JSON payloads through existing `header_info` in `src/aimx/commands/query.py` +- [X] T017 [US1] Preserve query option validation and target-specific restrictions for params, images, and metrics in `src/aimx/commands/query.py` +- [X] T018 [US1] Run `uv run pytest tests/unit/test_query_helpers.py tests/contract/test_query_contract.py tests/integration/test_query_command.py -q` and fix US1 failures in `src/aimx/commands/query.py`, `src/aimx/rendering/query_views.py`, and `src/aimx/rendering/params_views.py` + +**Checkpoint**: User Story 1 is fully functional and independently testable. + +--- + +## Phase 4: User Story 2 - Trace Runs Without Typing The Default Expression (Priority: P1) + +**Goal**: Users can run `aimx trace` and `aimx trace distribution` without a +query-language expression, and each command uses `run.hash != ''` as the +effective expression. + +**Independent Test**: Run `uv run aimx trace --repo data --head 5` and +`uv run aimx trace distribution --repo data --head 2 --no-color`; confirm both +behave as though `"run.hash != ''"` had been supplied. + +### Tests for User Story 2 + +- [X] T019 [P] [US2] Add contract tests for omitted-expression metric trace default plot, JSON mode, and explicit-expression preservation in `tests/contract/test_trace_contract.py` +- [X] T020 [P] [US2] Add contract tests for omitted-expression distribution trace default visual mode and explicit `--table`, `--csv`, and `--json` modes in `tests/contract/test_trace_contract.py` +- [X] T021 [P] [US2] Add integration tests comparing omitted-expression trace commands to explicit `run.hash != ''` commands against `data` in `tests/integration/test_trace_command.py` +- [X] T022 [P] [US2] Add unit tests proving option-first trace invocations preserve existing option parsing for `--repo`, `--json`, `--table`, `--csv`, `--steps`, `--head`, `--tail`, `--every`, `--width`, `--height`, `--no-color`, and `--step` in `tests/unit/test_trace_helpers.py` + +### Implementation for User Story 2 + +- [X] T023 [US2] Update trace usage and validation messages so metric and distribution expressions are optional while preserving the distribution subtarget in `src/aimx/commands/trace.py` +- [X] T024 [US2] Ensure `_execute_trace_pipeline()` receives the effective default expression for metric and distribution collectors in `src/aimx/commands/trace.py` +- [X] T025 [US2] Preserve trace no-match, no-data-in-step-range, invalid expression, invalid repo, and explicit mode behavior in `src/aimx/commands/trace.py` +- [X] T026 [US2] Run `uv run pytest tests/unit/test_trace_helpers.py tests/contract/test_trace_contract.py tests/integration/test_trace_command.py -q` and fix US2 failures in `src/aimx/commands/trace.py` and `src/aimx/rendering/trace_views.py` + +**Checkpoint**: User Stories 1 and 2 both work independently. + +--- + +## Phase 5: User Story 3 - Compare Params With Run Duration Visible (Priority: P2) + +**Goal**: `aimx query params` displays each returned run's duration in rich, +plain, and JSON output, while keeping runs visible when duration is unavailable. + +**Independent Test**: Run `uv run aimx query params --repo data`, +`uv run aimx query params --repo data --plain`, and +`uv run aimx query params --repo data --json`; confirm every returned run shows +a calculated duration or explicit unavailable/running status. + +### Tests for User Story 3 + +- [X] T027 [P] [US3] Add unit tests for params duration formatting in rich/plain renderers and JSON serialization in `tests/unit/test_run_params.py` +- [X] T028 [P] [US3] Add contract tests for `query params --json` duration object shape and rich/plain duration visibility in `tests/contract/test_query_contract.py` +- [X] T029 [P] [US3] Add integration tests for sample-repo params duration output with omitted expression and explicit expression in `tests/integration/test_query_command.py` +- [X] T030 [P] [US3] Add unit tests proving selected params and missing params remain independent from duration metadata in `tests/unit/test_run_params.py` + +### Implementation for User Story 3 + +- [X] T031 [US3] Extend `RunParams` to carry `RunDuration` without changing selected-key or missing-key semantics in `src/aimx/aim_bridge/run_params.py` +- [X] T032 [US3] Populate `RunDuration` during `collect_run_params()` from raw Aim run timing attributes before sorting rows in `src/aimx/aim_bridge/run_params.py` +- [X] T033 [US3] Add duration display helpers and a `DURATION` column to params rich output in `src/aimx/rendering/params_views.py` +- [X] T034 [US3] Add duration cells to params plain output and stable `duration` objects to params JSON output in `src/aimx/rendering/params_views.py` +- [X] T035 [US3] Run `uv run pytest tests/unit/test_run_params.py tests/contract/test_query_contract.py tests/integration/test_query_command.py -q` and fix US3 failures in `src/aimx/aim_bridge/run_params.py` and `src/aimx/rendering/params_views.py` + +**Checkpoint**: All user stories are independently functional. + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +**Purpose**: Complete discoverability, safety validation, and full regression +coverage across the feature. + +- [X] T036 [P] Update owned-command help text with omitted-expression defaults for query and trace examples in `src/aimx/commands/help.py` +- [X] T037 [P] Update README query and trace examples to show omitted-expression forms and params duration output in `README.md` +- [X] T038 [P] Update quickstart verification notes if implementation behavior differs from planned examples in `specs/006-default-query-runtime/quickstart.md` +- [X] T039 Run quickstart sections 2-6 manually and record any deviations in `specs/006-default-query-runtime/quickstart.md` +- [X] T040 Run passthrough and owned-command regression tests with `uv run pytest tests/contract/test_cli_contract.py tests/integration/test_passthrough_behavior.py tests/integration/test_missing_native_aim.py tests/integration/test_missing_python_aim_package.py -q` and fix regressions in `src/aimx/router.py`, `src/aimx/cli.py`, `src/aimx/commands/query.py`, or `src/aimx/commands/trace.py` +- [X] T041 Run the full suite with `uv run pytest -q` and fix regressions in touched files under `src/aimx/` and `tests/` +- [X] T042 Update final implementation verification notes in `specs/006-default-query-runtime/checklists/requirements.md` + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Phase 1 Setup**: No dependencies; can start immediately. +- **Phase 2 Foundational**: Depends on Phase 1; blocks every user story. +- **Phase 3 US1**: Depends on Phase 2; MVP scope. +- **Phase 4 US2**: Depends on Phase 2; can proceed after shared default-expression parsing is stable. +- **Phase 5 US3**: Depends on Phase 2; can proceed after params collection and rendering paths from existing features are understood. +- **Phase 6 Polish**: Depends on whichever user stories are included in the delivery. + +### User Story Dependencies + +- **US1 (P1)**: First independently valuable slice for query commands; no dependency on US2 or US3. +- **US2 (P1)**: First independently valuable slice for trace commands; no dependency on US1 except the shared default-expression constant from Phase 2. +- **US3 (P2)**: Enhances params output with duration metadata; no dependency on US2 and can be validated independently through `query params`. + +### Within Each User Story + +- Write tests first and confirm they fail for the missing behavior. +- Parser behavior before command dispatch assertions. +- Bridge/data extraction before renderer assertions for params duration. +- Story-specific pytest command before moving to the next priority. + +--- + +## Parallel Opportunities + +- T002, T003, and T004 can run in parallel after T001 ownership is clear. +- T005, T006, and T007 can run in parallel because they touch different test files. +- T012, T013, and T014 can run in parallel for US1 test coverage. +- T019, T020, T021, and T022 can run in parallel for US2 test coverage. +- T027, T028, T029, and T030 can run in parallel for US3 test coverage. +- T036, T037, and T038 can run in parallel during polish because they touch different documentation files. + +--- + +## Parallel Example: User Story 1 + +```text +Task: "T012 [P] [US1] Add contract tests for omitted-expression `query metrics`, `query images`, and `query params` JSON/rich output effective expression in tests/contract/test_query_contract.py" +Task: "T013 [P] [US1] Add integration tests comparing omitted-expression query commands to explicit `run.hash != ''` commands against `data` and `data/.aim` in tests/integration/test_query_command.py" +Task: "T014 [P] [US1] Add unit tests proving option-first query invocations preserve existing option parsing for `--repo`, `--json`, `--plain`, `--steps`, `--epochs`, `--head`, `--tail`, `--every`, `--max-images`, and `--param` in tests/unit/test_query_helpers.py" +``` + +## Parallel Example: User Story 2 + +```text +Task: "T019 [P] [US2] Add contract tests for omitted-expression metric trace default plot, JSON mode, and explicit-expression preservation in tests/contract/test_trace_contract.py" +Task: "T020 [P] [US2] Add contract tests for omitted-expression distribution trace default visual mode and explicit `--table`, `--csv`, and `--json` modes in tests/contract/test_trace_contract.py" +Task: "T021 [P] [US2] Add integration tests comparing omitted-expression trace commands to explicit `run.hash != ''` commands against `data` in tests/integration/test_trace_command.py" +Task: "T022 [P] [US2] Add unit tests proving option-first trace invocations preserve existing option parsing for `--repo`, `--json`, `--table`, `--csv`, `--steps`, `--head`, `--tail`, `--every`, `--width`, `--height`, `--no-color`, and `--step` in tests/unit/test_trace_helpers.py" +``` + +## Parallel Example: User Story 3 + +```text +Task: "T027 [P] [US3] Add unit tests for params duration formatting in rich/plain renderers and JSON serialization in tests/unit/test_run_params.py" +Task: "T028 [P] [US3] Add contract tests for `query params --json` duration object shape and rich/plain duration visibility in tests/contract/test_query_contract.py" +Task: "T029 [P] [US3] Add integration tests for sample-repo params duration output with omitted expression and explicit expression in tests/integration/test_query_command.py" +Task: "T030 [P] [US3] Add unit tests proving selected params and missing params remain independent from duration metadata in tests/unit/test_run_params.py" +``` + +--- + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete Phase 1 setup. +2. Complete Phase 2 foundational parser and duration primitives. +3. Complete Phase 3 US1. +4. Stop and validate `uv run aimx query metrics --repo data`, `uv run aimx query images --repo data --plain`, and `uv run aimx query params --repo data`. +5. Run US1 unit, contract, and integration tests before adding trace defaults or params duration rendering. + +### Incremental Delivery + +1. US1: deliver omitted-expression defaults for query commands. +2. US2: deliver omitted-expression defaults for trace commands. +3. US3: add params duration metadata and display. +4. Polish: update docs, run quickstart, and run full regression suite. + +### Multi-Developer Coordination + +- One developer owns `src/aimx/commands/query.py` while US1 parser and dispatch tasks are active. +- One developer owns `src/aimx/commands/trace.py` while US2 parser and dispatch tasks are active. +- One developer owns `src/aimx/aim_bridge/run_params.py`, `src/aimx/rendering/params_views.py`, and `tests/unit/test_run_params.py` while US3 duration tasks are active. +- Contract and integration tests can be split by `tests/contract/test_query_contract.py`, `tests/contract/test_trace_contract.py`, `tests/integration/test_query_command.py`, and `tests/integration/test_trace_command.py`. + +--- + +## Notes + +- `[P]` means the task can be parallelized only after its stated phase dependencies are satisfied. +- Story labels map directly to the spec user stories. +- Keep every command read-only; do not call Aim mutation APIs such as `run.set`, `track`, artifact logging, migration, or repair operations. +- Preserve existing `query metrics`, `query images`, `query params`, `trace`, `trace distribution`, and native Aim passthrough contracts throughout implementation. +- Commit after each phase or a small coherent task group when using the git hook workflow. diff --git a/src/aimx/aim_bridge/run_params.py b/src/aimx/aim_bridge/run_params.py index c4203de..1b49deb 100644 --- a/src/aimx/aim_bridge/run_params.py +++ b/src/aimx/aim_bridge/run_params.py @@ -2,21 +2,64 @@ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path from typing import Any from aimx.aim_bridge.metric_stats import RunMeta, _extract_run_meta +@dataclass(frozen=True) +class RunDuration: + seconds: float | None = None + status: str = "unavailable" + source: str = "missing_metadata" + + @dataclass(frozen=True) class RunParams: run: RunMeta params: dict[str, Any] + duration: RunDuration = field(default_factory=RunDuration) selected_keys: tuple[str, ...] = () missing_keys: tuple[str, ...] = () +def _numeric_seconds(value: Any) -> float | None: + if value is None or isinstance(value, bool): + return None + try: + seconds = float(value) + except (TypeError, ValueError): + return None + if seconds < 0: + return None + return seconds + + +def extract_run_duration(run: Any) -> RunDuration: + end_time = _numeric_seconds(getattr(run, "end_time", None)) + creation_time = _numeric_seconds(getattr(run, "creation_time", None)) + if creation_time is not None and end_time is None: + return RunDuration(status="running") + + duration = _numeric_seconds(getattr(run, "duration", None)) + if duration is not None: + return RunDuration(seconds=duration, status="available", source="duration") + + if end_time is not None and creation_time is not None: + seconds = end_time - creation_time + if seconds >= 0: + return RunDuration( + seconds=seconds, + status="available", + source="end_time_minus_creation_time", + ) + return RunDuration() + + return RunDuration() + + def flatten_params(params: dict[str, Any], *, prefix: str = "") -> dict[str, Any]: """Flatten nested dictionaries into stable dotted parameter keys.""" flattened: dict[str, Any] = {} @@ -99,6 +142,7 @@ def collect_run_params( RunParams( run=_extract_run_meta(run), params=params, + duration=extract_run_duration(run), selected_keys=selected_keys, missing_keys=missing, ) diff --git a/src/aimx/commands/help.py b/src/aimx/commands/help.py index 6eedb9e..c14fe5e 100644 --- a/src/aimx/commands/help.py +++ b/src/aimx/commands/help.py @@ -11,7 +11,7 @@ def render_help() -> str: " version Show the aimx version and detected native Aim version", " doctor Show native Aim availability and passthrough readiness", " query Query metrics, images, or run params from a local Aim repository", - " Usage: aimx query [--repo ]", + " Usage: aimx query [] [--repo ]", " Options: --json --oneline --no-color --verbose", " --steps start:end | --epochs start:end (mutually exclusive)", " --head N --tail N --every K", @@ -19,21 +19,29 @@ def render_help() -> str: " --param KEY (params only, repeatable selected parameter)", " Repo defaults to the current directory; paths may point at either", " the repo root or its .aim directory", + " If expression is omitted, aimx uses: run.hash != ''", " Short run hashes in the expression are transparently expanded.", + " Params output includes run duration when available.", + " Example: aimx query metrics --repo data", + " Example: aimx query images --repo data --plain", + " Example: aimx query params --repo data --json", " Example: aimx query metrics \"run.hash=='eca37394'\" --repo data", " Example: aimx query images \"images\" --repo data --epochs 10:50", " Example: aimx query images \"images\" --repo data --head 10", " Example: aimx query params \"run.experiment=='cloud-segmentation'\" --repo data --param hparam.lr", " trace Plot metric time-series or distribution histograms from a local Aim repository", - " Usage: aimx trace [--repo ]", - " Usage: aimx trace distribution [--repo ]", + " Usage: aimx trace [] [--repo ]", + " Usage: aimx trace distribution [] [--repo ]", " Options: --table --csv --json", " --steps start:end (e.g. --steps 100:500, :50, 100:)", " --step N (distribution visual mode only; nearest tracked step)", " --head N --tail N --every K", " --width W --height H --no-color", " Repo defaults to the current directory.", + " If expression is omitted, aimx uses: run.hash != ''", " Short run hashes in the expression are transparently expanded.", + " Example: aimx trace --repo data --head 5", + " Example: aimx trace distribution --repo data --head 2 --no-color", " Example: aimx trace \"metric.name=='loss'\" --repo data --steps 100:500", " Example: aimx trace distribution \"distribution.name != ''\" --repo data --step 12300", " Example: aimx trace distribution \"distribution.name=='weights'\" --repo data --table", diff --git a/src/aimx/commands/query.py b/src/aimx/commands/query.py index b942ed3..599e3fe 100644 --- a/src/aimx/commands/query.py +++ b/src/aimx/commands/query.py @@ -6,6 +6,7 @@ from typing import Any SUPPORTED_TARGETS = {"metrics", "images", "params"} +DEFAULT_QUERY_EXPRESSION = "run.hash != ''" PARAMS_UNSUPPORTED_QUERY_FLAGS = { "--steps", "--epochs", @@ -36,7 +37,7 @@ class QueryInvocation: def __post_init__(self) -> None: if self.target not in SUPPORTED_TARGETS: raise ValueError( - f"Unsupported query target: {self.target}. Supported targets: metrics, images." + f"Unsupported query target: {self.target}. Supported targets: metrics, images, params." ) if not self.expression.strip(): raise ValueError("Query expression must not be empty.") @@ -90,9 +91,9 @@ def _parse_non_negative_int(flag: str, raw: str) -> int: def parse_query_invocation(args: list[str]) -> QueryInvocation: - if len(args) < 2: + if not args: raise ValueError( - "Usage: aimx query [--repo ] " + "Usage: aimx query [] [--repo ] " "[--json] [--oneline] [--no-color] [--verbose] " "[--steps start:end | --epochs start:end] " "[--head N] [--tail N] [--every K] " @@ -100,8 +101,12 @@ def parse_query_invocation(args: list[str]) -> QueryInvocation: ) target = args[0] - expression = args[1] - rest = args[2:] + if len(args) >= 2 and not args[1].startswith("-"): + expression = args[1].strip() or DEFAULT_QUERY_EXPRESSION + rest = args[2:] + else: + expression = DEFAULT_QUERY_EXPRESSION + rest = args[1:] output_json = False plain = False diff --git a/src/aimx/commands/trace.py b/src/aimx/commands/trace.py index bc014dd..516f324 100644 --- a/src/aimx/commands/trace.py +++ b/src/aimx/commands/trace.py @@ -6,7 +6,11 @@ from pathlib import Path from typing import Any, Literal -from aimx.commands.query import QueryCommandResult, normalize_repo_path +from aimx.commands.query import ( + DEFAULT_QUERY_EXPRESSION, + QueryCommandResult, + normalize_repo_path, +) _MODES = {"plot", "table", "csv", "json"} @@ -37,28 +41,23 @@ class TraceInvocation: def parse_trace_invocation(args: list[str]) -> TraceInvocation: - if len(args) < 1: - raise ValueError( - "Usage: aimx trace [distribution] [--repo ] [--table|--csv|--json]" - " [--steps start:end] [--head N] [--tail N] [--every K]" - " [--width W] [--height H] [--no-color]" - ) - target: Literal["metrics", "distribution"] = "metrics" expression: str | None = None rest = args - if args[0] == "distribution": + if args and args[0] == "distribution": target = "distribution" - if len(args) < 2: - raise ValueError( - "Usage: aimx trace distribution [--repo ] [--table|--csv|--json]" - " [--steps start:end] [--head N] [--tail N] [--every K] [--no-color]" - ) - expression = args[1] - rest = args[2:] - else: - expression = args[0] + if len(args) >= 2 and not args[1].startswith("-"): + expression = args[1].strip() or DEFAULT_QUERY_EXPRESSION + rest = args[2:] + else: + expression = DEFAULT_QUERY_EXPRESSION + rest = args[1:] + elif args and not args[0].startswith("-"): + expression = args[0].strip() or DEFAULT_QUERY_EXPRESSION rest = args[1:] + else: + expression = DEFAULT_QUERY_EXPRESSION + rest = args mode: Literal["plot", "table", "csv", "json"] = "plot" repo_value = "." diff --git a/src/aimx/rendering/params_views.py b/src/aimx/rendering/params_views.py index d061747..f101b36 100644 --- a/src/aimx/rendering/params_views.py +++ b/src/aimx/rendering/params_views.py @@ -10,7 +10,7 @@ from rich.console import Console from rich.table import Table -from aimx.aim_bridge.run_params import RunParams, default_param_keys +from aimx.aim_bridge.run_params import RunDuration, RunParams, default_param_keys from aimx.rendering import colors DEFAULT_PARAM_COLUMN_LIMIT = 6 @@ -42,6 +42,32 @@ def _jsonable(value: Any) -> Any: return str(value) +def _display_duration(duration: RunDuration) -> str: + if duration.status != "available" or duration.seconds is None: + return duration.status + + seconds = max(0.0, duration.seconds) + if seconds < 1: + text = f"{seconds:.3f}".rstrip("0").rstrip(".") + return f"{text}s" + whole = int(seconds) + if whole < 60: + return f"{whole}s" + minutes, sec = divmod(whole, 60) + if minutes < 60: + return f"{minutes}m{sec:02d}s" + hours, minute = divmod(minutes, 60) + return f"{hours}h{minute:02d}m{sec:02d}s" + + +def _duration_json(duration: RunDuration) -> dict[str, Any]: + return { + "seconds": duration.seconds if duration.status == "available" else None, + "status": duration.status, + "source": duration.source, + } + + def _keys_for_display(rows: list[RunParams], limit: int | None = DEFAULT_PARAM_COLUMN_LIMIT) -> tuple[tuple[str, ...], int]: selected = next((row.selected_keys for row in rows if row.selected_keys), ()) keys = selected or default_param_keys(rows) @@ -56,7 +82,7 @@ def render_params_rich_table( *, no_color: bool = False, ) -> str: - width = 120 if no_color else shutil.get_terminal_size(fallback=(120, 24)).columns + width = 160 if no_color else shutil.get_terminal_size(fallback=(160, 24)).columns buf = io.StringIO() console = Console( file=buf, @@ -91,8 +117,9 @@ def render_params_rich_table( table.add_column("RUN", style=colors.RUN_HASH, no_wrap=True) table.add_column("EXPERIMENT", style=colors.EXPERIMENT, no_wrap=True) table.add_column("NAME", style=colors.METRIC_NAME, no_wrap=True) + table.add_column("DURATION", justify="right", no_wrap=True) for key in keys: - table.add_column(key, style=colors.CONTEXT_VAL) + table.add_column(key, style=colors.CONTEXT_VAL, no_wrap=True) if not keys: table.add_column("PARAMS", style=colors.CONTEXT_VAL) @@ -101,6 +128,7 @@ def render_params_rich_table( _short_hash(row.run.hash), row.run.experiment or "", row.run.name or "", + _display_duration(row.duration), ] if keys: for key in keys: @@ -125,6 +153,7 @@ def render_params_oneline(rows: list[RunParams], header_info: dict[str, Any]) -> _short_hash(row.run.hash), row.run.experiment or "", row.run.name or "", + _display_duration(row.duration), ] for key in keys: value = _display(row.params[key]) if key in row.params else "-" @@ -151,6 +180,7 @@ def render_params_json(rows: list[RunParams], header_info: dict[str, Any]) -> st "hash": row.run.hash, "experiment": row.run.experiment, "name": row.run.name, + "duration": _duration_json(row.duration), "params": _jsonable(row.params), "missing_params": list(row.missing_keys), } diff --git a/tests/contract/test_query_contract.py b/tests/contract/test_query_contract.py index e6a1594..59a1c40 100644 --- a/tests/contract/test_query_contract.py +++ b/tests/contract/test_query_contract.py @@ -1,10 +1,9 @@ from __future__ import annotations import json -import sys -from unittest.mock import patch from aimx.__main__ import main +from aimx.commands.query import DEFAULT_QUERY_EXPRESSION def test_query_metrics_json_contract_uses_nested_runs_envelope(capfd, sample_repo_root) -> None: @@ -54,6 +53,56 @@ def test_query_metrics_text_contract_reports_repo_count_and_metric_name( assert "loss" in captured.out +def test_query_metrics_json_contract_reports_default_expression_when_omitted( + capfd, sample_repo_root +) -> None: + exit_code = main(["query", "metrics", "--repo", str(sample_repo_root), "--json"]) + + captured = capfd.readouterr() + payload = json.loads(captured.out) + assert exit_code == 0 + assert payload["target"] == "metrics" + assert payload["expression"] == DEFAULT_QUERY_EXPRESSION + assert payload["metrics_count"] > 0 + + +def test_query_images_json_contract_reports_default_expression_when_omitted( + capfd, sample_repo_root +) -> None: + exit_code = main(["query", "images", "--repo", str(sample_repo_root), "--json"]) + + captured = capfd.readouterr() + payload = json.loads(captured.out) + assert exit_code == 0 + assert payload["target"] == "images" + assert payload["expression"] == DEFAULT_QUERY_EXPRESSION + assert payload["count"] > 0 + + +def test_query_params_rich_contract_reports_default_expression_when_omitted( + capfd, sample_repo_root +) -> None: + exit_code = main(["query", "params", "--repo", str(sample_repo_root)]) + + captured = capfd.readouterr() + assert exit_code == 0 + assert DEFAULT_QUERY_EXPRESSION in captured.out + assert "params where" in captured.out + + +def test_query_params_json_contract_reports_default_expression_when_omitted( + capfd, sample_repo_root +) -> None: + exit_code = main(["query", "params", "--repo", str(sample_repo_root), "--json"]) + + captured = capfd.readouterr() + payload = json.loads(captured.out) + assert exit_code == 0 + assert payload["target"] == "params" + assert payload["expression"] == DEFAULT_QUERY_EXPRESSION + assert payload["runs_count"] > 0 + + def test_query_metrics_oneline_contract_is_tab_separated_and_contains_metric_name( capfd, sample_repo_root ) -> None: @@ -70,7 +119,7 @@ def test_query_metrics_oneline_contract_is_tab_separated_and_contains_metric_nam captured = capfd.readouterr() assert exit_code == 0 - lines = [l for l in captured.out.splitlines() if l.strip()] + lines = [line for line in captured.out.splitlines() if line.strip()] assert lines, "Expected at least one output line" assert "loss" in captured.out assert "\t" in lines[0] @@ -108,10 +157,31 @@ def test_query_params_json_contract_uses_stable_envelope(capfd, sample_repo_root assert "hash" in first_run assert "experiment" in first_run assert "name" in first_run + assert "duration" in first_run assert "params" in first_run assert "missing_params" in first_run +def test_query_params_json_contract_includes_duration_object( + capfd, sample_repo_root +) -> None: + exit_code = main( + ["query", "params", "--repo", str(sample_repo_root), "--json"] + ) + + captured = capfd.readouterr() + payload = json.loads(captured.out) + assert exit_code == 0 + assert payload["runs"] + duration = payload["runs"][0]["duration"] + assert set(duration) == {"seconds", "status", "source"} + assert duration["status"] in {"available", "running", "unavailable"} + if duration["status"] == "available": + assert isinstance(duration["seconds"], (int, float)) + else: + assert duration["seconds"] is None + + def test_query_params_text_contract_reports_repo_count_and_params( capfd, sample_repo_root ) -> None: @@ -124,6 +194,23 @@ def test_query_params_text_contract_reports_repo_count_and_params( assert "hparam.lr" in captured.out +def test_query_params_text_contract_reports_duration( + capfd, sample_repo_root +) -> None: + rich_exit = main(["query", "params", "--repo", str(sample_repo_root)]) + rich_captured = capfd.readouterr() + assert rich_exit == 0 + assert "DURATION" in rich_captured.out + + plain_exit = main(["query", "params", "--repo", str(sample_repo_root), "--plain"]) + plain_captured = capfd.readouterr() + assert plain_exit == 0 + first_line = next(line for line in plain_captured.out.splitlines() if line.strip()) + cells = first_line.split("\t") + assert len(cells) >= 5 + assert cells[4] + + def test_query_params_json_contract_honors_selected_params( capfd, sample_repo_root ) -> None: @@ -334,21 +421,21 @@ def _get_plain(extra_args: list[str]) -> str: def test_missing_max_images_value_exits_with_code_2(capfd) -> None: """T018 (contract): --max-images with no value → exit 2.""" exit_code = main(["query", "images", "images", "--max-images"]) - captured = capfd.readouterr() + capfd.readouterr() assert exit_code == 2 def test_negative_max_images_exits_with_code_2(capfd) -> None: """T018 (contract): negative --max-images → exit 2.""" exit_code = main(["query", "images", "images", "--max-images", "-1"]) - captured = capfd.readouterr() + capfd.readouterr() assert exit_code == 2 def test_non_integer_max_images_exits_with_code_2(capfd) -> None: """T018 (contract): non-integer --max-images → exit 2.""" exit_code = main(["query", "images", "images", "--max-images", "abc"]) - captured = capfd.readouterr() + capfd.readouterr() assert exit_code == 2 diff --git a/tests/contract/test_trace_contract.py b/tests/contract/test_trace_contract.py index 93abf2f..d2210b7 100644 --- a/tests/contract/test_trace_contract.py +++ b/tests/contract/test_trace_contract.py @@ -9,7 +9,7 @@ from aimx.__main__ import main from aimx.aim_bridge.metric_stats import DistributionSeries, collect_distribution_series -from aimx.commands.query import normalize_repo_path +from aimx.commands.query import DEFAULT_QUERY_EXPRESSION, normalize_repo_path def _require_distribution_series(sample_repo_root: Path) -> list[DistributionSeries]: @@ -66,6 +66,32 @@ def test_trace_json_contract_has_steps_and_values_arrays(capfd, sample_repo_root assert isinstance(first["values"], list) +def test_trace_json_contract_omitted_expression_matches_explicit_default( + capfd, sample_repo_root +) -> None: + explicit_exit = main( + ["trace", DEFAULT_QUERY_EXPRESSION, "--repo", str(sample_repo_root), "--json"] + ) + explicit_payload = json.loads(capfd.readouterr().out) + + omitted_exit = main(["trace", "--repo", str(sample_repo_root), "--json"]) + omitted_payload = json.loads(capfd.readouterr().out) + + assert explicit_exit == 0 + assert omitted_exit == 0 + assert omitted_payload == explicit_payload + + +def test_trace_plot_contract_omitted_expression_produces_output( + capfd, sample_repo_root +) -> None: + exit_code = main(["trace", "--repo", str(sample_repo_root), "--head", "5"]) + + captured = capfd.readouterr() + assert exit_code == 0 + assert captured.out.strip() + + def test_trace_csv_contract_has_correct_headers(capfd, sample_repo_root) -> None: exit_code = main( ["trace", "metric.name == 'loss'", "--repo", str(sample_repo_root), "--csv"] @@ -122,6 +148,43 @@ def test_trace_distribution_default_visual_contract(capfd, sample_repo_root: Pat assert not captured.err +def test_trace_distribution_default_visual_omitted_expression_matches_explicit_default( + capfd, sample_repo_root: Path +) -> None: + _require_distribution_series(sample_repo_root) + + explicit_exit = main( + [ + "trace", + "distribution", + DEFAULT_QUERY_EXPRESSION, + "--repo", + str(sample_repo_root), + "--head", + "2", + "--no-color", + ] + ) + explicit_output = capfd.readouterr().out + + omitted_exit = main( + [ + "trace", + "distribution", + "--repo", + str(sample_repo_root), + "--head", + "2", + "--no-color", + ] + ) + omitted_output = capfd.readouterr().out + + assert explicit_exit == 0 + assert omitted_exit == 0 + assert omitted_output == explicit_output + + def test_trace_distribution_step_missing_value_reports_error( capfd, sample_repo_root: Path ) -> None: @@ -226,3 +289,45 @@ def test_trace_distribution_explicit_modes_exclude_visual_sections( assert payload assert "points" in payload[0] assert "Heatmap (steps x bins)" not in json_output + + +@pytest.mark.parametrize("mode", ["--table", "--csv", "--json"]) +def test_trace_distribution_explicit_modes_omitted_expression_match_default( + capfd, + sample_repo_root: Path, + mode: str, +) -> None: + _require_distribution_series(sample_repo_root) + + explicit_exit = main( + [ + "trace", + "distribution", + DEFAULT_QUERY_EXPRESSION, + "--repo", + str(sample_repo_root), + mode, + "--head", + "1", + "--no-color", + ] + ) + explicit_output = capfd.readouterr().out + + omitted_exit = main( + [ + "trace", + "distribution", + "--repo", + str(sample_repo_root), + mode, + "--head", + "1", + "--no-color", + ] + ) + omitted_output = capfd.readouterr().out + + assert explicit_exit == 0 + assert omitted_exit == 0 + assert omitted_output == explicit_output diff --git a/tests/integration/test_query_command.py b/tests/integration/test_query_command.py index 0cb70e0..b48e270 100644 --- a/tests/integration/test_query_command.py +++ b/tests/integration/test_query_command.py @@ -1,15 +1,13 @@ from __future__ import annotations -import io import json from typing import Any from unittest.mock import patch -import pytest - from aimx.__main__ import main from aimx.aim_bridge.metric_stats import RunMeta -from aimx.rendering.image_render import ImageRenderPlan, TerminalCapability, plan_render, render_inline +from aimx.commands.query import DEFAULT_QUERY_EXPRESSION +from aimx.rendering.image_render import TerminalCapability, plan_render, render_inline def test_metric_query_accepts_repo_root_and_dot_aim_paths( @@ -89,7 +87,7 @@ def test_metric_query_oneline_mode_returns_tab_separated_rows( captured = capfd.readouterr() assert exit_code == 0 - lines = [l for l in captured.out.splitlines() if l.strip()] + lines = [line for line in captured.out.splitlines() if line.strip()] assert lines assert "loss" in captured.out assert "\t" in lines[0] @@ -124,6 +122,29 @@ def test_metric_query_json_mode_returns_nested_structure( assert last_val is not None +def test_omitted_metric_query_matches_explicit_default_expression( + capfd, sample_repo_root +) -> None: + explicit_exit = main( + [ + "query", + "metrics", + DEFAULT_QUERY_EXPRESSION, + "--repo", + str(sample_repo_root), + "--json", + ] + ) + explicit_payload = json.loads(capfd.readouterr().out) + + omitted_exit = main(["query", "metrics", "--repo", str(sample_repo_root), "--json"]) + omitted_payload = json.loads(capfd.readouterr().out) + + assert explicit_exit == 0 + assert omitted_exit == 0 + assert omitted_payload == explicit_payload + + def test_image_query_returns_matches_from_sample_repository(capfd, sample_repo_root) -> None: exit_code = main( ["query", "images", "images", "--repo", str(sample_repo_root), "--json"] @@ -136,6 +157,29 @@ def test_image_query_returns_matches_from_sample_repository(capfd, sample_repo_r assert payload["rows"][0]["name"] == "example" +def test_omitted_image_query_matches_explicit_default_expression( + capfd, sample_repo_root +) -> None: + explicit_exit = main( + [ + "query", + "images", + DEFAULT_QUERY_EXPRESSION, + "--repo", + str(sample_repo_root), + "--json", + ] + ) + explicit_payload = json.loads(capfd.readouterr().out) + + omitted_exit = main(["query", "images", "--repo", str(sample_repo_root), "--json"]) + omitted_payload = json.loads(capfd.readouterr().out) + + assert explicit_exit == 0 + assert omitted_exit == 0 + assert omitted_payload == explicit_payload + + def test_params_query_accepts_repo_root_and_dot_aim_paths( capfd, sample_repo_root, sample_repo_dot_aim ) -> None: @@ -157,6 +201,29 @@ def test_params_query_accepts_repo_root_and_dot_aim_paths( assert root_payload["param_keys"] == dot_aim_payload["param_keys"] +def test_omitted_params_query_matches_explicit_default_expression_for_dot_aim_path( + capfd, sample_repo_dot_aim +) -> None: + explicit_exit = main( + [ + "query", + "params", + DEFAULT_QUERY_EXPRESSION, + "--repo", + str(sample_repo_dot_aim), + "--json", + ] + ) + explicit_payload = json.loads(capfd.readouterr().out) + + omitted_exit = main(["query", "params", "--repo", str(sample_repo_dot_aim), "--json"]) + omitted_payload = json.loads(capfd.readouterr().out) + + assert explicit_exit == 0 + assert omitted_exit == 0 + assert omitted_payload == explicit_payload + + def test_params_query_returns_matches_from_sample_repository( capfd, sample_repo_root ) -> None: @@ -169,6 +236,36 @@ def test_params_query_returns_matches_from_sample_repository( assert "hparam.lr" in captured.out +def test_params_query_duration_visible_with_omitted_and_explicit_expression( + capfd, sample_repo_root +) -> None: + omitted_exit = main(["query", "params", "--repo", str(sample_repo_root), "--json"]) + omitted_payload = json.loads(capfd.readouterr().out) + + explicit_exit = main( + [ + "query", + "params", + DEFAULT_QUERY_EXPRESSION, + "--repo", + str(sample_repo_root), + "--json", + ] + ) + explicit_payload = json.loads(capfd.readouterr().out) + + assert omitted_exit == 0 + assert explicit_exit == 0 + assert omitted_payload["runs"] + assert explicit_payload["runs"] + assert omitted_payload["runs"][0]["duration"] == explicit_payload["runs"][0]["duration"] + assert omitted_payload["runs"][0]["duration"]["status"] in { + "available", + "running", + "unavailable", + } + + def test_params_query_zero_matches_succeeds(capfd, sample_repo_root) -> None: exit_code = main( [ diff --git a/tests/integration/test_trace_command.py b/tests/integration/test_trace_command.py index 0d88a52..21ecef2 100644 --- a/tests/integration/test_trace_command.py +++ b/tests/integration/test_trace_command.py @@ -9,7 +9,7 @@ from aimx.__main__ import main from aimx.aim_bridge.metric_stats import DistributionSeries, collect_distribution_series -from aimx.commands.query import normalize_repo_path +from aimx.commands.query import DEFAULT_QUERY_EXPRESSION, normalize_repo_path def _require_distribution_series(sample_repo_root: Path) -> list[DistributionSeries]: @@ -102,6 +102,22 @@ def test_trace_json_mode_returns_full_value_arrays(capfd, sample_repo_root) -> N assert len(series["values"]) > 0 +def test_omitted_trace_query_matches_explicit_default_expression( + capfd, sample_repo_root +) -> None: + explicit_exit = main( + ["trace", DEFAULT_QUERY_EXPRESSION, "--repo", str(sample_repo_root), "--json"] + ) + explicit_payload = json.loads(capfd.readouterr().out) + + omitted_exit = main(["trace", "--repo", str(sample_repo_root), "--json"]) + omitted_payload = json.loads(capfd.readouterr().out) + + assert explicit_exit == 0 + assert omitted_exit == 0 + assert omitted_payload == explicit_payload + + def test_trace_csv_mode_contains_correct_fields(capfd, sample_repo_root) -> None: exit_code = main( ["trace", "metric.name == 'loss'", "--repo", str(sample_repo_root), "--csv"] @@ -303,6 +319,35 @@ def test_trace_distribution_json_mode_preserves_series_payload_with_step( assert "points" in payload[0] +def test_omitted_distribution_trace_matches_explicit_default_expression( + capfd, sample_repo_root: Path +) -> None: + _require_distribution_series(sample_repo_root) + + explicit_exit = main( + [ + "trace", + "distribution", + DEFAULT_QUERY_EXPRESSION, + "--repo", + str(sample_repo_root), + "--json", + "--head", + "2", + ] + ) + explicit_payload = json.loads(capfd.readouterr().out) + + omitted_exit = main( + ["trace", "distribution", "--repo", str(sample_repo_root), "--json", "--head", "2"] + ) + omitted_payload = json.loads(capfd.readouterr().out) + + assert explicit_exit == 0 + assert omitted_exit == 0 + assert omitted_payload == explicit_payload + + def test_trace_distribution_csv_mode_preserves_rows_with_step( capfd, sample_repo_root: Path ) -> None: diff --git a/tests/unit/test_query_helpers.py b/tests/unit/test_query_helpers.py index 4189007..0e91f9f 100644 --- a/tests/unit/test_query_helpers.py +++ b/tests/unit/test_query_helpers.py @@ -5,6 +5,7 @@ import pytest from aimx.commands.query import ( + DEFAULT_QUERY_EXPRESSION, QueryInvocation, _sort_image_rows, normalize_repo_path, @@ -61,6 +62,69 @@ def test_parse_query_invocation_params_defaults() -> None: assert inv.param_keys == () +@pytest.mark.parametrize("target", ["metrics", "images", "params"]) +def test_parse_query_invocation_uses_default_expression_when_omitted(target: str) -> None: + inv = parse_query_invocation([target]) + + assert inv.target == target + assert inv.expression == DEFAULT_QUERY_EXPRESSION + + +def test_parse_query_invocation_treats_option_first_as_omitted_expression() -> None: + inv = parse_query_invocation(["params", "--repo", "data", "--json", "--param", "hparam.lr"]) + + assert inv.expression == DEFAULT_QUERY_EXPRESSION + assert inv.repo_path == Path("data") + assert inv.output_json is True + assert inv.param_keys == ("hparam.lr",) + + +@pytest.mark.parametrize( + ("target", "args", "attr", "expected"), + [ + ("metrics", ["--repo", "data"], "repo_path", Path("data")), + ("metrics", ["--json"], "output_json", True), + ("metrics", ["--plain"], "plain", True), + ("metrics", ["--steps", "1:5"], "step_slice", "1:5"), + ("images", ["--epochs", "1:5"], "epoch_slice", "1:5"), + ("metrics", ["--head", "2"], "head", 2), + ("metrics", ["--tail", "3"], "tail", 3), + ("metrics", ["--every", "4"], "every", 4), + ("images", ["--max-images", "5"], "max_images", 5), + ("params", ["--param", "hparam.lr"], "param_keys", ("hparam.lr",)), + ], +) +def test_parse_query_invocation_option_first_flags_preserve_option_parsing( + target: str, + args: list[str], + attr: str, + expected: object, +) -> None: + inv = parse_query_invocation([target, *args]) + + assert inv.expression == DEFAULT_QUERY_EXPRESSION + assert getattr(inv, attr) == expected + + +def test_parse_query_invocation_uses_default_expression_when_blank() -> None: + inv = parse_query_invocation(["metrics", " ", "--repo", "data"]) + + assert inv.expression == DEFAULT_QUERY_EXPRESSION + assert inv.repo_path == Path("data") + + +def test_parse_query_invocation_preserves_explicit_expression_precedence() -> None: + inv = parse_query_invocation(["metrics", "metric.name == 'loss'", "--repo", "data"]) + + assert inv.expression == "metric.name == 'loss'" + assert inv.repo_path == Path("data") + + +def test_parse_query_invocation_still_requires_target() -> None: + with pytest.raises(ValueError, match="Usage: aimx query"): + parse_query_invocation([]) + + def test_parse_query_invocation_params_repeated_param_keys() -> None: inv = parse_query_invocation( [ diff --git a/tests/unit/test_run_params.py b/tests/unit/test_run_params.py index dff648c..9af37ab 100644 --- a/tests/unit/test_run_params.py +++ b/tests/unit/test_run_params.py @@ -1,14 +1,37 @@ from __future__ import annotations +import json + +import pytest + from aimx.aim_bridge.metric_stats import RunMeta from aimx.aim_bridge.run_params import ( + RunDuration, RunParams, default_param_keys, + extract_run_duration, flatten_params, select_params, sort_run_params, ) -from aimx.rendering.params_views import render_params_oneline, render_params_rich_table +from aimx.rendering.params_views import ( + render_params_json, + render_params_oneline, + render_params_rich_table, +) + + +class _FakeRun: + def __init__( + self, + *, + duration: object = None, + end_time: object = None, + creation_time: object = None, + ) -> None: + self.duration = duration + self.end_time = end_time + self.creation_time = creation_time def test_flatten_params_preserves_scalar_values_with_dotted_keys() -> None: @@ -80,6 +103,65 @@ def test_sort_run_params_orders_by_experiment_name_and_hash() -> None: assert [row.run.hash for row in result] == ["bbb", "eee", "ddd", "aaa", "ccc"] +def test_extract_run_duration_prefers_duration_attribute() -> None: + duration = extract_run_duration( + _FakeRun(duration=12.5, end_time=200.0, creation_time=100.0) + ) + + assert duration.seconds == 12.5 + assert duration.status == "available" + assert duration.source == "duration" + + +def test_extract_run_duration_falls_back_to_end_minus_creation_time() -> None: + duration = extract_run_duration(_FakeRun(end_time=200.0, creation_time=100.0)) + + assert duration.seconds == 100.0 + assert duration.status == "available" + assert duration.source == "end_time_minus_creation_time" + + +@pytest.mark.parametrize( + "run", + [ + _FakeRun(duration=-1.0), + _FakeRun(end_time=100.0, creation_time=200.0), + ], +) +def test_extract_run_duration_treats_negative_values_as_unavailable(run: _FakeRun) -> None: + duration = extract_run_duration(run) + + assert duration.seconds is None + assert duration.status == "unavailable" + assert duration.source == "missing_metadata" + + +def test_extract_run_duration_treats_non_numeric_values_as_unavailable() -> None: + duration = extract_run_duration( + _FakeRun(duration="slow", end_time="later", creation_time="earlier") + ) + + assert duration.seconds is None + assert duration.status == "unavailable" + assert duration.source == "missing_metadata" + + +def test_extract_run_duration_reports_missing_metadata_as_unavailable() -> None: + duration = extract_run_duration(_FakeRun()) + + assert duration.seconds is None + assert duration.status == "unavailable" + assert duration.source == "missing_metadata" + + +def test_extract_run_duration_reports_created_run_without_end_time_as_running() -> None: + duration = extract_run_duration(_FakeRun(creation_time=100.0)) + + assert duration.seconds is None + assert duration.status == "running" + assert duration.source == "missing_metadata" + + def test_render_params_marks_runs_with_no_params() -> None: rows = [RunParams(RunMeta("abc123", "exp", "run", None), {})] header = {"target": "params", "repo": "repo", "expression": "run.hash != ''"} @@ -89,3 +171,94 @@ def test_render_params_marks_runs_with_no_params() -> None: assert "no params" in rich assert "params=-" in plain + + +def test_render_params_includes_duration_in_rich_and_plain_output() -> None: + rows = [ + RunParams( + RunMeta("abc123", "exp", "run", None), + {"hparam.lr": 0.1}, + duration=RunDuration(seconds=65.2, status="available", source="duration"), + ) + ] + header = {"target": "params", "repo": "repo", "expression": "run.hash != ''"} + + rich = render_params_rich_table(rows, header, no_color=True) + plain = render_params_oneline(rows, header) + + assert "DURATION" in rich + assert "1m05s" in rich + assert "\t1m05s\t" in plain + + +def test_render_params_json_includes_stable_duration_object() -> None: + rows = [ + RunParams( + RunMeta("abc123", "exp", "run", None), + {"hparam.lr": 0.1}, + duration=RunDuration( + seconds=65.2, + status="available", + source="end_time_minus_creation_time", + ), + ) + ] + header = {"target": "params", "repo": "repo", "expression": "run.hash != ''"} + + payload = json.loads(render_params_json(rows, header)) + + assert payload["runs"][0]["duration"] == { + "seconds": 65.2, + "status": "available", + "source": "end_time_minus_creation_time", + } + + +def test_render_params_marks_unavailable_and_running_duration() -> None: + rows = [ + RunParams( + RunMeta("abc123", "exp", "missing", None), + {"hparam.lr": 0.1}, + duration=RunDuration(status="unavailable"), + ), + RunParams( + RunMeta("def456", "exp", "active", None), + {"hparam.lr": 0.2}, + duration=RunDuration(status="running"), + ), + ] + header = {"target": "params", "repo": "repo", "expression": "run.hash != ''"} + + rich = render_params_rich_table(rows, header, no_color=True) + plain = render_params_oneline(rows, header) + + assert "unavailable" in rich + assert "running" in rich + assert "\tunavailable\t" in plain + assert "\trunning\t" in plain + + +def test_render_params_keeps_duration_independent_from_selected_params() -> None: + rows = [ + RunParams( + RunMeta("abc123", "exp", "run", None), + {"hparam.lr": 0.1}, + duration=RunDuration(seconds=1.0, status="available", source="duration"), + selected_keys=("hparam.lr", "missing.key"), + missing_keys=("missing.key",), + ) + ] + header = { + "target": "params", + "repo": "repo", + "expression": "run.hash != ''", + "param_keys": ("hparam.lr", "missing.key"), + } + + payload = json.loads(render_params_json(rows, header)) + run = payload["runs"][0] + + assert payload["param_keys"] == ["hparam.lr", "missing.key"] + assert run["params"] == {"hparam.lr": 0.1} + assert run["missing_params"] == ["missing.key"] + assert run["duration"]["seconds"] == 1.0 diff --git a/tests/unit/test_trace_helpers.py b/tests/unit/test_trace_helpers.py index bfabdc3..e0dd5a1 100644 --- a/tests/unit/test_trace_helpers.py +++ b/tests/unit/test_trace_helpers.py @@ -11,6 +11,7 @@ _execute_trace_pipeline, parse_trace_invocation, ) +from aimx.commands.query import DEFAULT_QUERY_EXPRESSION @dataclass @@ -36,6 +37,87 @@ def test_parse_trace_defaults() -> None: assert not inv.no_color +def test_parse_trace_uses_default_expression_when_omitted() -> None: + inv = parse_trace_invocation([]) + + assert inv.target == "metrics" + assert inv.expression == DEFAULT_QUERY_EXPRESSION + + +def test_parse_trace_treats_option_first_as_omitted_expression() -> None: + inv = parse_trace_invocation(["--repo", "data", "--json", "--head", "5"]) + + assert inv.expression == DEFAULT_QUERY_EXPRESSION + assert inv.repo_path == Path("data") + assert inv.mode == "json" + assert inv.head == 5 + + +def test_parse_trace_distribution_uses_default_expression_when_omitted() -> None: + inv = parse_trace_invocation(["distribution"]) + + assert inv.target == "distribution" + assert inv.expression == DEFAULT_QUERY_EXPRESSION + + +def test_parse_trace_distribution_treats_option_first_as_omitted_expression() -> None: + inv = parse_trace_invocation(["distribution", "--repo", "data", "--table"]) + + assert inv.target == "distribution" + assert inv.expression == DEFAULT_QUERY_EXPRESSION + assert inv.repo_path == Path("data") + assert inv.mode == "table" + + +@pytest.mark.parametrize( + ("args", "attr", "expected"), + [ + (["--repo", "data"], "repo_path", Path("data")), + (["--json"], "mode", "json"), + (["--table"], "mode", "table"), + (["--csv"], "mode", "csv"), + (["--steps", "1:5"], "step_slice", "1:5"), + (["--head", "2"], "head", 2), + (["--tail", "3"], "tail", 3), + (["--every", "4"], "every", 4), + (["--width", "100"], "width", 100), + (["--height", "30"], "height", 30), + (["--no-color"], "no_color", True), + ], +) +def test_parse_trace_option_first_flags_preserve_option_parsing( + args: list[str], + attr: str, + expected: object, +) -> None: + inv = parse_trace_invocation(args) + + assert inv.expression == DEFAULT_QUERY_EXPRESSION + assert getattr(inv, attr) == expected + + +def test_parse_trace_distribution_option_first_step_preserves_option_parsing() -> None: + inv = parse_trace_invocation(["distribution", "--step", "123"]) + + assert inv.target == "distribution" + assert inv.expression == DEFAULT_QUERY_EXPRESSION + assert inv.selected_step == 123 + + +def test_parse_trace_uses_default_expression_when_blank() -> None: + inv = parse_trace_invocation([" ", "--repo", "data"]) + + assert inv.expression == DEFAULT_QUERY_EXPRESSION + assert inv.repo_path == Path("data") + + +def test_parse_trace_preserves_explicit_expression_precedence() -> None: + inv = parse_trace_invocation(["metric.name=='loss'", "--repo", "data"]) + + assert inv.expression == "metric.name=='loss'" + assert inv.repo_path == Path("data") + + def test_parse_trace_table_mode() -> None: inv = parse_trace_invocation(["expr", "--repo", "data", "--table"]) assert inv.mode == "table" @@ -89,21 +171,11 @@ def test_parse_trace_distribution_target() -> None: assert inv.repo_path == Path("data") -def test_parse_trace_distribution_requires_expression() -> None: - with pytest.raises(ValueError, match="trace distribution"): - parse_trace_invocation(["distribution"]) - - def test_parse_trace_rejects_unknown_flag() -> None: with pytest.raises(ValueError, match="Unsupported trace option"): parse_trace_invocation(["expr", "--repo", "data", "--bogus"]) -def test_parse_trace_rejects_missing_expression() -> None: - with pytest.raises(ValueError, match="Usage"): - parse_trace_invocation([]) - - def test_parse_trace_rejects_non_integer_head() -> None: with pytest.raises(ValueError, match="--head requires an integer"): parse_trace_invocation(["expr", "--repo", "data", "--head", "abc"])