Bug: Negative `max_lines` in `ThoughtBlock` model leads to unexpected behavior #9295

New issue

Open

opened 2026-04-14 14:19:15 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-14 14:19:15 +00:00

Owner

Metadata

Commit Message: fix(tui): add non-negative validation to ThoughtBlock.max_lines field
Branch: fix/thought-block-max-lines-validation

Background and Context

The ThoughtBlock domain model in src/cleveragents/domain/models/thought/thought_block.py is responsible for representing actor reasoning/thought trace blocks in the TUI. It exposes visible_lines(), hidden_line_count(), is_truncated(), and rendered_text() methods that all depend on the max_lines field to determine how many lines to show when the block is collapsed.

Currently, the max_lines field is declared as a plain int with no lower-bound constraint:

# src/cleveragents/domain/models/thought/thought_block.py, line 22
max_lines: int = Field(default=_DEFAULT_MAX_LINES)

This means a caller can construct ThoughtBlock(content="...", max_lines=-1) without any validation error. When max_lines is negative, the slice all_lines[: self.max_lines] in visible_lines() returns an unexpected subset of lines (Python negative slicing), hidden_line_count() returns a negative count, and is_truncated() may return True even for short content — all of which violate the contract described in the specification (§ Actor Thought Block: "max 10 lines (expandable)").

Per CONTRIBUTING.md's Argument Validation and Fail-Fast Principles sections, all public methods must validate arguments at the boundary where they enter, and numeric values must be within acceptable bounds.

Expected Behavior

Constructing a ThoughtBlock with max_lines < 0 must raise a ValidationError immediately.
max_lines = 0 is a valid edge case (show no lines when collapsed) and must be accepted.
All existing behaviour for max_lines >= 0 is unchanged.

Acceptance Criteria

ThoughtBlock(content="...", max_lines=-1) raises a Pydantic ValidationError.
ThoughtBlock(content="...", max_lines=0) is accepted without error.
ThoughtBlock(content="...", max_lines=10) (default) continues to work as before.
visible_lines(), hidden_line_count(), is_truncated(), and rendered_text() all behave correctly for all valid max_lines values.
A BDD scenario (Behave) is added to features/tui_thought_block.feature covering the negative max_lines rejection.
All existing BDD scenarios in features/tui_thought_block.feature continue to pass.
Test coverage remains ≥ 97%.
All quality gates pass (nox default sessions: lint, typecheck, unit tests, integration tests, coverage).

Subtasks

