Refactor
This commit is contained in:
235
docs/QA-2026-04-11_1132.md
Normal file
235
docs/QA-2026-04-11_1132.md
Normal file
@@ -0,0 +1,235 @@
|
||||
# Quality Assessment -- 2026-04-11
|
||||
|
||||
> **Score: 83.7 / 100 — Grade: B+**
|
||||
> Codebase remains at B+ with no material change (-0.0 points) from the previous assessment; all eight dimension scores held steady — lint is still perfect, architecture is fully exposed, and the remaining paths to A-grade (coverage ≥80%, type errors <10, fewer files >300 lines) are the same priority items identified last cycle.
|
||||
|
||||
| Dimension | Score |
|
||||
|-----------|-------|
|
||||
| Test Health | 8.0/10 |
|
||||
| Type Safety | 6.8/10 |
|
||||
| Lint Hygiene | 10.0/10 |
|
||||
| Architecture | 10.0/10 |
|
||||
| Documentation | 9.0/10 |
|
||||
| Complexity | 6.5/10 |
|
||||
| Security | 8.5/10 |
|
||||
| Maintainability | 8.5/10 |
|
||||
|
||||
**Version:** 0.1.0
|
||||
**Assessed by:** Claude Sonnet 4.6
|
||||
**Previous assessment:** QA-2026-04-11_1054.md
|
||||
|
||||
---
|
||||
|
||||
## Inventory
|
||||
|
||||
| Metric | Value |
|
||||
|--------|-------|
|
||||
| Source files | 72 |
|
||||
| Source lines | 10,575 |
|
||||
| Test files | 30 |
|
||||
| Test lines | 2,736 |
|
||||
| Test:source ratio | 0.259 |
|
||||
| Direct dependencies | 10 core + 1 optional + 4 dev |
|
||||
|
||||
---
|
||||
|
||||
## Raw Metrics
|
||||
|
||||
### Test Suite
|
||||
|
||||
```
|
||||
src/impakt/web/components/transforms.py 18 11 39% 102-164
|
||||
src/impakt/web/state.py 152 11 93% 86, 96, 102, 112, 150, 201, 217, 223, 273-275
|
||||
------------------------------------------------------------------------------
|
||||
TOTAL 3763 1150 69%
|
||||
|
||||
20 files skipped due to complete coverage.
|
||||
Required test coverage of 60% reached. Total coverage: 69.44%
|
||||
240 passed, 7 warnings in 16.77s
|
||||
```
|
||||
|
||||
- Tests collected: 240
|
||||
- Tests passed: 240
|
||||
- Tests failed: 0
|
||||
- Test duration: 16.77s
|
||||
- Coverage: 69.44% (60% floor enforced via pytest-cov)
|
||||
|
||||
### Type Safety (mypy --ignore-missing-imports)
|
||||
|
||||
```
|
||||
src/impakt/web/callbacks/channel_callbacks.py:103: error: Incompatible return value type (got "NoReturn", expected "str | None") [return-value]
|
||||
src/impakt/web/callbacks/export_callbacks.py:85: error: Module "dash.dcc" does not explicitly export attribute "send_data_frame" [attr-defined]
|
||||
src/impakt/web/callbacks/export_callbacks.py:85: error: Call to untyped function "send_data_frame" in typed context [no-untyped-call]
|
||||
src/impakt/web/callbacks/cursor_callbacks.py:25: error: Call to untyped function "clientside_callback" in typed context [no-untyped-call]
|
||||
Found 34 errors in 16 files (checked 72 source files)
|
||||
```
|
||||
|
||||
- Total errors: 34 (unchanged from previous)
|
||||
- Files with errors: 16 / 72 (78% clean)
|
||||
- Top error categories:
|
||||
- `[attr-defined]` 9 — attribute access on loosely typed objects (Dash stubs)
|
||||
- `[no-any-return]` 6 — returning `Any` from typed functions
|
||||
- `[return-value]` 4 — incompatible return type
|
||||
- `[import-untyped]` 3 — third-party stubs missing
|
||||
- `[assignment]` 3 — incompatible assignment
|
||||
- `[var-annotated]` 2 — need type annotation
|
||||
- `[valid-type]` 2 — invalid type expression
|
||||
- `[no-untyped-call]` 2 — call to untyped function
|
||||
- `[unused-ignore]` 1
|
||||
- `[no-untyped-def]` 1
|
||||
- `[comparison-overlap]` 1
|
||||
|
||||
### Lint (ruff)
|
||||
|
||||
```
|
||||
All checks passed!
|
||||
```
|
||||
|
||||
- Total violations: 0
|
||||
- Auto-fixable: N/A
|
||||
- Top violation rules: none
|
||||
|
||||
### Complexity
|
||||
|
||||
- File size: min=1 / median=132 / mean=147 / max=692
|
||||
- Files >300 lines: 8 / 72
|
||||
- High-complexity files (branch density >15):
|
||||
|
||||
```
|
||||
80 src/impakt/io/mme.py (692 lines) -- ISO 13499 parser, justified
|
||||
44 src/impakt/web/components/criteria.py (342 lines) -- UI assembly with protocol logic
|
||||
32 src/impakt/web/callbacks/plot_callbacks.py (317 lines) -- transform pipeline orchestration
|
||||
30 src/impakt/channel/model.py (456 lines) -- core data model, multiple classes
|
||||
27 src/impakt/web/state.py (275 lines) -- app state with multi-test support
|
||||
27 src/impakt/protocol/euro_ncap.py (237 lines) -- sliding-scale scoring tables
|
||||
26 src/impakt/plot/engine.py (316 lines) -- Plotly rendering with corridors
|
||||
21 src/impakt/web/callbacks/channel_callbacks.py (236 lines) -- selection/filter callbacks
|
||||
21 src/impakt/protocol/iihs.py (179 lines) -- G/A/M/P rating logic
|
||||
20 src/impakt/web/components/channel_grid.py (418 lines) -- DataTable assembly
|
||||
19 src/impakt/script/cli.py (137 lines) -- CLI arg parsing
|
||||
```
|
||||
|
||||
### Documentation
|
||||
|
||||
- Docstring coverage: 420 / 457 definitions (91.9%)
|
||||
- Modules with `__all__`: 11 / 11 (all modules)
|
||||
- channel: YES
|
||||
- criteria: YES
|
||||
- io: YES
|
||||
- plot: YES
|
||||
- plugin: YES
|
||||
- protocol: YES
|
||||
- report: YES
|
||||
- script: YES
|
||||
- template: YES
|
||||
- transform: YES
|
||||
- web: YES
|
||||
- README: 1,266 lines with Mermaid diagrams
|
||||
- Architectural diagrams: yes
|
||||
|
||||
### Security
|
||||
|
||||
```
|
||||
eval/exec (all hits):
|
||||
src/impakt/transform/math_expr.py:149: eval(self.expression, {"__builtins__": {}}, namespace) # noqa: S307
|
||||
|
||||
subprocess/os.system hits:
|
||||
src/impakt/transform/math_expr.py:68: "subprocess", ← string in forbidden-token blocklist, NOT a call
|
||||
```
|
||||
|
||||
- eval/exec (sandboxed): 1 — `math_expr.py:149`, restricted builtins `{}` + 16-token blocklist; noqa: S307
|
||||
- eval/exec (unsandboxed): 0 (grep excluding # noqa returns 0)
|
||||
- subprocess: 1 confirmed false positive — line 68 is a blocklist string entry, not an invocation
|
||||
- Hardcoded secrets: 0
|
||||
- Bare except: 0
|
||||
|
||||
### Maintainability
|
||||
|
||||
```
|
||||
TODO: 0
|
||||
FIXME: 0
|
||||
HACK: 0
|
||||
logging calls: 50
|
||||
try/except blocks: 53
|
||||
bare excepts: 0
|
||||
internal imports: 198
|
||||
```
|
||||
|
||||
- TODO: 0
|
||||
- FIXME: 0
|
||||
- HACK: 0
|
||||
- Logging calls: 50
|
||||
- try/except blocks: 53
|
||||
- Bare excepts: 0
|
||||
- Internal imports (coupling): 198
|
||||
|
||||
---
|
||||
|
||||
## Scorecard
|
||||
|
||||
| # | Dimension | Weight | Score | Weighted | Justification |
|
||||
|---|-----------|--------|-------|----------|---------------|
|
||||
| 1 | Test Health | 20% | 8.0/10 | 1.60 | 240/240 pass; test:source ratio 0.259 (within 0.2–0.5 band); integration tests with real datasets present; 69.44% coverage measured (below 80% ceiling for a 9+). Matches rubric row "100% pass, ratio 0.2–0.5, integration tests present" = 8. |
|
||||
| 2 | Type Safety | 15% | 6.8/10 | 1.02 | mypy (ignore-missing-imports), 34 errors in 16 files — unchanged. Linear interpolation between 6 (<50 errors) and 8 (<10 errors): 6 + (50−34)/(50−10) × 2 = 6.8. |
|
||||
| 3 | Lint Hygiene | 10% | 10.0/10 | 1.00 | `ruff check src/` reports 0 violations — perfect score maintained. |
|
||||
| 4 | Architecture | 15% | 10.0/10 | 1.50 | Clear 4-layer design (io → transform/channel → protocol → web/plot). Plugin system present. All 11 public modules export `__all__`. No layer violations confirmed (io/transform/protocol/channel contain no imports from web or plot). |
|
||||
| 5 | Documentation | 10% | 9.0/10 | 0.90 | 91.9% docstring coverage exceeds 90% threshold. Comprehensive README with Mermaid diagrams. No generated API reference (Sphinx/mkdocs), so not a full 10. |
|
||||
| 6 | Complexity | 10% | 6.5/10 | 0.65 | Median 132 (<150, excellent). 8 files >300 lines (unchanged). Interpolated between score 8 (≤3 files) and 6 (≤10 files): 6 + (10−8)/(10−3) × 2 = 6.57 → 6.5. |
|
||||
| 7 | Security | 10% | 8.5/10 | 0.85 | Single eval sandboxed with `{"__builtins__": {}}` + 16-item token blocklist; noqa annotation in place. Subprocess hit confirmed false positive. No secrets, no bare excepts. Interpolated between 9 (fully sandboxed) and 7 (partially sandboxed) for conservative assessment. |
|
||||
| 8 | Maintainability | 10% | 8.5/10 | 0.85 | Zero debt markers. Zero bare excepts. 50 logging calls. Modern tooling (uv, hatchling, ruff, mypy). Interpolated between 10 (0 markers + all criteria) and 8 (<5 markers) at 8.5 for consistency with prior assessments; high internal coupling (198) noted. |
|
||||
|
||||
### Composite Score: **83.7 / 100**
|
||||
### Grade: **B+**
|
||||
|
||||
Calculation: (8.0×0.20 + 6.8×0.15 + 10.0×0.10 + 10.0×0.15 + 9.0×0.10 + 6.5×0.10 + 8.5×0.10 + 8.5×0.10) × 10
|
||||
= (1.60 + 1.02 + 1.00 + 1.50 + 0.90 + 0.65 + 0.85 + 0.85) × 10
|
||||
= 8.37 × 10 = **83.7**
|
||||
|
||||
---
|
||||
|
||||
## Delta from Previous Assessment
|
||||
|
||||
| Dimension | Previous | Current | Change |
|
||||
|-----------|----------|---------|--------|
|
||||
| Test Health | 8.0 | 8.0 | 0.0 |
|
||||
| Type Safety | 6.8 | 6.8 | 0.0 |
|
||||
| Lint Hygiene | 10.0 | 10.0 | 0.0 |
|
||||
| Architecture | 10.0 | 10.0 | 0.0 |
|
||||
| Documentation | 9.0 | 9.0 | 0.0 |
|
||||
| Complexity | 6.5 | 6.5 | 0.0 |
|
||||
| Security | 8.5 | 8.5 | 0.0 |
|
||||
| Maintainability | 8.5 | 8.5 | 0.0 |
|
||||
| **Composite** | **83.7** | **83.7** | **0.0** |
|
||||
|
||||
---
|
||||
|
||||
## Top Improvements Since Last Assessment
|
||||
|
||||
1. **No regression** — All dimensions held steady; the 8-file >300-lines count is unchanged, coverage held at ~69.4%, and 34 mypy errors remain.
|
||||
2. **Minor source growth** — +12 lines (10,563 → 10,575) with no new files or test changes; composition is stable.
|
||||
3. **Coverage delta negligible** — 69.51% → 69.44% (−0.07 pp); within measurement noise.
|
||||
|
||||
---
|
||||
|
||||
## Recommended Actions (Priority Order)
|
||||
|
||||
| # | Action | Effort | Impact | Dimensions Affected |
|
||||
|---|--------|--------|--------|---------------------|
|
||||
| 1 | Increase test coverage from 69.44% to ≥80%: add unit tests for uncovered branches in `web/components/transforms.py` (39% covered), `plot/engine.py`, and `io/mme.py` parser edge-cases | 2–4 hr | Test Health +1.0 → 9.0 (+0.20 composite) | Test Health |
|
||||
| 2 | Resolve 9 `[attr-defined]` + 6 `[no-any-return]` + 4 `[return-value]` mypy errors in the web layer to reach <10 total errors; consider adding Dash type stubs or local overrides | 1–2 hr | Type Safety +1.2 → 8.0 (+0.18 composite) | Type Safety |
|
||||
| 3 | Decompose `channel_grid.py` (418 lines) and `channel/model.py` (456 lines) into focused sub-modules; extract sub-parsers from `mme.py` if it grows beyond 800 lines | 3–5 hr | Complexity +0.5 → 7.0 (+0.05 composite) | Complexity |
|
||||
| 4 | Set up mkdocs-material + mkdocstrings to auto-generate API reference from existing docstrings (91.9% coverage makes this low-friction) | 1–2 hr | Documentation +1.0 → 10.0 (+0.10 composite) | Documentation |
|
||||
| 5 | Replace `eval`-based math expression evaluator in `math_expr.py` with an AST-based parser (`ast.parse` + safe node visitor) to eliminate last eval usage entirely | 2–3 hr | Security +0.5 → 9.0 (+0.05 composite) | Security |
|
||||
|
||||
**Projected composite after actions 1–4: ~87.0 (B+, approaching A threshold); after all 5: ~87.5 (B+)**
|
||||
|
||||
---
|
||||
|
||||
## Notes
|
||||
|
||||
- **All scores unchanged from QA-2026-04-11_1054.md.** The codebase added 12 source lines with no structural changes. This is a stability confirmation assessment, not a milestone improvement run.
|
||||
- **Architecture qualitatively verified.** All 11 `__init__.py` files confirmed present with `__all__`. Grep for web/plot imports in io/transform/channel/protocol layers returned zero hits — no layer violations exist.
|
||||
- **Security subprocess false positive persists.** `math_expr.py:68` contains `"subprocess"` as a string entry in its forbidden-token blocklist, not an invocation. The grep count of 1 is expected and benign.
|
||||
- **eval sandboxing confirmed.** The single `eval` call at `math_expr.py:149` uses `{"__builtins__": {}}` (removes all builtins) plus the 16-token text blocklist scanned before evaluation. The `# noqa: S307` suppresses the ruff/bandit rule correctly.
|
||||
- **Coverage measurement note.** The `--co` (collect-only) run reports 30.67% because 11 files are skipped during collection; the full run gives 69.44%. Use only the full-run number for scoring.
|
||||
- **Next A-grade path.** Reaching 90+ requires: coverage ≥80% (+0.20), type errors <10 (+0.18), and generated API docs (+0.10) = minimum +0.48 composite → ~87–88. Getting to 90 additionally requires complexity improvement (more decomposition) or maintainability nudge.
|
||||
Reference in New Issue
Block a user