fix(session): correct field names and data types in Session.as_cli_dict() for spec compliance #3461

2026-04-05T17:56:28Z

freemo commented

2026-04-05 17:56:28 +00:00

Summary

Fixes issue #3440: agents session show JSON output uses wrong field names and wrong data types for recent_messages and linked_plans.

Problem

Session.as_cli_dict() returned incorrect field names and data structures throughout, breaking spec compliance for the agents session show JSON output:

#	Current key / value	Spec-required key / value
1	`session_id`	`id` (inside `session_summary`)
2	`actor_name`	`actor` (inside `session_summary`)
3	`message_count`	`messages` (inside `session_summary`)
4	`created_at`	`created` (inside `session_summary`)
5	`updated_at`	`updated` (inside `session_summary`)
6	`namespace` (present)	not in spec's `session_summary`
7	(missing)	`automation` field in `session_summary`
8	`recent_messages[].content`	`recent_messages[].text`
9	`linked_plan_ids` (flat `list[str]`)	`linked_plans` (list of `{plan_id, phase, state}` objects)
10	`estimated_cost` as `float`	`estimated_cost` as formatted string e.g. `"$0.0184"`

Solution

Architectural decision: Added LinkedPlan value object to the Session domain model to store plan phase and state alongside the plan ID. This is the cleanest approach — the domain model owns its data.

Changes:

session.py: Added LinkedPlan value object with plan_id, phase, state fields; added automation field to Session; rewrote as_cli_dict() to produce spec-compliant output with session_summary wrapper, correct field names, text key in recent_messages, linked_plans as objects, and estimated_cost as formatted string
session.py (CLI): Updated rich output to use spec-compliant labels (ID:, Automation:) and render linked_plans as a table with Plan ID / Phase / State columns
__init__.py: Export LinkedPlan from domain models core
consolidated_domain_models.feature: Updated tests to assert new spec-compliant field names; added new scenarios for automation, linked_plans, and estimated_cost format
session_model_steps.py: Added step definitions for new assertions

Quality Gates

✅ nox -s lint — passes
✅ nox -s typecheck — 0 errors, 0 warnings
✅ Manual Python unit tests — all assertions pass
⚠️ nox -s unit_tests — behave-parallel hangs in local environment (pre-existing infrastructure issue, not caused by this change — verified by testing without changes)

Closes #3440

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

## Summary Fixes issue #3440: `agents session show` JSON output uses wrong field names and wrong data types for `recent_messages` and `linked_plans`. ### Problem `Session.as_cli_dict()` returned incorrect field names and data structures throughout, breaking spec compliance for the `agents session show` JSON output: | # | Current key / value | Spec-required key / value | |---|---|---| | 1 | `session_id` | `id` (inside `session_summary`) | | 2 | `actor_name` | `actor` (inside `session_summary`) | | 3 | `message_count` | `messages` (inside `session_summary`) | | 4 | `created_at` | `created` (inside `session_summary`) | | 5 | `updated_at` | `updated` (inside `session_summary`) | | 6 | `namespace` (present) | not in spec's `session_summary` | | 7 | *(missing)* | `automation` field in `session_summary` | | 8 | `recent_messages[].content` | `recent_messages[].text` | | 9 | `linked_plan_ids` (flat `list[str]`) | `linked_plans` (list of `{plan_id, phase, state}` objects) | | 10 | `estimated_cost` as `float` | `estimated_cost` as formatted string e.g. `"$0.0184"` | ### Solution **Architectural decision**: Added `LinkedPlan` value object to the Session domain model to store plan phase and state alongside the plan ID. This is the cleanest approach — the domain model owns its data. **Changes:** - **`session.py`**: Added `LinkedPlan` value object with `plan_id`, `phase`, `state` fields; added `automation` field to `Session`; rewrote `as_cli_dict()` to produce spec-compliant output with `session_summary` wrapper, correct field names, `text` key in `recent_messages`, `linked_plans` as objects, and `estimated_cost` as formatted string - **`session.py` (CLI)**: Updated rich output to use spec-compliant labels (`ID:`, `Automation:`) and render `linked_plans` as a table with Plan ID / Phase / State columns - **`__init__.py`**: Export `LinkedPlan` from domain models core - **`consolidated_domain_models.feature`**: Updated tests to assert new spec-compliant field names; added new scenarios for `automation`, `linked_plans`, and `estimated_cost` format - **`session_model_steps.py`**: Added step definitions for new assertions ### Quality Gates - ✅ `nox -s lint` — passes - ✅ `nox -s typecheck` — 0 errors, 0 warnings - ✅ Manual Python unit tests — all assertions pass - ⚠️ `nox -s unit_tests` — behave-parallel hangs in local environment (pre-existing infrastructure issue, not caused by this change — verified by testing without changes) Closes #3440 --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker

