cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [boundary] Empty string for named level not handled in DetailLevelMap #1981

New issue

Open

opened 2026-04-03 00:30:07 +00:00 by freemo · 2 comments

freemo commented

2026-04-03 00:30:07 +00:00

Owner

Metadata

Branch: fix/boundary-detail-level-map-empty-name-validation
Commit Message: fix(acms): validate named level keys are non-empty in DetailLevelMap
Milestone: v3.4.0
Parent Epic: #935

Background and Context

The DetailLevelMap class in the ACMS domain model is responsible for mapping named detail levels to integer depths. The validate_levels field validator currently guards against negative depth values but does not validate the keys (named levels) themselves. This violates the project's fail-fast and argument validation principles, which require that empty strings be rejected at the boundary where they enter.

Current Behavior

The validate_levels method in DetailLevelMap (located at cleveragents.domain.models.acms.detail_level.DetailLevelMap.validate_levels) accepts any string as a named level key, including empty strings ("") and whitespace-only strings (e.g., " "). This could lead to unexpected behavior when resolving detail levels downstream.

@field_validator("levels", mode="before")
@classmethod
def validate_levels(
    cls: type[DetailLevelMap],
    v: Mapping[str, int],
) -> dict[str, int]:
    plain = dict(v)
    for name, depth in plain.items():
        if depth < 0:
            raise ValueError(f"Detail level '{name}' has negative depth {depth}")
    return plain

Expected Behavior

The validate_levels method should reject any named level key that is an empty string or contains only whitespace, raising a ValueError with a clear message. This aligns with CONTRIBUTING.md's argument validation requirements: "Empty Strings: Reject empty strings where non-empty strings are required."

Suggested fix:

@field_validator("levels", mode="before")
@classmethod
def validate_levels(
    cls: type[DetailLevelMap],
    v: Mapping[str, int],
) -> dict[str, int]:
    """Validate non-empty names, non-negative depths, and normalise to a plain dict."""
    plain = dict(v)
    for name, depth in plain.items():
        if not name or not name.strip():
            raise ValueError(
                "Detail level name must not be empty or contain only whitespace"
            )
        if depth < 0:
            raise ValueError(f"Detail level '{name}' has negative depth {depth}")
    return plain

Acceptance Criteria

DetailLevelMap raises ValueError when a named level key is an empty string ("").
DetailLevelMap raises ValueError when a named level key is a whitespace-only string (e.g., " ").
Valid named level keys (non-empty, non-whitespace) continue to be accepted without error.
The error message clearly identifies the problem (empty/whitespace name).
All existing tests continue to pass.

Supporting Information

File: src/cleveragents/domain/models/acms/detail_level.py
Class/Method: DetailLevelMap.validate_levels
Category: boundary
Severity: Low — unlikely scenario but violates fail-fast principles
Related issue: #1937 (similar boundary bug for ContextRequest)

Subtasks