Add a failing BDD scenario to features/tui_thought_block.feature that asserts ThoughtBlock(content="...", max_lines=-1) raises a ValidationError (TDD — this scenario must fail before the fix is applied, tagged @tdd_issue @tdd_issue_<N> @tdd_expected_fail)
Add the corresponding step implementation to features/steps/tui_thought_block_steps.py
Apply the fix: constrain max_lines to ge=0 using Pydantic's Field(ge=0, ...) or Annotated[int, Field(ge=0)] in ThoughtBlock
Remove @tdd_expected_fail from the scenario once the fix makes it pass
Run nox (all default sessions) and fix any errors
Verify coverage ≥ 97% via nox -s coverage_report
Update CHANGELOG.md with an entry for this fix
Update CONTRIBUTORS.md if applicable

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(tui): add non-negative validation to ThoughtBlock.max_lines field), followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly (fix/thought-block-max-lines-validation).
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata - **Commit Message**: `fix(tui): add non-negative validation to ThoughtBlock.max_lines field` - **Branch**: `fix/thought-block-max-lines-validation` ## Background and Context The `ThoughtBlock` domain model in `src/cleveragents/domain/models/thought/thought_block.py` is responsible for representing actor reasoning/thought trace blocks in the TUI. It exposes `visible_lines()`, `hidden_line_count()`, `is_truncated()`, and `rendered_text()` methods that all depend on the `max_lines` field to determine how many lines to show when the block is collapsed. Currently, the `max_lines` field is declared as a plain `int` with no lower-bound constraint: ```python # src/cleveragents/domain/models/thought/thought_block.py, line 22 max_lines: int = Field(default=_DEFAULT_MAX_LINES) ``` This means a caller can construct `ThoughtBlock(content="...", max_lines=-1)` without any validation error. When `max_lines` is negative, the slice `all_lines[: self.max_lines]` in `visible_lines()` returns an unexpected subset of lines (Python negative slicing), `hidden_line_count()` returns a negative count, and `is_truncated()` may return `True` even for short content — all of which violate the contract described in the specification (§ Actor Thought Block: "max 10 lines (expandable)"). Per CONTRIBUTING.md's **Argument Validation** and **Fail-Fast Principles** sections, all public methods must validate arguments at the boundary where they enter, and numeric values must be within acceptable bounds. ## Expected Behavior - Constructing a `ThoughtBlock` with `max_lines < 0` must raise a `ValidationError` immediately. - `max_lines = 0` is a valid edge case (show no lines when collapsed) and must be accepted. - All existing behaviour for `max_lines >= 0` is unchanged. ## Acceptance Criteria - [ ] `ThoughtBlock(content="...", max_lines=-1)` raises a Pydantic `ValidationError`. - [ ] `ThoughtBlock(content="...", max_lines=0)` is accepted without error. - [ ] `ThoughtBlock(content="...", max_lines=10)` (default) continues to work as before. - [ ] `visible_lines()`, `hidden_line_count()`, `is_truncated()`, and `rendered_text()` all behave correctly for all valid `max_lines` values. - [ ] A BDD scenario (Behave) is added to `features/tui_thought_block.feature` covering the negative `max_lines` rejection. - [ ] All existing BDD scenarios in `features/tui_thought_block.feature` continue to pass. - [ ] Test coverage remains ≥ 97%. - [ ] All quality gates pass (`nox` default sessions: lint, typecheck, unit tests, integration tests, coverage). ## Subtasks - [ ] Add a failing BDD scenario to `features/tui_thought_block.feature` that asserts `ThoughtBlock(content="...", max_lines=-1)` raises a `ValidationError` (TDD — this scenario must fail before the fix is applied, tagged `@tdd_issue @tdd_issue_<N> @tdd_expected_fail`) - [ ] Add the corresponding step implementation to `features/steps/tui_thought_block_steps.py` - [ ] Apply the fix: constrain `max_lines` to `ge=0` using Pydantic's `Field(ge=0, ...)` or `Annotated[int, Field(ge=0)]` in `ThoughtBlock` - [ ] Remove `@tdd_expected_fail` from the scenario once the fix makes it pass - [ ] Run `nox` (all default sessions) and fix any errors - [ ] Verify coverage ≥ 97% via `nox -s coverage_report` - [ ] Update `CHANGELOG.md` with an entry for this fix - [ ] Update `CONTRIBUTORS.md` if applicable ## 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(tui): add non-negative validation to ThoughtBlock.max_lines field`), followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly (`fix/thought-block-max-lines-validation`). - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000 added the

labels

2026-04-14 14:22:24 +00:00

HAL9000 added this to the v3.7.0 milestone

2026-04-14 14:22:24 +00:00

HAL9000 commented

2026-04-14 14:22:24 +00:00

Author

Owner

✅ Triage: Verified [AUTO-OWNR-1]

Valid bug: ThoughtBlock.max_lines has no lower-bound validation, allowing negative values that cause unexpected behavior (Python negative slicing in visible_lines(), negative hidden_line_count(), incorrect is_truncated() results). This violates CONTRIBUTING.md's argument validation requirements.

The issue correctly follows the TDD workflow — the first subtask is to add a failing BDD scenario with @tdd_expected_fail before implementing the fix.

Note: Per CONTRIBUTING.md's Bug Fix Workflow, a separate Type/Testing TDD issue should be created first. However, since this issue already includes the TDD step as a subtask, I'm accepting it as-is. The implementer should follow the TDD workflow described in the subtasks.

Assigning to v3.7.0 (TUI Implementation) as ThoughtBlock is a TUI domain model. Priority Medium — validation gap, but negative values are unlikely in normal usage.

MoSCoW: Should Have — argument validation is required by CONTRIBUTING.md and prevents subtle bugs.

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

✅ **Triage: Verified** [AUTO-OWNR-1] Valid bug: `ThoughtBlock.max_lines` has no lower-bound validation, allowing negative values that cause unexpected behavior (Python negative slicing in `visible_lines()`, negative `hidden_line_count()`, incorrect `is_truncated()` results). This violates CONTRIBUTING.md's argument validation requirements. The issue correctly follows the TDD workflow — the first subtask is to add a failing BDD scenario with `@tdd_expected_fail` before implementing the fix. Note: Per CONTRIBUTING.md's Bug Fix Workflow, a separate `Type/Testing` TDD issue should be created first. However, since this issue already includes the TDD step as a subtask, I'm accepting it as-is. The implementer should follow the TDD workflow described in the subtasks. Assigning to **v3.7.0** (TUI Implementation) as `ThoughtBlock` is a TUI domain model. Priority **Medium** — validation gap, but negative values are unlikely in normal usage. MoSCoW: **Should Have** — argument validation is required by CONTRIBUTING.md and prevents subtle bugs. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue

2026-04-14 14:35:50 +00:00

[AUTO-GROOM] Status: Grooming Pool Supervisor (Cycle 1) #9309