freemo added 1 commit 2026-04-05 17:56:28 +00:00

fix(session): correct field names and data types in Session.as_cli_dict() for spec compliance

CI / lint (pull_request) Successful in 32s

Details

CI / quality (pull_request) Successful in 34s

Details

CI / typecheck (pull_request) Successful in 54s

Details

CI / security (pull_request) Successful in 1m7s

Details

CI / build (pull_request) Successful in 28s

Details

CI / helm (pull_request) Successful in 23s

Details

CI / unit_tests (pull_request) Successful in 6m47s

Details

CI / e2e_tests (pull_request) Successful in 17m15s

Details

CI / integration_tests (pull_request) Failing after 22m54s

Details

CI / docker (pull_request) Successful in 1m22s

Details

CI / coverage (pull_request) Successful in 11m0s

Details

CI / status-check (pull_request) Failing after 2s

Details

CI / benchmark-publish (pull_request) Has been skipped

Details

CI / benchmark-regression (pull_request) Successful in 57m15s

Details

c1ab8c86b3

Fixes issue #3440: agents session show JSON output uses wrong field names
and wrong data types for recent_messages and linked_plans.

Changes:
- Add LinkedPlan value object to session domain model with plan_id, phase,
  state fields for spec-compliant linked_plans output
- Add automation field to Session domain model for automation profile name
- Rewrite as_cli_dict() to produce spec-compliant output:
  - Wrap session fields in session_summary sub-dict
  - Rename session_id -> id, actor_name -> actor, message_count -> messages,
    created_at -> created, updated_at -> updated
  - Remove namespace from session_summary (not in spec)
  - Add automation field to session_summary
  - Use 'text' key (not 'content') in recent_messages items
  - Replace linked_plan_ids (flat list) with linked_plans (list of objects
    with plan_id, phase, state)
  - Format estimated_cost as string (e.g. '$0.0184') not raw float
- Update session show rich output to use spec-compliant labels and
  render linked_plans as a table with Plan ID / Phase / State columns
- Update consolidated_domain_models.feature tests to assert new field names
- Add new Behave step definitions for spec-compliant field assertions
- Export LinkedPlan from domain models core __init__.py

ISSUES CLOSED: #3440

freemo referenced this pull request

2026-04-05 17:59:36 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo reviewed 2026-04-05 17:59:54 +00:00

freemo left a comment

PR #3461 Review — `fix(session): correct field names and data types in Session.as_cli_dict() for spec compliance`

Review Focus: specification-compliance, behavior-correctness, api-consistency
Linked Issue: #3440 (UAT: agents session show JSON output uses wrong field names)

✅ What Looks Good

