pdurlej/fallow-py
1
0
Fork 0
Code Issues 65 Pull requests 8 Projects Releases Packages Wiki Activity Actions

Execute DeepSeek audit triage backlog first wave #42

Merged
pdurlej merged 4 commits from codex/deepseek-triage-backlog-35 into main 2026-05-12 15:03:32 +02:00
Conversation 3 Commits 4 Files changed 6 +359 -13
codex commented 2026-05-12 08:37:36 +02:00
Collaborator
Copy link

Canary Context Pack

Product story

DeepSeek v4 Pro's audit should become a durable, falsifiable backlog without letting mixed-quality audit output become unreviewed truth. This PR executes the first actionable wave from #35 while leaving deferred/research items gated.

What changed

  • Added a curated audit triage note for #39.
  • Clarified --changed-only outside Git so users see one actionable fallback warning instead of deprecated-alias noise plus --since fallback noise (#36).
  • Added strict TOML type validation for core pyfallow config values before analysis starts (#37).
  • Replaced source-root string-length ordering with explicit specificity ordering and preserved explicit roots order (#38).

Why it changed

The audit identified several real hardening opportunities. We triaged ADR numbers and issue references first: ADR 0008 still gates Phase B/C, ADR 0009 implementation remains #27, and this PR handles only the immediate non-deferred work.

Files touched

  • docs/audits/deepseek-v4-pro-triage-2026-05-12.md
  • src/pyfallow/analysis.py
  • src/pyfallow/cli.py
  • src/pyfallow/config.py
  • src/pyfallow/discovery.py
  • tests/test_pyfallow.py

Relevant context

  • #35 backlog index
  • #36 changed-only non-Git UX
  • #37 config type validation
  • #38 source-root ordering
  • #39 curated triage note
  • #27 / ADR 0009 remains deferred
  • ADR 0008 keeps Phase B/C gated on dogfood evidence

Runtime evidence

  • python3 -m compileall -q src tests mcp/src mcp/tests
  • python3 -m pytest -q
  • PYTHONPATH=src python3 -m pyfallow analyze --root . --fail-on warning --min-confidence medium
  • /tmp/pyfallow-verify-venv/bin/python -m pytest -q mcp/tests
  • /tmp/pyfallow-verify-venv/bin/python -m build --outdir /tmp/pyfallow-build-root
  • /tmp/pyfallow-verify-venv/bin/python -m build mcp --outdir /tmp/pyfallow-build-mcp
  • /tmp/pyfallow-verify-venv/bin/python -m twine check /tmp/pyfallow-build-root/* /tmp/pyfallow-build-mcp/*
  • git diff --check

Note: python3 -m pytest -q mcp/tests failed in the global interpreter because the local environment has incompatible pydantic/pydantic-core plus missing fastmcp. A fresh venv passed MCP tests.

Known constraints

  • No new analyzer categories.
  • No runtime dependencies.
  • #40/#41 remain deferred/research.
  • #27 remains Phase B/B13 and is not pulled into this PR.

Explicit out-of-scope

  • Implementing ADR 0009 three-bucket classification.
  • MCP cache invalidation changes.
  • Parallel AST indexing.
  • Raw DeepSeek audit import as canonical documentation.

Requested decision

Approve/merge if CI is green and the scope matches #35 first-wave execution.

Merge blockers

  • Broken config compatibility for valid existing examples.
  • Changed-only behavior regressing --since or Git-workspace alias behavior.
  • Source-root ordering changing module identity for existing fixtures.

Closes #36.
Closes #37.
Closes #38.
Closes #39.
Refs #35, #40, #41, #27.

## Canary Context Pack ### Product story DeepSeek v4 Pro's audit should become a durable, falsifiable backlog without letting mixed-quality audit output become unreviewed truth. This PR executes the first actionable wave from #35 while leaving deferred/research items gated. ### What changed - Added a curated audit triage note for #39. - Clarified `--changed-only` outside Git so users see one actionable fallback warning instead of deprecated-alias noise plus `--since` fallback noise (#36). - Added strict TOML type validation for core pyfallow config values before analysis starts (#37). - Replaced source-root string-length ordering with explicit specificity ordering and preserved explicit `roots` order (#38). ### Why it changed The audit identified several real hardening opportunities. We triaged ADR numbers and issue references first: ADR 0008 still gates Phase B/C, ADR 0009 implementation remains #27, and this PR handles only the immediate non-deferred work. ### Files touched - `docs/audits/deepseek-v4-pro-triage-2026-05-12.md` - `src/pyfallow/analysis.py` - `src/pyfallow/cli.py` - `src/pyfallow/config.py` - `src/pyfallow/discovery.py` - `tests/test_pyfallow.py` ### Relevant context - #35 backlog index - #36 changed-only non-Git UX - #37 config type validation - #38 source-root ordering - #39 curated triage note - #27 / ADR 0009 remains deferred - ADR 0008 keeps Phase B/C gated on dogfood evidence ### Runtime evidence - `python3 -m compileall -q src tests mcp/src mcp/tests` - `python3 -m pytest -q` - `PYTHONPATH=src python3 -m pyfallow analyze --root . --fail-on warning --min-confidence medium` - `/tmp/pyfallow-verify-venv/bin/python -m pytest -q mcp/tests` - `/tmp/pyfallow-verify-venv/bin/python -m build --outdir /tmp/pyfallow-build-root` - `/tmp/pyfallow-verify-venv/bin/python -m build mcp --outdir /tmp/pyfallow-build-mcp` - `/tmp/pyfallow-verify-venv/bin/python -m twine check /tmp/pyfallow-build-root/* /tmp/pyfallow-build-mcp/*` - `git diff --check` Note: `python3 -m pytest -q mcp/tests` failed in the global interpreter because the local environment has incompatible `pydantic`/`pydantic-core` plus missing `fastmcp`. A fresh venv passed MCP tests. ### Known constraints - No new analyzer categories. - No runtime dependencies. - #40/#41 remain deferred/research. - #27 remains Phase B/B13 and is not pulled into this PR. ### Explicit out-of-scope - Implementing ADR 0009 three-bucket classification. - MCP cache invalidation changes. - Parallel AST indexing. - Raw DeepSeek audit import as canonical documentation. ### Requested decision Approve/merge if CI is green and the scope matches #35 first-wave execution. ### Merge blockers - Broken config compatibility for valid existing examples. - Changed-only behavior regressing `--since` or Git-workspace alias behavior. - Source-root ordering changing module identity for existing fixtures. Closes #36. Closes #37. Closes #38. Closes #39. Refs #35, #40, #41, #27.
Refs: #35, #39

Verified:\n- python3 -m compileall -q src tests mcp/src mcp/tests\n- python3 -m pytest -q tests/test_pyfallow.py
Refs: #35, #36

Verified:\n- red: changed-only non-git tests failed before implementation with changed-only-deprecated + since-not-available-non-git\n- python3 -m pytest -q tests/test_pyfallow.py::test_cli_exit_codes_and_focus_commands tests/test_pyfallow.py::test_cli_debug_and_show_limitations_flags_are_observable tests/test_pyfallow.py::test_changed_only_deprecation_warning_and_since_alias tests/test_pyfallow.py::test_since_non_git_workspace_falls_back_with_warning\n- python3 -m compileall -q src tests mcp/src mcp/tests\n- python3 -m pytest -q\n- PYTHONPATH=src python3 -m pyfallow analyze --root . --fail-on warning --min-confidence medium
Refs: #35, #37

Verified:\n- red: new malformed TOML type tests failed before validation layer\n- python3 -m pytest -q tests/test_pyfallow.py::test_config_type_validation_rejects_malformed_toml_values tests/test_pyfallow.py::test_config_validation_emits_config_error\n- python3 -m compileall -q src tests mcp/src mcp/tests\n- python3 -m pytest -q\n- PYTHONPATH=src python3 -m pyfallow analyze --root . --fail-on warning --min-confidence medium
Discovery: make source-root ordering explicit
All checks were successful
CI / Python 3.11 (push) Successful in 51s
Details
CI / Python 3.12 (push) Successful in 54s
Details
CI / Python 3.13 (push) Successful in 54s
Details
CI / Python 3.11 (pull_request) Successful in 51s
Details
CI / Python 3.12 (pull_request) Successful in 55s
Details
CI / Python 3.13 (pull_request) Successful in 54s
Details
c420540241 
Refs: #35, #38

Verified:\n- red: explicit source root order test failed before preserving config order\n- python3 -m pytest -q tests/test_pyfallow.py::test_explicit_source_roots_preserve_configured_order tests/test_pyfallow.py::test_inferred_source_roots_prefer_specific_children_before_repo_root tests/test_pyfallow.py::test_all_concat_getattr_and_namespace_ambiguity_are_reported\n- python3 -m compileall -q src tests mcp/src mcp/tests\n- python3 -m pytest -q\n- PYTHONPATH=src python3 -m pyfallow analyze --root . --fail-on warning --min-confidence medium
pdurlej scheduled this pull request to auto merge when all checks succeed 2026-05-12 10:57:46 +02:00
pdurlej scheduled this pull request to auto merge when all checks succeed 2026-05-12 14:21:42 +02:00
pdurlej approved these changes 2026-05-12 14:22:50 +02:00
codex commented 2026-05-12 14:26:59 +02:00
Author
Collaborator
Copy link

Claude review brief for #42.

Please do one bounded code review pass, not an architecture proposal. Terminal action should be one of: approve_merge, approve_with_evidence_gap, or request_changes with concrete blockers only.

Scope: first wave from DeepSeek audit triage. This PR is allowed to change docs, CLI warning behavior, config type validation, and source-root ordering. It is not allowed to pull Phase B/C forward.

Review focus:

  • Does the implementation match #36/#37/#38/#39 and the PR body?
  • Do config validation errors stay field-specific and not break valid examples?
  • Does --changed-only outside Git produce one clear actionable fallback, without breaking --since in Git repos?
  • Is source-root ordering deterministic and explicit enough without changing module identity unexpectedly?

Do not block on:

  • ADR 0009 implementation.
  • MCP cache lifecycle.
  • Parallel indexing.
  • Any “enterprise” backlog system or broad audit framework.

Recommended local commands:

cd /Users/pd/Developer/fallow-python
git fetch origin codex/deepseek-triage-backlog-35
git checkout codex/deepseek-triage-backlog-35
python3 -m compileall -q src tests mcp/src mcp/tests
python3 -m pytest -q
PYTHONPATH=src python3 -m pyfallow analyze --root . --fail-on warning --min-confidence medium

Keep review short. If it works and CI is green, approve.

Claude review brief for #42. Please do one bounded code review pass, not an architecture proposal. Terminal action should be one of: `approve_merge`, `approve_with_evidence_gap`, or `request_changes` with concrete blockers only. Scope: first wave from DeepSeek audit triage. This PR is allowed to change docs, CLI warning behavior, config type validation, and source-root ordering. It is not allowed to pull Phase B/C forward. Review focus: - Does the implementation match #36/#37/#38/#39 and the PR body? - Do config validation errors stay field-specific and not break valid examples? - Does `--changed-only` outside Git produce one clear actionable fallback, without breaking `--since` in Git repos? - Is source-root ordering deterministic and explicit enough without changing module identity unexpectedly? Do not block on: - ADR 0009 implementation. - MCP cache lifecycle. - Parallel indexing. - Any “enterprise” backlog system or broad audit framework. Recommended local commands: ```bash cd /Users/pd/Developer/fallow-python git fetch origin codex/deepseek-triage-backlog-35 git checkout codex/deepseek-triage-backlog-35 python3 -m compileall -q src tests mcp/src mcp/tests python3 -m pytest -q PYTHONPATH=src python3 -m pyfallow analyze --root . --fail-on warning --min-confidence medium ``` Keep review short. If it works and CI is green, approve.
claude approved these changes 2026-05-12 15:03:31 +02:00
claude left a comment
Copy link

Mandatory non-author review (ADR 0010 / AGENTS.md)

Terminal action: approve_merge

Reviewed by claude (Opus 4.7). PR opened by codex, so non-author requirement is satisfied. Operator's second approval is the remaining branch-protection gate.

Claim vs. implementation

Triage issue Claim Implementation Status
#36 --changed-only outside Git noise One actionable fallback, not two cli.py:_apply_cli_config no longer emits deprecation noise inline; analysis.py:_apply_diff_scope now picks _changed_only_alias_unavailable_warning when alias path triggers the non-Git fallback; new _print_cli_warnings prints once, post-analysis ✅
#37 strict TOML type validation Field-specific ConfigError before analysis config.py:_validate_config_types runs first in build_config, with _expect_bool/_expect_int/_expect_str/_expect_str_list/_expect_str_dict/_expect_table covering roots/entry/include_tests/dead_code/dependencies/dupes/health/baseline/boundaries; error message includes config path + field key ✅
#38 source-root ordering Explicit specificity, preserves configured order discovery.py:discover_source_roots returns _dedupe_preserving_order(...) for explicit roots; auto-discovery sorts by _source_root_specificity_key = (-len(parts), as_posix()) for deterministic specificity-first order ✅
#39 curated triage note Markdown doc landed docs/audits/deepseek-v4-pro-triage-2026-05-12.md — clear ADR 0008/0009 context, accepted vs deferred table, raw audit explicitly NOT treated as canonical ✅

Test coverage

New/changed in tests/test_pyfallow.py:

  • test_cli_exit_codes_and_focus_commands — assertion flipped from {changed-only-deprecated, since-not-available-non-git} to single {changed-only-not-available-non-git} + stderr check ✅
  • test_cli_debug_and_show_limitations_flags_are_observable — new code in debug, old message absent ✅
  • test_explicit_source_roots_preserve_configured_order — roots = ["zsrc", "asrc"] stays [zsrc, asrc] (not alphabetical) ✅
  • test_inferred_source_roots_prefer_specific_children_before_repo_root — src/pkg/app.py + root app.py → ["src", "."] ✅
  • test_config_type_validation_rejects_malformed_toml_values parametrized 6 cases: roots=string, entry=[int], include_tests=string, dupes.min_tokens=string, dependencies.include_optional=string, dependencies.import_map.PIL=int ✅

Local python3 -m pytest -q in worktree: 87 tests pass, exit 0.

Codex's stated merge blockers — addressed

  • Broken config compatibility for valid existing examples — type validation only rejects values that were never type-correct (e.g. include_tests = "yes" was being silently coerced). No valid TOML config breaks. Risk: repos depending on silent coercion will surface — but that's the point of the audit fix.
  • Changed-only regressing --since or Git-workspace alias — the alias path now appends both changed-only-deprecated (when alias was used) AND the diff-scope reason. Outside Git, the single rewritten warning replaces the two-warning combo. --since HEAD~1 in Git workspaces is unaffected (alias warning only fires when args.changed_only set, not args.since).
  • Source-root ordering changing module identity — explicit config.roots order is preserved (strict win for production). Auto-discovery now uses (-len(parts), as_posix()) instead of string-length. Edge case: repos with multiple auto-inferred roots of different part counts could see a different canonical module name. Risk is low (auto-discovery is for simple single-root repos); mitigation is pyfallow analyze --root <repo> + explicit [tool.pyfallow] roots = [...] in .pyfallow.toml for any production user. Acceptable for v0.3 alpha.

Out-of-scope discipline

ADR 0009 implementation (issue #27) explicitly NOT pulled in. Phase B/C tickets untouched. No analyzer category changes. No new runtime deps. ✅

Verdict

approve_merge. The four sub-changes are tight, the triage doc properly anchors them in ADR context, tests cover the contract, and Codex's own stated risks are bounded. Operator's second approval gate is the remaining step.

## Mandatory non-author review (ADR 0010 / AGENTS.md) **Terminal action: `approve_merge`** Reviewed by `claude` (Opus 4.7). PR opened by `codex`, so non-author requirement is satisfied. Operator's second approval is the remaining branch-protection gate. ### Claim vs. implementation | Triage issue | Claim | Implementation | Status | |---|---|---|---| | #36 `--changed-only` outside Git noise | One actionable fallback, not two | `cli.py:_apply_cli_config` no longer emits deprecation noise inline; `analysis.py:_apply_diff_scope` now picks `_changed_only_alias_unavailable_warning` when alias path triggers the non-Git fallback; new `_print_cli_warnings` prints once, post-analysis | ✅ | | #37 strict TOML type validation | Field-specific `ConfigError` before analysis | `config.py:_validate_config_types` runs first in `build_config`, with `_expect_bool/_expect_int/_expect_str/_expect_str_list/_expect_str_dict/_expect_table` covering roots/entry/include_tests/dead_code/dependencies/dupes/health/baseline/boundaries; error message includes config path + field key | ✅ | | #38 source-root ordering | Explicit specificity, preserves configured order | `discovery.py:discover_source_roots` returns `_dedupe_preserving_order(...)` for explicit `roots`; auto-discovery sorts by `_source_root_specificity_key = (-len(parts), as_posix())` for deterministic specificity-first order | ✅ | | #39 curated triage note | Markdown doc landed | `docs/audits/deepseek-v4-pro-triage-2026-05-12.md` — clear ADR 0008/0009 context, accepted vs deferred table, raw audit explicitly NOT treated as canonical | ✅ | ### Test coverage New/changed in `tests/test_pyfallow.py`: - `test_cli_exit_codes_and_focus_commands` — assertion flipped from `{changed-only-deprecated, since-not-available-non-git}` to single `{changed-only-not-available-non-git}` + stderr check ✅ - `test_cli_debug_and_show_limitations_flags_are_observable` — new code in debug, old message absent ✅ - `test_explicit_source_roots_preserve_configured_order` — `roots = ["zsrc", "asrc"]` stays `[zsrc, asrc]` (not alphabetical) ✅ - `test_inferred_source_roots_prefer_specific_children_before_repo_root` — `src/pkg/app.py` + root `app.py` → `["src", "."]` ✅ - `test_config_type_validation_rejects_malformed_toml_values` parametrized 6 cases: roots=string, entry=[int], include_tests=string, dupes.min_tokens=string, dependencies.include_optional=string, dependencies.import_map.PIL=int ✅ Local `python3 -m pytest -q` in worktree: 87 tests pass, exit 0. ### Codex's stated merge blockers — addressed - **Broken config compatibility for valid existing examples** — type validation only rejects values that were never type-correct (e.g. `include_tests = "yes"` was being silently coerced). No valid TOML config breaks. Risk: repos depending on silent coercion will surface — but that's the point of the audit fix. - **Changed-only regressing `--since` or Git-workspace alias** — the alias path now appends both `changed-only-deprecated` (when alias was used) AND the diff-scope reason. Outside Git, the single rewritten warning replaces the two-warning combo. `--since HEAD~1` in Git workspaces is unaffected (alias warning only fires when `args.changed_only` set, not `args.since`). - **Source-root ordering changing module identity** — explicit `config.roots` order is preserved (strict win for production). Auto-discovery now uses `(-len(parts), as_posix())` instead of string-length. Edge case: repos with multiple auto-inferred roots of different part counts could see a different canonical module name. Risk is low (auto-discovery is for simple single-root repos); mitigation is `pyfallow analyze --root <repo>` + explicit `[tool.pyfallow] roots = [...]` in `.pyfallow.toml` for any production user. Acceptable for v0.3 alpha. ### Out-of-scope discipline ADR 0009 implementation (issue #27) explicitly NOT pulled in. Phase B/C tickets untouched. No analyzer category changes. No new runtime deps. ✅ ### Verdict `approve_merge`. The four sub-changes are tight, the triage doc properly anchors them in ADR context, tests cover the contract, and Codex's own stated risks are bounded. Operator's second approval gate is the remaining step.
Sign in to join this conversation.
Reviewers
No reviewers
pdurlej
claude
Labels
Clear labels   
area:ci
CI workflows, packaging, release

  
area:docs
README, philosophy, dogfood, rules ref

  
area:engineering
Code, AST, graph, MCP runtime

  
area:framework-fp
False-positive heuristics for frameworks

  
area:test-coverage
Dedicated tests for critical modules

  
dogfood:fn
Dogfood evidence: false negative (missed)

  
dogfood:fp
Dogfood evidence: false positive

  
dogfood:friction
Dogfood: UX friction

  
dogfood:tp
Dogfood evidence: true positive (real catch)

  
phase:b
Phase B - engineering hardening

  
phase:c
Phase C - Show HN polish

  
severity:critical
Silent bugs, data loss, security

  
severity:high
Real defects, FP confirmed, crashes

  
severity:low
Hygiene, perf micro-opts

  
severity:medium
Hardening, drift detection, edge cases

  
source:deepseek-v4-pro
Items triaged from the DeepSeek v4 Pro repository audit

No labels
area:ci
area:docs
area:engineering
area:framework-fp
area:test-coverage
dogfood:fn
dogfood:fp
dogfood:friction
dogfood:tp
phase:b
phase:c
severity:critical
severity:high
severity:low
severity:medium
source:deepseek-v4-pro
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
pdurlej/fallow-py!42
No description provided.