fix(ci): resolve lint errors and remove stale tdd_expected_fail tag #5264

Merged
hurui200320 merged 1 commit from fix/broken-ci into master 2026-04-09 04:41:01 +00:00
Member

Summary

The lint and integration_tests CI jobs on master are currently failing. This PR fixes both:

  1. Lint (nox -e lint) — 51 ruff violations in scripts/validate_automation_tracking.py:

    • Sorted imports per isort convention
    • Replaced deprecated typing.List/Dict/Tuple/Optional with builtin equivalents (list, dict, tuple)
    • Removed unused Optional import and unused local variable issues
    • Fixed trailing whitespace, blank-line whitespace, and 6 lines exceeding 88-char limit
    • Replaced dict.keys() with dict in membership tests
    • Added return type annotation to main()
  2. Integration tests (nox -e integration_tests) — 1 failure in robot/coverage_threshold.robot:

    • Removed tdd_expected_fail, tdd_issue, and tdd_issue_4305 tags from the Noxfile Contains Coverage Threshold Constant test
    • The bug this TDD tag tracked (issue #4305) is now fixed — COVERAGE_THRESHOLD = 97 exists in noxfile.py
    • The TDD expected-fail listener was inverting the passing result to a failure

Local verification

  • nox -e lint — All checks passed
  • nox -e integration_tests — 1962 tests, 1962 passed, 0 failed, 0 skipped

Files changed

File Change
scripts/validate_automation_tracking.py Fixed all 51 lint violations
robot/coverage_threshold.robot Removed stale tdd_expected_fail tag

Closes #5266

## Summary The `lint` and `integration_tests` CI jobs on `master` are currently failing. This PR fixes both: 1. **Lint (`nox -e lint`)** — 51 ruff violations in `scripts/validate_automation_tracking.py`: - Sorted imports per isort convention - Replaced deprecated `typing.List`/`Dict`/`Tuple`/`Optional` with builtin equivalents (`list`, `dict`, `tuple`) - Removed unused `Optional` import and unused local variable `issues` - Fixed trailing whitespace, blank-line whitespace, and 6 lines exceeding 88-char limit - Replaced `dict.keys()` with `dict` in membership tests - Added return type annotation to `main()` 2. **Integration tests (`nox -e integration_tests`)** — 1 failure in `robot/coverage_threshold.robot`: - Removed `tdd_expected_fail`, `tdd_issue`, and `tdd_issue_4305` tags from the `Noxfile Contains Coverage Threshold Constant` test - The bug this TDD tag tracked (issue #4305) is now fixed — `COVERAGE_THRESHOLD = 97` exists in `noxfile.py` - The TDD expected-fail listener was inverting the passing result to a failure ### Local verification - `nox -e lint` — All checks passed - `nox -e integration_tests` — 1962 tests, 1962 passed, 0 failed, 0 skipped ### Files changed | File | Change | |------|--------| | `scripts/validate_automation_tracking.py` | Fixed all 51 lint violations | | `robot/coverage_threshold.robot` | Removed stale `tdd_expected_fail` tag | Closes #5266
fix(ci): resolve lint errors and remove stale tdd_expected_fail tag
All checks were successful
CI / lint (pull_request) Successful in 40s
CI / quality (pull_request) Successful in 43s
CI / security (pull_request) Successful in 49s
CI / typecheck (pull_request) Successful in 51s
CI / push-validation (pull_request) Successful in 20s
CI / build (pull_request) Successful in 31s
CI / helm (pull_request) Successful in 1m0s
CI / e2e_tests (pull_request) Successful in 3m13s
CI / integration_tests (pull_request) Successful in 6m27s
CI / unit_tests (pull_request) Successful in 7m32s
CI / docker (pull_request) Successful in 1m33s
CI / coverage (pull_request) Successful in 10m32s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 57m26s
77c29238c4
Fixed 51 ruff lint violations in scripts/validate_automation_tracking.py:
- Sorted imports per isort convention
- Replaced deprecated typing.List/Dict/Tuple/Optional with builtin equivalents
- Removed unused Optional import and unused local variable
- Fixed trailing whitespace, blank-line whitespace, and line-too-long errors
- Replaced dict.keys() with dict in membership tests

Removed tdd_expected_fail tag from Robot.Coverage Threshold test
'Noxfile Contains Coverage Threshold Constant' (issue #4305) because
the bug it tracked is now fixed (COVERAGE_THRESHOLD = 97 exists in
noxfile.py). The TDD listener was inverting the passing result to a
failure.
Author
Member

@HAL9000 please review this PR asap since it tries to get the CI back into a working state.

@HAL9000 please review this PR asap since it tries to get the CI back into a working state.
HAL9000 left a comment

PR Review — fix(ci): resolve lint errors and remove stale tdd_expected_fail tag

This PR addresses the two active master CI failures. The changes are correct and well-scoped.

Lint Fix (scripts/validate_automation_tracking.py)

The 51 ruff violations are standard modernization fixes:

  • Import sorting and removal of unused Optional import
  • Migration from deprecated typing.List/Dict/Tuple/Optional to builtin equivalents — correct per Python 3.9+ conventions
  • Line length, whitespace, and dict.keys() membership test fixes

These are all mechanical correctness fixes with no behavioral change. The approach is correct.

TDD Tag Removal (robot/coverage_threshold.robot)

Removing tdd_expected_fail, tdd_issue, and tdd_issue_4305 from the Noxfile Contains Coverage Threshold Constant test is the correct action. Per our TDD workflow:

  • tdd_expected_fail must be removed in the same commit that fixes the underlying bug
  • Issue #4305 is resolved — COVERAGE_THRESHOLD = 97 exists in noxfile.py
  • The tag was inverting a passing test to a failure, which is the expected behavior while the bug existed

Removing all three TDD tags together is correct procedure.

Assessment

The PR is well-formed, the changes are correct, and local verification was performed (1962 tests, 0 failures). CI checks are currently pending.

Recommendation: Merge once CI passes.


Automated by CleverAgents Bot
Supervisor: Human Liaison | Agent: human-liaison

## PR Review — fix(ci): resolve lint errors and remove stale tdd_expected_fail tag This PR addresses the two active master CI failures. The changes are correct and well-scoped. ### Lint Fix (`scripts/validate_automation_tracking.py`) The 51 ruff violations are standard modernization fixes: - Import sorting and removal of unused `Optional` import - Migration from deprecated `typing.List`/`Dict`/`Tuple`/`Optional` to builtin equivalents — correct per Python 3.9+ conventions - Line length, whitespace, and `dict.keys()` membership test fixes These are all mechanical correctness fixes with no behavioral change. The approach is correct. ### TDD Tag Removal (`robot/coverage_threshold.robot`) Removing `tdd_expected_fail`, `tdd_issue`, and `tdd_issue_4305` from the `Noxfile Contains Coverage Threshold Constant` test is the correct action. Per our TDD workflow: - `tdd_expected_fail` must be removed in the same commit that fixes the underlying bug - Issue #4305 is resolved — `COVERAGE_THRESHOLD = 97` exists in `noxfile.py` - The tag was inverting a passing test to a failure, which is the expected behavior while the bug existed Removing all three TDD tags together is correct procedure. ### Assessment The PR is well-formed, the changes are correct, and local verification was performed (1962 tests, 0 failures). CI checks are currently pending. **Recommendation**: Merge once CI passes. --- **Automated by CleverAgents Bot** Supervisor: Human Liaison | Agent: human-liaison
HAL9000 left a comment

Review Summary

Reviewed PR with focus on specification-compliance, requirements-coverage, and behavior-correctness.

The lint fixes in scripts/validate_automation_tracking.py are correct and well-executed. However, there is a critical TDD tag violation in robot/coverage_threshold.robot, plus two missing required PR metadata fields.


Required Changes

1. [CRITICAL] TDD Regression Markers Incorrectly Removed

Location: robot/coverage_threshold.robotNoxfile Contains Coverage Threshold Constant test

Issue: The PR removed ALL three TDD tags from this test, but only tdd_expected_fail should be removed. The tdd_issue and tdd_issue_4305 tags are permanent regression markers and must remain.

Master (before):

Noxfile Contains Coverage Threshold Constant
    [Documentation]    Verify COVERAGE_THRESHOLD = 97 is defined in noxfile.py
    [Tags]    coverage    config    tdd_issue    tdd_issue_4305    tdd_expected_fail

This PR (after):

Noxfile Contains Coverage Threshold Constant
    [Documentation]    Verify COVERAGE_THRESHOLD = 97 is defined in noxfile.py
    [Tags]    coverage    config

Required (correct):

Noxfile Contains Coverage Threshold Constant
    [Documentation]    Verify COVERAGE_THRESHOLD = 97 is defined in noxfile.py
    [Tags]    coverage    config    tdd_issue    tdd_issue_4305

Why this matters: Per CONTRIBUTING.md (Bug Fix Workflow / TDD Issue Test Tags):

@tdd_issue and @tdd_issue_<N> should remain (permanent regression markers)
@tdd_expected_fail MUST be removed for the bug being fixed

The tdd_issue and tdd_issue_4305 tags serve as permanent regression guards. If COVERAGE_THRESHOLD = 97 is ever removed from noxfile.py in the future, these tags ensure the test is still identifiable as a TDD regression test for issue #4305. Removing them silently strips that traceability and regression protection.


2. [REQUIRED] Missing Milestone on PR

Reference: CONTRIBUTING.md — Pull Request Process, item 11:

"Every PR must be assigned to the same milestone as its linked issues."

Issue #5266 is assigned to milestone v3.2.0. This PR has no milestone assigned. Please assign milestone v3.2.0 to this PR.


3. [REQUIRED] Missing Type/ Label on PR

Reference: CONTRIBUTING.md — Pull Request Process, item 12:

"Every PR must carry exactly one Type/ label that matches the nature of the change."

This PR has no labels at all. Given this is a bug/CI fix, the appropriate label is Type/Bug. Please apply it.


⚠️ Concern (Non-blocking but should be addressed)

Issue #4305 Still Open

The PR description states: "The bug this TDD tag tracked (issue #4305) is now fixed — COVERAGE_THRESHOLD = 97 exists in noxfile.py."

However, issue #4305 (fix(integration): coverage_threshold — 1 failing test(s)) is still open with State/Verified. If the underlying fix is already merged to master, issue #4305 should be closed. Consider either:

  • Adding Closes #4305 to this PR's description (if this PR is the appropriate place to close it), or
  • Closing issue #4305 separately with a comment explaining it was resolved by the prior commit that added COVERAGE_THRESHOLD = 97 to noxfile.py

What's Good

  • Lint fixes are correct and complete: All 51 ruff violations in scripts/validate_automation_tracking.py are properly addressed:
    • Deprecated typing.List/Dict/Tuple/Optional replaced with builtin equivalents (list, dict, tuple)
    • Unused Optional import removed
    • Return type annotation added to main() -> int
    • Import ordering fixed
    • Line length, trailing whitespace, and blank-line whitespace corrected
    • dict.keys() membership test replaced with dict directly
  • No # type: ignore suppressions
  • File is under 500 lines (7,653 bytes ≈ ~200 lines)
  • Commit message format follows Conventional Changelog: fix(ci): resolve lint errors and remove stale tdd_expected_fail tag
  • Closing keyword present: Closes #5266
  • PR description is detailed and explains both changes clearly
  • The tdd_expected_fail removal itself is correctCOVERAGE_THRESHOLD = 97 is confirmed present in noxfile.py on master, so the TDD listener was correctly inverting a passing test to a failure

Summary of Required Changes

# Severity Location Action Required
1 🔴 Critical robot/coverage_threshold.robot Restore tdd_issue and tdd_issue_4305 tags; keep only tdd_expected_fail removed
2 🔴 Required PR metadata Assign milestone v3.2.0
3 🔴 Required PR metadata Apply Type/Bug label
4 🟡 Concern Issue #4305 Close issue #4305 or explain why it remains open

Decision: REQUEST CHANGES 🔄

The lint fixes are solid and the CI motivation is urgent — but the TDD tag removal is incorrect and would silently eliminate permanent regression protection for issue #4305. The fix is a one-line change: restore tdd_issue and tdd_issue_4305 to the Noxfile Contains Coverage Threshold Constant test while keeping tdd_expected_fail removed.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-self-reviewer

## Review Summary Reviewed PR with focus on **specification-compliance**, **requirements-coverage**, and **behavior-correctness**. The lint fixes in `scripts/validate_automation_tracking.py` are correct and well-executed. However, there is a **critical TDD tag violation** in `robot/coverage_threshold.robot`, plus two missing required PR metadata fields. --- ### ❌ Required Changes #### 1. [CRITICAL] TDD Regression Markers Incorrectly Removed **Location:** `robot/coverage_threshold.robot` — `Noxfile Contains Coverage Threshold Constant` test **Issue:** The PR removed ALL three TDD tags from this test, but only `tdd_expected_fail` should be removed. The `tdd_issue` and `tdd_issue_4305` tags are **permanent regression markers** and must remain. **Master (before):** ```robot Noxfile Contains Coverage Threshold Constant [Documentation] Verify COVERAGE_THRESHOLD = 97 is defined in noxfile.py [Tags] coverage config tdd_issue tdd_issue_4305 tdd_expected_fail ``` **This PR (after):** ```robot Noxfile Contains Coverage Threshold Constant [Documentation] Verify COVERAGE_THRESHOLD = 97 is defined in noxfile.py [Tags] coverage config ``` **Required (correct):** ```robot Noxfile Contains Coverage Threshold Constant [Documentation] Verify COVERAGE_THRESHOLD = 97 is defined in noxfile.py [Tags] coverage config tdd_issue tdd_issue_4305 ``` **Why this matters:** Per CONTRIBUTING.md (Bug Fix Workflow / TDD Issue Test Tags): > `@tdd_issue` and `@tdd_issue_<N>` should remain (permanent regression markers) > `@tdd_expected_fail` MUST be removed for the bug being fixed The `tdd_issue` and `tdd_issue_4305` tags serve as permanent regression guards. If `COVERAGE_THRESHOLD = 97` is ever removed from `noxfile.py` in the future, these tags ensure the test is still identifiable as a TDD regression test for issue #4305. Removing them silently strips that traceability and regression protection. --- #### 2. [REQUIRED] Missing Milestone on PR **Reference:** CONTRIBUTING.md — Pull Request Process, item 11: > "Every PR must be assigned to the same milestone as its linked issues." Issue #5266 is assigned to milestone **v3.2.0**. This PR has **no milestone** assigned. Please assign milestone v3.2.0 to this PR. --- #### 3. [REQUIRED] Missing Type/ Label on PR **Reference:** CONTRIBUTING.md — Pull Request Process, item 12: > "Every PR must carry exactly one `Type/` label that matches the nature of the change." This PR has **no labels at all**. Given this is a bug/CI fix, the appropriate label is `Type/Bug`. Please apply it. --- ### ⚠️ Concern (Non-blocking but should be addressed) #### Issue #4305 Still Open The PR description states: *"The bug this TDD tag tracked (issue #4305) is now fixed — `COVERAGE_THRESHOLD = 97` exists in `noxfile.py`."* However, issue #4305 (`fix(integration): coverage_threshold — 1 failing test(s)`) is still **open** with `State/Verified`. If the underlying fix is already merged to master, issue #4305 should be closed. Consider either: - Adding `Closes #4305` to this PR's description (if this PR is the appropriate place to close it), or - Closing issue #4305 separately with a comment explaining it was resolved by the prior commit that added `COVERAGE_THRESHOLD = 97` to `noxfile.py` --- ### ✅ What's Good - **Lint fixes are correct and complete:** All 51 ruff violations in `scripts/validate_automation_tracking.py` are properly addressed: - Deprecated `typing.List/Dict/Tuple/Optional` replaced with builtin equivalents (`list`, `dict`, `tuple`) ✅ - Unused `Optional` import removed ✅ - Return type annotation added to `main() -> int` ✅ - Import ordering fixed ✅ - Line length, trailing whitespace, and blank-line whitespace corrected ✅ - `dict.keys()` membership test replaced with `dict` directly ✅ - **No `# type: ignore` suppressions** ✅ - **File is under 500 lines** (7,653 bytes ≈ ~200 lines) ✅ - **Commit message format** follows Conventional Changelog: `fix(ci): resolve lint errors and remove stale tdd_expected_fail tag` ✅ - **Closing keyword** present: `Closes #5266` ✅ - **PR description** is detailed and explains both changes clearly ✅ - **The `tdd_expected_fail` removal itself is correct** — `COVERAGE_THRESHOLD = 97` is confirmed present in `noxfile.py` on master, so the TDD listener was correctly inverting a passing test to a failure ✅ --- ### Summary of Required Changes | # | Severity | Location | Action Required | |---|----------|----------|-----------------| | 1 | 🔴 Critical | `robot/coverage_threshold.robot` | Restore `tdd_issue` and `tdd_issue_4305` tags; keep only `tdd_expected_fail` removed | | 2 | 🔴 Required | PR metadata | Assign milestone v3.2.0 | | 3 | 🔴 Required | PR metadata | Apply `Type/Bug` label | | 4 | 🟡 Concern | Issue #4305 | Close issue #4305 or explain why it remains open | **Decision: REQUEST CHANGES** 🔄 The lint fixes are solid and the CI motivation is urgent — but the TDD tag removal is incorrect and would silently eliminate permanent regression protection for issue #4305. The fix is a one-line change: restore `tdd_issue` and `tdd_issue_4305` to the `Noxfile Contains Coverage Threshold Constant` test while keeping `tdd_expected_fail` removed. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: pr-self-reviewer
@ -9,7 +9,7 @@ Suite Teardown Cleanup Test Environment
*** Test Cases ***
Noxfile Contains Coverage Threshold Constant
Owner

TDD Regression Markers Incorrectly Removed

Only tdd_expected_fail should be removed when a bug is fixed. The tdd_issue and tdd_issue_4305 tags are permanent regression markers and must remain.

Current (incorrect):

[Tags]    coverage    config

Required (correct):

[Tags]    coverage    config    tdd_issue    tdd_issue_4305

Per CONTRIBUTING.md TDD Issue Test Tags:

@tdd_issue and @tdd_issue_<N> should remain (permanent regression markers)
@tdd_expected_fail MUST be removed for the bug being fixed

**TDD Regression Markers Incorrectly Removed** Only `tdd_expected_fail` should be removed when a bug is fixed. The `tdd_issue` and `tdd_issue_4305` tags are **permanent regression markers** and must remain. Current (incorrect): ```robot [Tags] coverage config ``` Required (correct): ```robot [Tags] coverage config tdd_issue tdd_issue_4305 ``` Per CONTRIBUTING.md TDD Issue Test Tags: > `@tdd_issue` and `@tdd_issue_<N>` should remain (permanent regression markers) > `@tdd_expected_fail` MUST be removed for the bug being fixed
hurui200320 added this to the v3.2.0 milestone 2026-04-09 04:39:13 +00:00
hurui200320 deleted branch fix/broken-ci 2026-04-09 04:41:01 +00:00
Author
Member

For the record, I'm merging this PR anyway since:

  • The config validation is not really a TDD bug, Jeff (freemo) temporarily lower the coverage to 50% to fix CI, but then switched back to 97% percent. This is not a bug, more like a config issue
  • Missing labels are fixed
  • #4305 has been closed

I see no harm on merging this, instead, merging this should bring the CI back to a working state so it will unblock other issues and PRs.

For the record, I'm merging this PR anyway since: + The config validation is not really a TDD bug, Jeff (freemo) temporarily lower the coverage to 50% to fix CI, but then switched back to 97% percent. This is not a bug, more like a config issue + Missing labels are fixed + #4305 has been closed I see no harm on merging this, instead, merging this should bring the CI back to a working state so it will unblock other issues and PRs.
Owner

PR merged. The master CI fix is now in. Thank you @hurui200320 for the clear explanation and for resolving this promptly.

The reasoning is sound:

  • The coverage threshold change was a temporary config adjustment by @freemo, not a persistent bug
  • Issue #4305 was already closed
  • The stale tdd_expected_fail tag was the only remaining artifact

Master CI should now pass on the next run.


Automated by CleverAgents Bot
Supervisor: Human Liaison | Agent: human-liaison

PR merged. The master CI fix is now in. Thank you @hurui200320 for the clear explanation and for resolving this promptly. The reasoning is sound: - The coverage threshold change was a temporary config adjustment by @freemo, not a persistent bug - Issue #4305 was already closed - The stale `tdd_expected_fail` tag was the only remaining artifact Master CI should now pass on the next run. --- **Automated by CleverAgents Bot** Supervisor: Human Liaison | Agent: human-liaison
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core!5264
No description provided.