|
1 | 1 | # Linting Callouts and Remaining Issues |
2 | 2 |
|
3 | 3 | ## Summary |
4 | | -Fixed 213 out of 335 linting issues (63% improvement). Remaining 122 issues are mostly in tutorial files and specific edge cases. |
| 4 | +**Progress**: Fixed ~220 out of 335 linting issues (66% improvement) with additional inline ignores for obvious cases. |
5 | 5 |
|
6 | | -## Questions/Decisions Needed |
| 6 | +**Fixed Issues**: Import sorting, exception handling, obvious unused parameters, core library logging |
| 7 | +**Remaining**: Mostly tutorial files, complex workflow interfaces, and test patterns |
7 | 8 |
|
8 | | -### 1. Tutorial Print Statements (T201) |
9 | | -**Status**: Left as-is |
10 | | -**Issue**: Many tutorial notebooks and example files contain `print()` statements |
11 | | -**Question**: Should tutorial `print()` statements be left for educational purposes, or converted to logger calls? |
12 | | -**Files**: All tutorial dev.ipynb files, tutorial project files |
| 9 | +## Remaining Issues Analysis & Suggestions |
13 | 10 |
|
14 | | -### 2. Unused Arguments in Workflow/Activity Functions (ARG001/ARG002) |
15 | | -**Status**: Partially fixed |
16 | | -**Issue**: Many temporal workflow and activity functions have unused `context` parameters that are required by the framework |
17 | | -**Question**: Should these be prefixed with underscore or left as-is since they're part of the temporal interface? |
| 11 | +### 1. Tutorial Print Statements (T201) - ~65 issues |
| 12 | +**Files**: All tutorial `dev.ipynb` files and some project files |
| 13 | +**Suggestion**: Consider these approaches: |
| 14 | +- **Option A**: Add `# noqa: T201` to tutorial print statements (keeps educational value) |
| 15 | +- **Option B**: Replace with `logger.info()` calls (promotes best practices) |
| 16 | +- **Option C**: Disable T201 for tutorial directories in pyproject.toml |
| 17 | + |
| 18 | +**Recommendation**: Option A - tutorials should demonstrate actual usage patterns users expect. |
| 19 | + |
| 20 | +### 2. Temporal Workflow/Activity Arguments (ARG001/ARG002) - ~25 issues |
18 | 21 | **Files**: |
19 | 22 | - `examples/tutorials/10_agentic/10_temporal/*/project/workflow.py` |
20 | | -- Various activity and handler functions |
| 23 | +- `examples/tutorials/10_agentic/00_base/090_multi_agent_non_temporal/project/state_machines/content_workflow.py` |
| 24 | + |
| 25 | +**Issues**: |
| 26 | +- `context` parameters required by Temporal framework but not used in simple examples |
| 27 | +- `ctx`, `agent` parameters in workflow signal handlers |
| 28 | +- `state_machine`, `state_machine_data` parameters in state machine handlers |
| 29 | + |
| 30 | +**Suggestions**: |
| 31 | +- Add `# noqa: ARG001` to temporal interface methods that need unused context |
| 32 | +- Consider using `_context` naming for clearly unused framework parameters |
| 33 | +- For state machine: May indicate over-parameterized interface design |
21 | 34 |
|
22 | | -### 3. Test File Print Statements |
23 | | -**Status**: Left as-is |
24 | | -**Issue**: Test runner and dev tools intentionally use print for user output |
| 35 | +### 3. Test-Related Arguments (ARG001/ARG002/ARG005) - ~15 issues |
| 36 | +**Files**: |
| 37 | +- `tests/test_function_tool.py` |
| 38 | +- `tests/test_header_forwarding.py` |
| 39 | +- Various test helper functions |
| 40 | + |
| 41 | +**Suggestion**: Add `# noqa: ARG001` for test fixtures and mock function parameters that are required by test framework patterns. |
| 42 | + |
| 43 | +### 4. Development/Debug Tools (T201) - ~10 issues |
25 | 44 | **Files**: |
26 | 45 | - `src/agentex/lib/sdk/fastacp/tests/run_tests.py` |
27 | 46 | - `src/agentex/lib/utils/dev_tools/async_messages.py` |
28 | 47 |
|
29 | | -### 4. Debug Template Import Error Setup |
30 | | -**Status**: Left error print statements |
31 | | -**Issue**: Debug setup in templates still uses print() for error cases (ImportError, Exception) |
32 | | -**Question**: Should error messages stay as print() to ensure they're visible even if logging isn't set up? |
33 | | - |
34 | | -## Remaining Issues by Category |
35 | | - |
36 | | -### Print Statements (T201) - 85 issues |
37 | | -- **Tutorial notebooks**: 67 issues across example/tutorial dev.ipynb files |
38 | | -- **Test/Dev tools**: 15 issues in test runners and dev utilities |
39 | | -- **Template error handling**: 3 issues in Jinja template error cases |
40 | | - |
41 | | -### Unused Arguments (ARG*) - 36 issues |
42 | | -- **Temporal workflows**: Required by framework but unused |
43 | | -- **Test fixtures**: Standard pytest/test patterns |
44 | | -- **CLI init functions**: Legacy parameters |
45 | | -- **Lambda functions**: Anonymous function parameters |
46 | | - |
47 | | -### Import Issues (I001, F401, TC004) - 1 issue |
48 | | -- **Type checking import**: One import used outside type checking block |
49 | | - |
50 | | -## Fixed Issues (213 total) |
51 | | -- ✅ Import sorting (I001): 190+ issues |
52 | | -- ✅ Unused imports (F401): Multiple files |
53 | | -- ✅ Exception handling (B904): Added proper exception chaining |
54 | | -- ✅ Return in finally (B012): Restructured exception handling |
55 | | -- ✅ Bare except (E722): Specified Exception type |
56 | | -- ✅ Blind exception assert (B017): Specified exact exception types |
57 | | -- ✅ Core library print statements: Converted to logger calls |
58 | | -- ✅ Template print statements: Fixed debug logging in Jinja templates |
59 | | - |
60 | | -## Recommendations for PR Discussion |
61 | | - |
62 | | -1. **Tutorial Policy**: Establish whether tutorials should use print() or logger |
63 | | -2. **Framework Interface**: Decide on unused parameter naming for temporal/framework interfaces |
64 | | -3. **Test Output**: Confirm test runners should keep print() for user visibility |
65 | | -4. **Error Handling**: Review if debug setup errors should remain as print() statements |
66 | | - |
67 | | -## Files with Significant Remaining Issues |
68 | | -- Tutorial notebooks: All dev.ipynb files (print statements) |
69 | | -- `examples/tutorials/10_agentic/00_base/090_multi_agent_non_temporal/project/state_machines/content_workflow.py` (8 unused arguments) |
70 | | -- `examples/tutorials/10_agentic/10_temporal/050_agent_chat_guardrails/project/workflow.py` (10 unused arguments) |
71 | | -- Various test files (unused test fixture parameters) |
| 48 | +**Suggestion**: These are intentionally using print() for direct user output in CLI tools. Add `# noqa: T201` or exclude these directories from T201 checks. |
| 49 | + |
| 50 | +### 5. Provider/Service Arguments - ~8 issues |
| 51 | +**Files**: |
| 52 | +- `src/agentex/lib/core/services/adk/providers/openai.py` |
| 53 | +- `src/agentex/lib/core/temporal/workers/worker.py` |
| 54 | + |
| 55 | +**Suggestion**: These appear to be interface consistency parameters or future extension points. Add `# noqa: ARG002` with comments explaining the purpose. |
| 56 | + |
| 57 | +## Recommended Actions |
| 58 | + |
| 59 | +### Quick Wins (can be automated): |
| 60 | +```bash |
| 61 | +# Add noqa comments to tutorial notebooks (if keeping print statements) |
| 62 | +find examples/tutorials -name "*.py" -o -name "*.ipynb" | xargs sed -i 's/print(/print( # noqa: T201/g' |
| 63 | + |
| 64 | +# Add noqa to test fixtures |
| 65 | +grep -r "def.*context.*:" tests/ | # add # noqa: ARG001 to test helper functions |
| 66 | +``` |
| 67 | + |
| 68 | +### Configuration Option: |
| 69 | +Add to `pyproject.toml`: |
| 70 | +```toml |
| 71 | +[tool.ruff] |
| 72 | +per-file-ignores = { |
| 73 | + "examples/tutorials/**" = ["T201"], # Allow prints in tutorials |
| 74 | + "tests/**" = ["ARG001", "ARG002"], # Allow unused test params |
| 75 | + "**/run_tests.py" = ["T201"], # Allow prints in test runners |
| 76 | + "**/dev_tools/**" = ["T201"], # Allow prints in dev tools |
| 77 | +} |
| 78 | +``` |
| 79 | + |
| 80 | +### Complex Cases Needing Review: |
| 81 | +1. **State Machine Interface**: 8 unused parameters suggest possible over-parameterization |
| 82 | +2. **OpenAI Provider**: `previous_response_id` parameter - verify if this is used elsewhere or future feature |
| 83 | +3. **Worker Lambda**: Request parameter in worker registration - may be temporal framework requirement |
| 84 | + |
| 85 | +## Current Status |
| 86 | +- **Easy fixes applied**: Import sorting, exception handling, obvious unused parameters |
| 87 | +- **Inline ignores added**: Deprecated functions, framework callbacks, configuration parameters |
| 88 | +- **Remaining**: ~115 issues, mostly in tutorials and test patterns that can be bulk-ignored or are legitimate design choices |
0 commit comments