Update DetailLevelMap.validate_levels to reject empty and whitespace-only named level keys
Tests (Behave): Add BDD scenarios for empty string and whitespace-only named level keys
Tests (Robot): Add integration test verifying boundary validation in DetailLevelMap
Verify coverage >=97% via nox -s coverage_report
Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly (fix(acms): validate named level keys are non-empty in DetailLevelMap), followed by a blank line, then additional lines providing relevant implementation details.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly (fix/boundary-detail-level-map-empty-name-validation).
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
All nox stages pass.
Coverage >= 97%.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/boundary-detail-level-map-empty-name-validation` - **Commit Message**: `fix(acms): validate named level keys are non-empty in DetailLevelMap` - **Milestone**: v3.4.0 - **Parent Epic**: #935 ## Background and Context The `DetailLevelMap` class in the ACMS domain model is responsible for mapping named detail levels to integer depths. The `validate_levels` field validator currently guards against negative depth values but does not validate the keys (named levels) themselves. This violates the project's fail-fast and argument validation principles, which require that empty strings be rejected at the boundary where they enter. ## Current Behavior The `validate_levels` method in `DetailLevelMap` (located at `cleveragents.domain.models.acms.detail_level.DetailLevelMap.validate_levels`) accepts any string as a named level key, including empty strings (`""`) and whitespace-only strings (e.g., `" "`). This could lead to unexpected behavior when resolving detail levels downstream. ```python @field_validator("levels", mode="before") @classmethod def validate_levels( cls: type[DetailLevelMap], v: Mapping[str, int], ) -> dict[str, int]: plain = dict(v) for name, depth in plain.items(): if depth < 0: raise ValueError(f"Detail level '{name}' has negative depth {depth}") return plain ``` ## Expected Behavior The `validate_levels` method should reject any named level key that is an empty string or contains only whitespace, raising a `ValueError` with a clear message. This aligns with CONTRIBUTING.md's argument validation requirements: *"Empty Strings: Reject empty strings where non-empty strings are required."* Suggested fix: ```python @field_validator("levels", mode="before") @classmethod def validate_levels( cls: type[DetailLevelMap], v: Mapping[str, int], ) -> dict[str, int]: """Validate non-empty names, non-negative depths, and normalise to a plain dict.""" plain = dict(v) for name, depth in plain.items(): if not name or not name.strip(): raise ValueError( "Detail level name must not be empty or contain only whitespace" ) if depth < 0: raise ValueError(f"Detail level '{name}' has negative depth {depth}") return plain ``` ## Acceptance Criteria - [ ] `DetailLevelMap` raises `ValueError` when a named level key is an empty string (`""`). - [ ] `DetailLevelMap` raises `ValueError` when a named level key is a whitespace-only string (e.g., `" "`). - [ ] Valid named level keys (non-empty, non-whitespace) continue to be accepted without error. - [ ] The error message clearly identifies the problem (empty/whitespace name). - [ ] All existing tests continue to pass. ## Supporting Information - **File**: `src/cleveragents/domain/models/acms/detail_level.py` - **Class/Method**: `DetailLevelMap.validate_levels` - **Category**: boundary - **Severity**: Low — unlikely scenario but violates fail-fast principles - **Related issue**: #1937 (similar boundary bug for `ContextRequest`) ## Subtasks - [ ] Update `DetailLevelMap.validate_levels` to reject empty and whitespace-only named level keys - [ ] Tests (Behave): Add BDD scenarios for empty string and whitespace-only named level keys - [ ] Tests (Robot): Add integration test verifying boundary validation in `DetailLevelMap` - [ ] Verify coverage >=97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly (`fix(acms): validate named level keys are non-empty in DetailLevelMap`), followed by a blank line, then additional lines providing relevant implementation details. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly (`fix/boundary-detail-level-map-empty-name-validation`). - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. - All nox stages pass. - Coverage >= 97%. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: ca-new-issue-creator

freemo added this to the v3.4.0 milestone

2026-04-03 00:31:06 +00:00

freemo added a new dependency

2026-04-03 00:32:37 +00:00

#935 Epic: ACMS Context Assembly Pipeline — Functional CRP Handlers and Pipeline Components

freemo commented

2026-04-03 02:43:36 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Low (confirmed)
Milestone: v3.2.0 (already assigned — boundary/edge case handling)
MoSCoW: Could Have — Empty string handling for named levels in DetailLevelMap is an edge case that is unlikely to be hit in normal usage. This is a defensive coding improvement, not a functional requirement.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Low (confirmed) - **Milestone**: v3.2.0 (already assigned — boundary/edge case handling) - **MoSCoW**: Could Have — Empty string handling for named levels in `DetailLevelMap` is an edge case that is unlikely to be hit in normal usage. This is a defensive coding improvement, not a functional requirement. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo added the

MoSCoW

Could have

label

2026-04-03 02:44:14 +00:00

freemo removed the

MoSCoW

Could have

label

2026-04-05 07:29:04 +00:00

freemo commented

2026-04-05 07:30:59 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Low
Milestone: v3.4.0
MoSCoW: Should Have — boundary condition handling for empty strings in DetailLevelMap is a correctness issue that should be addressed

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Low - **Milestone**: v3.4.0 - **MoSCoW**: Should Have — boundary condition handling for empty strings in DetailLevelMap is a correctness issue that should be addressed --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo referenced this issue

2026-04-05 07:41:47 +00:00

[CA-AUTO] Project Owner — project-owner-1 — Session Tracker #2793