From 4787d39d655422dc80d889487983a1553199dadc Mon Sep 17 00:00:00 2001 From: Travis Shepherd <134653832+shepherd70@users.noreply.github.com> Date: Mon, 22 Jun 2026 11:08:19 -0600 Subject: [PATCH 1/2] Add repo audit report (2026-06) --- docs/pathways-audit-2026-06.md | 41 ++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 docs/pathways-audit-2026-06.md diff --git a/docs/pathways-audit-2026-06.md b/docs/pathways-audit-2026-06.md new file mode 100644 index 0000000..44f853e --- /dev/null +++ b/docs/pathways-audit-2026-06.md @@ -0,0 +1,41 @@ +# pathways — Audit (2026-06) + +Dimensions run: **core** (correctness; security & provenance; maintainability & dependencies) plus conditionals **Python conventions** and **regulatory defensibility** — the tool builds and reports DFO (Fisheries and Oceans Canada) Pathways-of-Effects graphs for fish-habitat assessments, so silent omission of an effect pathway is a defensibility problem. Statistical-validity and financial-data dimensions do not apply (no estimators, no money). Security scan: `pi_audit.py --root` reported **0 findings**; git provenance is clean (single author `shepherd70@gmail.com`; the one `noreply@github.com` committer is the normal GitHub-web initial commit; no unexpected `Co-authored-by` trailers). + +**Summary.** The code is readable and the `poe.py` version is reasonably structured, but the repository is mid-migration and currently holds **three-to-four divergent implementations/data models of the same tool with no single source of truth**, and two of its core features are silently broken. The graph in `stressors.json` is built purely by string-matching node names, and **seven misspelled child nodes become dead-ends that truncate effect pathways before they reach the impairment endpoint** — wrong output a user cannot see. The "Mitigation Measures" feature **never finds a match** and always prints "No mitigation information available." A newer Excel data model (`pathways.xlsx`) and a risk-scoring report (`output/2_report.txt`) reference logic and data that exist in **no committed code**. None of this is a security issue; it is correctness, reproducibility, and source-of-truth hygiene. Nothing here was changed — these are findings only. + +## Critical + +- `stressors.json` (graph as built in `poe.py:80-93`) — **Five misspelled child node names silently truncate regulatory effect pathways.** Because edges are keyed on free-text node names, any child whose spelling does not exactly match a defined key becomes an undefined terminal node, so the pathway dead-ends instead of continuing to the canonical endpoint *"Potential direct or indirect impairment of the habitat capacity to support one or more life processes."* Confirmed dead-ends and their intended targets: + - `:213,217` `"Change in channel morphology or shoreline morphology"` → should be `…shoreline morphometry` (defined at `:160`). + - `:172` `"Change of loss of wetted area"` (typo "of") → `"Change or loss of wetted area"` (defined at `:38`). + - `:77` `"Deposit of deleterious substances/ sediment"` (stray space after `/`) → `"Deposit of deleterious substances/sediment"` (defined at `:65`). + - `:96` `"Decreased water quality"` → `"Decrease in water quality"` (defined at `:73`). + - `:76,112` `"Decrease in food supply"` → almost certainly `"Altered food supply"` (defined at `:29`, which routes to the impairment endpoint). + + Two further dead-ends are lower impact: `"Sublethal effect and/or mortality"` (`:177,189`, singular) maps to the already-terminal `"Sublethal effects and/or mortality"` (`:138`); `"Entrainment of fish"` (`:182`) appears genuinely undefined (distinct from `"Entrainment/impingement"`). Fix: correct the spellings so every child matches a defined key, and add a build-time validation step in `PoEGraph.build_graph` that errors (or warns loudly) when a referenced child is never defined as a key — that single guard would have caught all seven. Longer term, key edges on stable IDs (as `pathways.xlsx` already does) so spelling never affects connectivity. + +## Major + +- `poe.py:57-70` + `poe.py:208-216` — **The "Mitigation Measures" feature can never match a file.** `identify_mitigation()` looks up mitigation for each entry in `self.active_stressors`, which are the `PRIMARY_ACTIVITIES` (e.g. `"Use of Machinery on Land / Alteration of Riparian Vegetation"`). `get_text()` sanitizes only `/`→`_`, producing `"Use of Machinery on Land _ Alteration…"`. But the files in `mitigation_texts/` are named for *effect* nodes using a different convention — spaces→underscores, with inconsistent casing (mostly lowercase, e.g. `decrease_in_shade.txt`, `change_or_loss_of_fish_passage.txt`, but also the capitalized `Increased_input_of_woody_material.txt`). The keys therefore never match the filenames, so every selected activity prints `"No mitigation information available…"`. Two distinct defects: (a) the sanitizer does not lowercase or replace spaces, so it cannot reproduce the file-naming convention; (b) the files are keyed to effects while the code only ever queries primary activities. Fix: decide the intended design — most likely mitigation should be shown for the **effect nodes discovered along the active pathways**, not the primary activities — then normalize keys and filenames through one shared function (`name.lower().replace(' ', '_')…`). + +- `mitigation_texts/test.py` — **A committed, broken, misfiled second implementation.** This 121-line earlier prototype expects the schema `{stressor: {"endpoints": [...]}}` (`test.py:28,51,58`), but the actual `stressors.json` maps each key to a **bare list**. Running it raises `TypeError: list indices must be integers… not 'str'` in `build_graph` on first load — it cannot work against the current data. It is also misplaced *inside the data directory* `mitigation_texts/` (harmless to the loader, which filters `.txt`, but confusing for provenance). Fix: delete it, or if it has reference value move it to `archive/` and mark it non-runnable. Do not leave a broken entry point in the data folder. + +- Repo-wide — **No single source of truth: 3–4 parallel models of the same tool.** (1) `poe.py` reads `stressors.json` as a flat `{node: [children]}` graph; (2) `mitigation_texts/test.py` reads it as nested `{node: {"endpoints": […]}}` (incompatible, above); (3) `pathways.xlsx` reimplements the graph properly with `Nodes_Data` + `Edges_Data` keyed on stable `Node_ID`s (`N000…`, `E001…`) plus `Node_Type`, `Risk_Score`, coordinates and an interactive `Dashboard`; (4) `output/2_report.txt` implies a fourth, risk-scoring version (below). The Excel migration is **half-done**: the cleaner ID-based data model is committed but **no code reads it** (neither script imports `openpyxl`/`pandas` or references `.xlsx`). Fix: pick the target (the ID-based Excel model resolves the Critical string-matching fragility and is the natural choice), make one implementation authoritative, and retire the others. Until then it is unclear which artifact is current. + +- `output/2_report.txt` — **Orphaned, non-reproducible output.** This report contains Project metadata, per-stressor Magnitude/Duration/Certainty, **numeric Risk Scores** (e.g. `3.4 (MEDIUM RISK)`), and Recommendations — none of which `poe.py` or `test.py` collect or compute, and neither script writes a report file at all. It also cites endpoints absent from `stressors.json` (`"Change or loss of riparian habitat"`, `"Change or loss of natural structure and cover"`). So committed output corresponds to no committed code or data and a reviewer cannot reproduce it. Fix: locate and commit the version of the tool that produced it (it appears to be the most capable variant and may be the real work-in-progress), or remove the stale artifact. Flag for regulatory defensibility: a report no one can regenerate from the repo will not survive scrutiny. + +## Minor + +- No dependency manifest — `networkx` and `matplotlib` (plus `openpyxl` for the Excel path) are undeclared and unpinned; there is no `requirements.txt`/`pyproject.toml` and no README. Reproducibility matters for a tool feeding habitat assessments. Fix: add a pinned manifest and a short README stating which artifact is authoritative and how to run it. (Verified versions in this env: networkx 3.4.2, matplotlib 3.10.0, Python 3.11.9.) +- `poe.py:95-121` `get_paths()` — calls `nx.all_simple_paths` for every (active stressor × every terminal node) pair; since `all_simple_paths` already yields nothing for unreachable targets, the terminal-node loop is unnecessary work, and the `except nx.NetworkXNoPath` (`:116`) is dead — `all_simple_paths` does not raise it. Fix: iterate targets only where needed, or compute reachable descendants once; drop the dead except. +- `poe.py:166` / `test.py:79` `plt.show()` — blocks on an interactive window and offers no save-to-file path, so the tool cannot run headless/automated. Fix: add a `--save ` option using `plt.savefig`. +- `pathways.xlsx` `Nodes_Data` — display-string typos that will surface in regulatory output even though they no longer break connectivity (edges use IDs): `N000` `"Use of Machinery **of** Land"` (should be "on"), `N005` `"deli**te**rious"` (deleterious), `N008` `"Altered external nutrients / energy input"` differs from `stressors.json`'s `"…energy inputs"` (spaces around the slash **and** singular vs plural). Fix: proofread node names against the DFO PoE source wording. +- `pathways.xlsx` is a scaffold/WIP — `Instructions` and `Analysis_Helpers` sheets are empty; `Lists_Data` holds placeholder *descriptions* of what the Instructions sheet should contain (overview, how-to, node-type definitions, support contact). Fine as in-progress, but note it is not a finished deliverable. +- Binary `pathways.xlsx` (182 KB) committed to git — acceptable as the data source, but diffs are opaque; consider also exporting `Nodes_Data`/`Edges_Data` to CSV for reviewable history. + +## Could not verify + +- **Intent of `pathways.xlsx`**: whether it is meant to *replace* the Python CLI as a standalone interactive workbook (it has a `Dashboard` with node-selection data validation) or to *feed* the Python tool as a data source. This determines what the migration should target. +- **Intended scope of mitigation**: whether mitigation should be shown for primary activities or for the effect nodes along discovered pathways. The file naming points to effect nodes; the code queries primary activities. Confirm the design before fixing the Major mitigation bug. +- **Existence of the risk-scoring tool version** implied by `output/2_report.txt` — it is not in the repo; confirm whether it exists elsewhere (possible lost/uncommitted work). From 568da7fb90a9301e7f3377b171b945ec8863122c Mon Sep 17 00:00:00 2001 From: Travis Shepherd <134653832+shepherd70@users.noreply.github.com> Date: Mon, 3 Aug 2026 23:45:07 +0000 Subject: [PATCH 2/2] chore: update .gitignore Updates .gitignore. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01DxxiZ2pk67ckT1vr7Tq864 --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 0a19790..c213f90 100644 --- a/.gitignore +++ b/.gitignore @@ -172,3 +172,6 @@ cython_debug/ # PyPI configuration file .pypirc + +# Local Claude/agent tool settings (machine-specific) +.claude/