diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 5601ace..81d2a90 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -77,7 +77,7 @@ testing · extension points. - **[rank-002-handoff.md](rank-002-handoff.md)** — RANK-002 implemented build brief for the `backend/scoring/` scorer (pure components + config + the `run_scan` call + UI sort/components + tests). - **[auth-003-role-model.md](auth-003-role-model.md)** — AUTH-003 role model: hierarchical viewer/analyst/admin, the capability→min-role map, the database-driven `user_roles` store with an `ADMIN_EMAILS` bootstrap floor, resolution precedence, defense-in-depth enforcement, and denial logging/audit. - **[auth-003-handoff.md](auth-003-handoff.md)** — AUTH-003 build brief for the `backend/auth/roles.py` policy + `user_roles` table/migration + repository + `require_capability` enforcement + the admin Roles page + tests. -- **[audit-2026-06.md](audit-2026-06.md)** — June 2026 codebase audit & hardening register (QUAL-001/002/003, REF-001, PERF-001, DOC-001): what was found, fixed, and deferred. +- **[audit-2026-06.md](audit-2026-06.md)** — June–July 2026 codebase audit and hardening register through PR #107: what was found, fixed, rejected, and deferred across both review waves. ## Conventions diff --git a/docs/architecture/audit-2026-06.md b/docs/architecture/audit-2026-06.md index c3192bd..287e44e 100644 --- a/docs/architecture/audit-2026-06.md +++ b/docs/architecture/audit-2026-06.md @@ -198,3 +198,80 @@ removal of one redundant flush, and the stronger regression guard. | Fundamentals eligibility tooltip UX | Users can't tell why "Check Fundamentals" mode differs by symbol; small UI improvement, needs product wording. | | `_parse_company_page(session=None)` + a page with an HTMX peers URL would crash in the unmocked path | Production-unreachable (`fetch_company_data` always builds a real Session); the test seam relies on mocking `_fetch_html`. Documented at the call site; tidy when the scraper next changes. | | Postgres-backed deployment guide with a worked example | docs/operations.md covers the switch; a full worked deploy (host, service file, reverse proxy) is its own doc when a real deployment exists. | + +## July 2026 — whole-app review, two waves (appended 2026-07-11) + +Two independent review passes ran this month and their outputs were reconciled +against main @ `0c87b65` before the second wave started, so nothing below was +built twice. **Wave 1** (another Claude session, PRs +[#88](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/88)–[#97](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/97)) +and **wave 2** (this session, PRs +[#98](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/98)–[#107](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/107), +one ticket per PR) together close most of the June deferral list; the status of +every June item is updated in the table at the end of this section. + +### Wave 1 (PRs #88–#97, other session) + +DOC-002 AGENTS.md IPO map (#88) · TEST-004 golden coverage for the five +untested deterministic screeners (#89) · UI-001 chart-cache +`screener_version` key, SEC-001 role-email shape check, QUAL-005 coverage +floor raised to 87 (#90–#92) · AI-005 shared sync bridge, landed as +REFACTOR-003 with ADR (#93) · TEST-005 screener.in HTML snapshot tests (#94) +· UI-002 fundamentals freshness caption + eligibility captions (#95) · +QUAL-004 mypy on tests/ phase 1 (#96) · REF-002 app.py extraction of +`ui/scan_view.py` + `ui/fundamentals_panel.py` (#97). + +### Wave 2 (PRs #98–#107, this session) + +| PR | Ticket | What landed | +|---|---|---| +| [#98](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/98) | TEST-007 | Render-path tests for the #97 extractions; locks the AUTH-003 export gate, OBS-003 export audit, symbol-stable chart selection after row reordering, secret-safe cached/agent failures, and force-refresh reruns. | +| [#99](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/99) | REF-003 | app.py 1,127→822 lines: status panel + parameter controls → `ui/`, 100% tested; `cache_summary` default no longer bound at import time. | +| [#100](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/100) | AI-006 | One shared verdict-JSON extractor in `backend/ai_runtime.py`; rejects non-finite constants and parser failures before validation/cache signing, with `allow_inf_nan=False` as model-level defense in depth. | +| [#101](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/101) | IPO-006 | One `canonical_sebi_url` behind both SSRF gates; rejects encoded traversal/separators, preserves PDF-path policy across redirects, resolves wrappers from the final URL, and normalizes malformed URL/stream failures into safe domain errors. | +| [#102](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/102) | TEST-006 | IPO pipeline end-to-end scenario (ingest→download→extract→ratios→verdict) with a provenance-digest chain across every stage hand-off. | +| [#103](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/103) | PERF-002 | Parquet footer-statistics cache decisions (see the sidecar supersession note below); benchmark-gated at ~12×. | +| [#104](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/104) | DEPLOY-004 | Password-safe env-file/prompt Postgres workflow, URL-encoding guidance, SQLite→Postgres migration recipe, Alembic encoded-password safety, and the correct `audit_logs` table; `pool_pre_ping` remains server-only. | +| [#105](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/105) | QUAL-006 | pandas-stubs adopted (see the estimate correction below). The wave's one sanctioned constraints/pyproject change. | +| [#106](https://github.com/DoRmAmMu1997/Streamlit-Scanner-App/pull/106) | QUAL-007 | mypy checks the whole test tree by default; 29 modules of pre-typing debt are enumerated in a mechanically enforced shrink-only override. | +| (this PR) | DOC-003 | This register section. | + +### Findings verified FALSE in this review (do not re-flag) + +1. **"`cpr_yearly` computes with floats, violating the Decimal-for-money + rule."** False as a violation: float math is the framework-wide convention + for *indicator* calculations across every screener and `backend/indicators.py`; + the Decimal rule governs money at persistence/contract boundaries. Changing + one screener would create inconsistency, not correctness. +2. **"`cpr_yearly` is untested."** False — four dedicated tests plus a golden + snapshot existed at review time. +3. **"The technical agent lacks a prompt-injection quarantine."** Not a gap: + its structured-candle-only posture (no untrusted-text tool) is the locked + TEST-003 decision above, with regression tests pinning it. + +### Considered and REJECTED in wave 1 (recorded so they are not re-proposed) + +- **Session-state registry abstraction** for the UI — indirection without a + demonstrated bug class. +- **Indicators decorator dedup** — the repetition is shallow; a decorator + would obscure the per-indicator math it wraps. +- **`sectors` iterrows micro-optimization** — not a measured hotspot. +- **DNS-rebinding hardening for the fixed SEBI listing URLs** — the listing + URLs are hardcoded constants, and the byte-fetching surface (the PDF + downloader) already resolves the host and rejects non-public answers. +- **`run_scan` length refactor** — long but linear and heavily commented; + splitting it would scatter one coherent lifecycle. + +### June deferral list — status after July + +| June item | July status | +|---|---| +| Agent SDK boilerplate dedup | **Landed in two steps**: the sync bridge (REFACTOR-003, #93) and the verdict-JSON extractor (AI-006, #100). Options-construction/retry/error-taxonomy blocks stay per-agent **by ADR decision** — that is the recorded end state, not remaining debt. | +| mypy on tests/ | **Landed in two phases**: whitelist (#96), then checked-by-default with a 29-module shrink-only debt list (#106). | +| pandas-stubs | **Landed** (#105). Correction for future estimators: June predicted "hundreds of errors"; with stubs version-matched to the pinned pandas minor (2.3.3), the real count was **22**, all mechanical. | +| Parquet metadata sidecar | **Superseded, honoring the June objection.** PERF-002 (#103) needs **no second cache format**: Parquet footers already carry row-group min/max statistics (health.py had read them since OBS-002). Benchmark-gated per the objection (~12× on the bounds question). Review hardening on the PR: footer bounds are treated as an advisory index — the prefetch still validates the actual frame before a "fresh" verdict, because a valid footer can coexist with corrupt data pages. | +| screener.in HTML snapshots | **Landed** (#94). | +| Fundamentals eligibility tooltip | **Landed** (#95). | +| Postgres worked deployment guide | **Landed** (#104), including the SQLite→Postgres data-migration recipe and pool guidance. | +| Redaction single-pass regex | Still deferred — remains a micro-optimization. | +| `_parse_company_page(session=None)` crash path | Still deferred — production-unreachable; tidy when the scraper next changes. |