Specification Compliance: The rewritten as_cli_dict() correctly produces the spec-required structure with session_summary wrapper, renamed fields (id, actor, messages, created, updated), automation field, text key in recent_messages, linked_plans as objects, and estimated_cost as formatted string. This directly addresses all 9 violations listed in #3440.
Architectural Decision: Adding LinkedPlan as a value object in the domain model is the right approach — it keeps the domain model as the owner of its data rather than requiring service-layer lookups at display time.
Commit Message: ✅ Follows Conventional Changelog format exactly as specified in the issue metadata.
ISSUES CLOSED Footer: ✅ Present (ISSUES CLOSED: #3440).
PR Description: ✅ Thorough, well-structured, includes Closes #3440.
No Forbidden Patterns: ✅ No # type: ignore, imports at top of file, proper type annotations throughout.
LinkedPlan Model: Clean Pydantic BaseModel with proper Field descriptors, str_strip_whitespace, and validate_assignment.
__init__.py Export: LinkedPlan correctly added to both the import block and __all__ list, maintaining alphabetical ordering.

⚠️ Issues Requiring Attention

1. [API-CONSISTENCY] Dual Representation Creates Consistency Risk — `linked_plan_ids` vs `linked_plans`

Location: src/cleveragents/domain/models/core/session.py — Session model fields
Issue: The Session model now has both linked_plan_ids: list[str] and linked_plans: list[LinkedPlan]. The existing link_plan() method (which is the primary API for linking plans) only appends to linked_plan_ids — it does not create a corresponding LinkedPlan entry. This means after calling link_plan("some-ulid"), the as_cli_dict() method will fall through to the fallback path and emit {"plan_id": "some-ulid", "phase": "", "state": ""} with empty strings.
Risk: Any code path that uses link_plan() (the existing API) will produce degraded output. The two lists can drift out of sync with no validation preventing it.
Suggestion: Consider either (a) adding a model validator that ensures consistency between the two lists, (b) updating link_plan() to accept optional phase/state parameters and populate both lists, or (c) documenting the intended migration path clearly. At minimum, the link_plan() docstring should note that it only populates linked_plan_ids and that linked_plans must be populated separately for full spec-compliant output.

2. [BEHAVIOR] Fallback Uses Empty Strings Instead of Explicit "Unknown"

Location: src/cleveragents/domain/models/core/session.py — as_cli_dict(), the elif self.linked_plan_ids: branch
Issue: When only flat IDs are available, the fallback emits {"plan_id": pid, "phase": "", "state": ""}. Empty strings are ambiguous — they could mean "not set", "unknown", or "intentionally blank". The spec example shows actual values like "execute" and "complete".
Suggestion: Consider using "unknown" instead of "" for the fallback phase/state values, which is more explicit and less likely to confuse downstream consumers.

3. [SPEC-COMPLIANCE] Missing ULID Validation on `LinkedPlan.plan_id`

Location: src/cleveragents/domain/models/core/session.py — LinkedPlan class
Issue: LinkedPlan.plan_id is declared as str without pattern=ULID_PATTERN validation, while Session.session_id and SessionMessage.message_id both enforce the ULID pattern. This is inconsistent — plan IDs should be ULIDs per the spec.
Suggestion: Add pattern=ULID_PATTERN to the plan_id field for consistency with the rest of the codebase.

4. [PROCESS] PR Missing Milestone and Labels

Location: PR metadata
Issue: Per CONTRIBUTING.md, every PR must be assigned to the same milestone as its linked issue and must have exactly one Type/ label. Issue #3440 is in milestone v3.7.0 and has label Type/Bug. This PR has neither a milestone nor any labels.
Required: Assign milestone v3.7.0 and add label Type/Bug.

5. [BEHAVIOR] `estimated_cost` Format Precision

Location: src/cleveragents/domain/models/core/session.py — as_cli_dict(), f"${cost:.4f}"
Issue: The format always uses exactly 4 decimal places. The spec example shows "$0.0184" (4 decimals). This works for the example, but for larger costs like $12.50, it would produce "$12.5000". For very small costs like $0.000001, it would produce "$0.0000" (losing precision). This is a minor concern but worth considering whether a dynamic precision or minimum significant digits approach would be more appropriate.
Suggestion: This is acceptable for now but consider whether the spec intends a specific precision contract.

6. [API-CONSISTENCY] `as_export_dict()` Not Updated

Location: src/cleveragents/domain/models/core/session.py — as_export_dict()
Issue: The export dict still uses linked_plan_ids (flat list) and estimated_cost as a raw float. While the export format may intentionally differ from the CLI display format (export is for data interchange, CLI is for human/machine display), this creates two different serialization contracts for the same model. The as_export_dict() docstring doesn't mention this distinction.
Suggestion: This may be intentional (export preserves raw data, CLI formats for display), but it should be documented explicitly in the as_export_dict() docstring to prevent future confusion.

📋 Summary

Category	Status
Spec Compliance (as_cli_dict output)	✅ Correct
Commit Message Format	✅ Correct
Type Safety	✅ No suppressions
LinkedPlan Value Object	✅ Well-designed
__init__.py Export	✅ Correct
Dual linked_plan_ids/linked_plans	⚠️ Consistency risk
LinkedPlan.plan_id validation	⚠️ Missing ULID pattern
PR Milestone & Labels	❌ Missing
Fallback empty strings	⚠️ Minor — prefer "unknown"
Test coverage	⚠️ Cannot verify — unit tests noted as hanging

The core implementation is solid and correctly addresses the spec compliance issues. The main concerns are around the dual-representation consistency risk and missing PR metadata. Items #1 (dual representation) and #4 (missing milestone/labels) should be addressed before merge.

Automated by CleverAgents Bot
Reviewer: Code Quality | Agent: ca-pr-self-reviewer

## PR #3461 Review — `fix(session): correct field names and data types in Session.as_cli_dict() for spec compliance` **Review Focus**: specification-compliance, behavior-correctness, api-consistency **Linked Issue**: #3440 (UAT: `agents session show` JSON output uses wrong field names) --- ### ✅ What Looks Good 1. **Specification Compliance**: The rewritten `as_cli_dict()` correctly produces the spec-required structure with `session_summary` wrapper, renamed fields (`id`, `actor`, `messages`, `created`, `updated`), `automation` field, `text` key in `recent_messages`, `linked_plans` as objects, and `estimated_cost` as formatted string. This directly addresses all 9 violations listed in #3440. 2. **Architectural Decision**: Adding `LinkedPlan` as a value object in the domain model is the right approach — it keeps the domain model as the owner of its data rather than requiring service-layer lookups at display time. 3. **Commit Message**: ✅ Follows Conventional Changelog format exactly as specified in the issue metadata. 4. **ISSUES CLOSED Footer**: ✅ Present (`ISSUES CLOSED: #3440`). 5. **PR Description**: ✅ Thorough, well-structured, includes `Closes #3440`. 6. **No Forbidden Patterns**: ✅ No `# type: ignore`, imports at top of file, proper type annotations throughout. 7. **`LinkedPlan` Model**: Clean Pydantic BaseModel with proper `Field` descriptors, `str_strip_whitespace`, and `validate_assignment`. 8. **`__init__.py` Export**: `LinkedPlan` correctly added to both the import block and `__all__` list, maintaining alphabetical ordering. --- ### ⚠️ Issues Requiring Attention #### 1. **[API-CONSISTENCY] Dual Representation Creates Consistency Risk** — `linked_plan_ids` vs `linked_plans` - **Location**: `src/cleveragents/domain/models/core/session.py` — `Session` model fields - **Issue**: The `Session` model now has **both** `linked_plan_ids: list[str]` and `linked_plans: list[LinkedPlan]`. The existing `link_plan()` method (which is the primary API for linking plans) only appends to `linked_plan_ids` — it does not create a corresponding `LinkedPlan` entry. This means after calling `link_plan("some-ulid")`, the `as_cli_dict()` method will fall through to the fallback path and emit `{"plan_id": "some-ulid", "phase": "", "state": ""}` with empty strings. - **Risk**: Any code path that uses `link_plan()` (the existing API) will produce degraded output. The two lists can drift out of sync with no validation preventing it. - **Suggestion**: Consider either (a) adding a model validator that ensures consistency between the two lists, (b) updating `link_plan()` to accept optional `phase`/`state` parameters and populate both lists, or (c) documenting the intended migration path clearly. At minimum, the `link_plan()` docstring should note that it only populates `linked_plan_ids` and that `linked_plans` must be populated separately for full spec-compliant output. #### 2. **[BEHAVIOR] Fallback Uses Empty Strings Instead of Explicit "Unknown"** - **Location**: `src/cleveragents/domain/models/core/session.py` — `as_cli_dict()`, the `elif self.linked_plan_ids:` branch - **Issue**: When only flat IDs are available, the fallback emits `{"plan_id": pid, "phase": "", "state": ""}`. Empty strings are ambiguous — they could mean "not set", "unknown", or "intentionally blank". The spec example shows actual values like `"execute"` and `"complete"`. - **Suggestion**: Consider using `"unknown"` instead of `""` for the fallback phase/state values, which is more explicit and less likely to confuse downstream consumers. #### 3. **[SPEC-COMPLIANCE] Missing ULID Validation on `LinkedPlan.plan_id`** - **Location**: `src/cleveragents/domain/models/core/session.py` — `LinkedPlan` class - **Issue**: `LinkedPlan.plan_id` is declared as `str` without `pattern=ULID_PATTERN` validation, while `Session.session_id` and `SessionMessage.message_id` both enforce the ULID pattern. This is inconsistent — plan IDs should be ULIDs per the spec. - **Suggestion**: Add `pattern=ULID_PATTERN` to the `plan_id` field for consistency with the rest of the codebase. #### 4. **[PROCESS] PR Missing Milestone and Labels** - **Location**: PR metadata - **Issue**: Per CONTRIBUTING.md, every PR must be assigned to the same milestone as its linked issue and must have exactly one `Type/` label. Issue #3440 is in milestone **v3.7.0** and has label `Type/Bug`. This PR has neither a milestone nor any labels. - **Required**: Assign milestone `v3.7.0` and add label `Type/Bug`. #### 5. **[BEHAVIOR] `estimated_cost` Format Precision** - **Location**: `src/cleveragents/domain/models/core/session.py` — `as_cli_dict()`, `f"${cost:.4f}"` - **Issue**: The format always uses exactly 4 decimal places. The spec example shows `"$0.0184"` (4 decimals). This works for the example, but for larger costs like `$12.50`, it would produce `"$12.5000"`. For very small costs like `$0.000001`, it would produce `"$0.0000"` (losing precision). This is a minor concern but worth considering whether a dynamic precision or minimum significant digits approach would be more appropriate. - **Suggestion**: This is acceptable for now but consider whether the spec intends a specific precision contract. #### 6. **[API-CONSISTENCY] `as_export_dict()` Not Updated** - **Location**: `src/cleveragents/domain/models/core/session.py` — `as_export_dict()` - **Issue**: The export dict still uses `linked_plan_ids` (flat list) and `estimated_cost` as a raw float. While the export format may intentionally differ from the CLI display format (export is for data interchange, CLI is for human/machine display), this creates two different serialization contracts for the same model. The `as_export_dict()` docstring doesn't mention this distinction. - **Suggestion**: This may be intentional (export preserves raw data, CLI formats for display), but it should be documented explicitly in the `as_export_dict()` docstring to prevent future confusion. --- ### 📋 Summary | Category | Status | |----------|--------| | Spec Compliance (as_cli_dict output) | ✅ Correct | | Commit Message Format | ✅ Correct | | Type Safety | ✅ No suppressions | | LinkedPlan Value Object | ✅ Well-designed | | __init__.py Export | ✅ Correct | | Dual linked_plan_ids/linked_plans | ⚠️ Consistency risk | | LinkedPlan.plan_id validation | ⚠️ Missing ULID pattern | | PR Milestone & Labels | ❌ Missing | | Fallback empty strings | ⚠️ Minor — prefer "unknown" | | Test coverage | ⚠️ Cannot verify — unit tests noted as hanging | The core implementation is solid and correctly addresses the spec compliance issues. The main concerns are around the dual-representation consistency risk and missing PR metadata. Items #1 (dual representation) and #4 (missing milestone/labels) should be addressed before merge. --- **Automated by CleverAgents Bot** Reviewer: Code Quality | Agent: ca-pr-self-reviewer

freemo merged commit 4d32281351 into master

2026-04-05 18:19:20 +00:00

freemo deleted branch bugfix/session-show-as-cli-dict-field-names

2026-04-05 18:19:20 +00:00

freemo referenced this pull request

2026-04-05 18:19:34 +00:00

UAT: `agents session show` JSON output uses wrong field names and wrong data types for recent_messages and linked_plans #3440

freemo referenced this pull request

2026-04-05 18:43:46 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo referenced this pull request

2026-04-05 19:34:34 +00:00