[Audit] ETL hardcodes manager_id but API-created SQLite managers table uses id

[stranske]

Why

The SQLite manager-id convention is inconsistent between the API (which has a dialect resolver) and the ETL flows (which hardcode manager_id), so ETL breaks on any SQLite database bootstrapped by the API.

• api/managers.py:136 creates the SQLite table as managers(id INTEGER PRIMARY KEY, ...), and api/managers.py:33 _manager_id_column(conn) correctly returns "id" for SQLite / "manager_id" for Postgres.
• ETL readers do NOT use that resolver — they hardcode manager_id: etl/news_flow.py:71 (SELECT manager_id, name, aliases FROM managers), etl/daily_diff_flow.py:138, diff_holdings.py:70, etl/activism_flow.py:124.

Failure mode: on a SQLite DB created via the manager API (the documented local/dev path), ETL flows fail with sqlite3.OperationalError: no such column: manager_id. Prod Postgres is unaffected (its column IS manager_id per schema.sql), and CI can miss it when fixtures create a different schema than the app itself creates.

Scope

Make ETL manager-id access dialect-correct so the SQLite-bootstrapped path works end-to-end.

Non-Goals

• Do NOT change the Postgres schema (manager_id stays canonical there).
• Do NOT duplicate the resolver — reuse the existing _manager_id_column logic.

Tasks

• Export/reuse a shared manager-id column resolver and apply it in etl/news_flow.py:71, etl/daily_diff_flow.py:138, diff_holdings.py:70, etl/activism_flow.py:124 (and any other hardcoded manager_id reader of the managers table). Alternatively, standardize the SQLite managers schema on manager_id.

Acceptance Criteria

• New test (currently failing): in a fresh SQLite DB, create a manager via the API path (_ensure_manager_table() + insert), then call an ETL reader (e.g. _fetch_all_manager_ids(conn) / match_entities); current code raises no such column: manager_id, fixed code returns the inserted manager id.
• Deliberate-break demonstration: reverting ETL to hardcoded manager_id fails the new test; restoring the resolver passes.

Implementation Notes

Audit baseline: main @ e5764a5 on 2026-06-28. Source-verified: API SQLite create at managers.py:136, resolver at managers.py:33, hardcoded ETL reads at the listed lines. Prod-Postgres path is unaffected; this is a SQLite/local-path wiring fix.

[stranske-automation-bot]

Opened PR #1304 to engage Codex.

Keepalive mode: OFF

Next steps for the PR author:

• Comment @codex start (auto-posted when possible).
  Ask Codex to draft the plan.
• After Codex replies with the checklist, post the execution command below.
  This begins delivery and enables keepalive.

Execution command (copy into a standalone PR comment):

@codex ## Keepalive Next Task

Your objective is to satisfy the **Acceptance Criteria** by completing each **Task** within the defined **Scope**.

**This round you MUST:**
1. Implement actual code or test changes that advance at least one incomplete task toward acceptance.
2. Commit meaningful source code (.py, .yml, .js, etc.)—not just status/docs updates.
3. Mark a task checkbox complete ONLY after verifying the implementation works.
4. Focus on the FIRST unchecked task unless blocked, then move to the next.

**Guidelines:**
- Keep edits scoped to the current task rather than reshaping the entire PR.
- Use repository instructions, conventions, and tests to validate work.
- Prefer small, reviewable commits; leave clear notes when follow-up is required.
- Do NOT work on unrelated improvements until all PR tasks are complete.

## Pre-Commit Formatting Gate (Black)

Before you commit or push any Python (`.py`) changes, you MUST:
1. Run Black to format the relevant files (line length 100).
2. Verify formatting passes CI by running:
   `black --check --line-length 100 --exclude '(\.workflows-lib|node_modules)' .`
3. If the check fails, do NOT commit/push; format again until it passes.

**COVERAGE TASKS - SPECIAL RULES:**
If a task mentions "coverage" or a percentage target (e.g., "≥95%", "to 95%"), you MUST:
1. After adding tests, run TARGETED coverage verification to avoid timeouts:
   - For a specific script like `scripts/foo.py`, run:
     `pytest tests/scripts/test_foo.py --cov=scripts/foo --cov-report=term-missing -m "not slow"`
   - If no matching test file exists, run:
     `pytest tests/ --cov=scripts/foo --cov-report=term-missing -m "not slow" -x`
2. Find the specific script in the coverage output table
3. Verify the `Cover` column shows the target percentage or higher
4. Only mark the task complete if the actual coverage meets the target
5. If coverage is below target, add more tests until it meets the target

IMPORTANT: Always use `-m "not slow"` to skip slow integration tests that may timeout.
IMPORTANT: Use targeted `--cov=scripts/specific_module` instead of `--cov=scripts` for faster feedback.

A coverage task is NOT complete just because you added tests. It is complete ONLY when the coverage command output confirms the target is met.

**The Tasks and Acceptance Criteria are provided in the appendix below.** Work through them in order.

Issue context (optional: copy into the PR description):

### Source Issue #1302: [Audit] ETL hardcodes manager_id but API-created SQLite managers table uses id
Keepalive: OFF
Base: main
Head: codex/issue-1302

Source: https://github.com/stranske/Manager-Database/issues/1302

> ## Why
> 
> The SQLite manager-id convention is inconsistent between the API (which has a dialect resolver) and the ETL flows (which hardcode `manager_id`), so ETL breaks on any SQLite database bootstrapped by the API.
> 
> - `api/managers.py:136` creates the SQLite table as `managers(id INTEGER PRIMARY KEY, ...)`, and `api/managers.py:33` `_manager_id_column(conn)` correctly returns `"id"` for SQLite / `"manager_id"` for Postgres.
> - ETL readers do NOT use that resolver — they hardcode `manager_id`: `etl/news_flow.py:71` (`SELECT manager_id, name, aliases FROM managers`), `etl/daily_diff_flow.py:138`, `diff_holdings.py:70`, `etl/activism_flow.py:124`.
> 
> **Failure mode:** on a SQLite DB created via the manager API (the documented local/dev path), ETL flows fail with `sqlite3.OperationalError: no such column: manager_id`. Prod Postgres is unaffected (its column IS `manager_id` per `schema.sql`), and CI can miss it when fixtures create a different schema than the app itself creates.
> 
> ## Scope
> 
> Make ETL manager-id access dialect-correct so the SQLite-bootstrapped path works end-to-end.
> 
> ## Non-Goals
> 
> - Do NOT change the Postgres schema (`manager_id` stays canonical there).
> - Do NOT duplicate the resolver — reuse the existing `_manager_id_column` logic.
> 
> ## Tasks
> 
> - [ ] Export/reuse a shared manager-id column resolver and apply it in `etl/news_flow.py:71`, `etl/daily_diff_flow.py:138`, `diff_holdings.py:70`, `etl/activism_flow.py:124` (and any other hardcoded `manager_id` reader of the `managers` table). Alternatively, standardize the SQLite `managers` schema on `manager_id`.
> 
> ## Acceptance Criteria
> 
> - [ ] **New test (currently failing):** in a fresh SQLite DB, create a manager via the API path (`_ensure_manager_table()` + insert), then call an ETL reader (e.g. `_fetch_all_manager_ids(conn)` / `match_entities`); current code raises `no such column: manager_id`, fixed code returns the inserted manager id.
> - [ ] **Deliberate-break demonstration:** reverting ETL to hardcoded `manager_id` fails the new test; restoring the resolver passes.
> 
> ## Implementation Notes
> 
> Audit baseline: `main` @ `e5764a5` on 2026-06-28. Source-verified: API SQLite create at `managers.py:136`, resolver at `managers.py:33`, hardcoded ETL reads at the listed lines. Prod-Postgres path is unaffected; this is a SQLite/local-path wiring fix.
> 

[stranske-keepalive]

✅ Capability Check: Agent can proceed

Recommendation: PROCEED

✅ Actionable Tasks (5):

• Export or reuse a shared manager-id column resolver and apply it in etl/news_flow.py:71, etl/daily_diff_flow.py:138, diff_holdings.py:70, etl/activism_flow.py:124, and any other hardcoded manager_id readers of the managers table.
• Alternatively, standardize the SQLite managers schema on manager_id if the repository already has a clear canonical schema direction and the change can be implemented consistently in code and tests.
• Add a new regression test that creates a fresh SQLite DB, creates a manager through the API path (_ensure_manager_table() + insert), then calls an ETL reader such as _fetch_all_manager_ids(conn) or match_entities, and verifies the inserted manager id is returned instead of raising no such column: manager_id.
• Implement the code fix so the new regression test passes.
• Add or update tests to demonstrate that reverting ETL logic to a hardcoded manager_id lookup would fail the new regression scenario, while the resolver-based implementation passes.

⚠️ Partial Tasks (3):

• Choose between introducing a shared resolver versus standardizing the SQLite managers schema on manager_id.
  • Limitation: An agent can implement either path, but if repository history, maintainers, or architectural expectations do not clearly indicate the intended canonical schema, this becomes a design choice that may require human confirmation.
• Update all other hardcoded manager_id readers of the managers table.
  • Limitation: An agent can search and update references in the repository, but cannot guarantee discovery of implicit/runtime-generated usages outside the checked-in codebase.
• Provide a deliberate-break demonstration by reverting ETL to hardcoded manager_id and showing the new test fails, then restoring the resolver and showing it passes.
  • Limitation: An agent can add a regression test that would fail under the old implementation and pass under the fixed implementation, but cannot literally maintain a reverted and restored code state simultaneously in the final branch; the demonstration is typically evidenced by the test's sensitivity to the implementation.

👤 Human Actions Needed:

• Confirm the preferred long-term approach if the codebase does not already establish one: shared manager-id resolver vs. schema standardization on manager_id.
• Review whether any external or downstream consumers depend on the current SQLite managers schema before choosing schema standardization.

---

Auto-generated by capability check

[stranske]

Merged PR #1364 and applied verify:compare. Reopening this source issue for the required post-merge verifier sequence; it should stay open until durable verifier PASS or an evidence-backed disposition is recorded.

[stranske-automation-bot]

Opened PR #1367 to engage Codex.

Keepalive mode: OFF

Next steps for the PR author:

• Comment @codex start (auto-posted when possible).
  Ask Codex to draft the plan.
• After Codex replies with the checklist, post the execution command below.
  This begins delivery and enables keepalive.

Execution command (copy into a standalone PR comment):

@codex ## Keepalive Next Task

Your objective is to satisfy the **Acceptance Criteria** by completing each **Task** within the defined **Scope**.

**This round you MUST:**
1. Implement actual code or test changes that advance at least one incomplete task toward acceptance.
2. Commit meaningful source code (.py, .yml, .js, etc.)—not just status/docs updates.
3. Mark a task checkbox complete ONLY after verifying the implementation works.
4. Focus on the FIRST unchecked task unless blocked, then move to the next.

**Guidelines:**
- Keep edits scoped to the current task rather than reshaping the entire PR.
- Use repository instructions, conventions, and tests to validate work.
- Prefer small, reviewable commits; leave clear notes when follow-up is required.
- Do NOT work on unrelated improvements until all PR tasks are complete.

## Pre-Commit Formatting Gate (Black)

Before you commit or push any Python (`.py`) changes, you MUST:
1. Run Black to format the relevant files (line length 100).
2. Verify formatting passes CI by running:
   `black --check --line-length 100 --exclude '(\.workflows-lib|node_modules)' .`
3. If the check fails, do NOT commit/push; format again until it passes.

**COVERAGE TASKS - SPECIAL RULES:**
If a task mentions "coverage" or a percentage target (e.g., "≥95%", "to 95%"), you MUST:
1. After adding tests, run TARGETED coverage verification to avoid timeouts:
   - For a specific script like `scripts/foo.py`, run:
     `pytest tests/scripts/test_foo.py --cov=scripts/foo --cov-report=term-missing -m "not slow"`
   - If no matching test file exists, run:
     `pytest tests/ --cov=scripts/foo --cov-report=term-missing -m "not slow" -x`
2. Find the specific script in the coverage output table
3. Verify the `Cover` column shows the target percentage or higher
4. Only mark the task complete if the actual coverage meets the target
5. If coverage is below target, add more tests until it meets the target

IMPORTANT: Always use `-m "not slow"` to skip slow integration tests that may timeout.
IMPORTANT: Use targeted `--cov=scripts/specific_module` instead of `--cov=scripts` for faster feedback.

A coverage task is NOT complete just because you added tests. It is complete ONLY when the coverage command output confirms the target is met.

**The Tasks and Acceptance Criteria are provided in the appendix below.** Work through them in order.

Issue context (optional: copy into the PR description):

### Source Issue #1302: [Audit] ETL hardcodes manager_id but API-created SQLite managers table uses id
Keepalive: OFF
Base: main
Head: codex/issue-1302

Source: https://github.com/stranske/Manager-Database/issues/1302

> ## Why
> 
> The SQLite manager-id convention is inconsistent between the API (which has a dialect resolver) and the ETL flows (which hardcode `manager_id`), so ETL breaks on any SQLite database bootstrapped by the API.
> 
> - `api/managers.py:136` creates the SQLite table as `managers(id INTEGER PRIMARY KEY, ...)`, and `api/managers.py:33` `_manager_id_column(conn)` correctly returns `"id"` for SQLite / `"manager_id"` for Postgres.
> - ETL readers do NOT use that resolver — they hardcode `manager_id`: `etl/news_flow.py:71` (`SELECT manager_id, name, aliases FROM managers`), `etl/daily_diff_flow.py:138`, `diff_holdings.py:70`, `etl/activism_flow.py:124`.
> 
> **Failure mode:** on a SQLite DB created via the manager API (the documented local/dev path), ETL flows fail with `sqlite3.OperationalError: no such column: manager_id`. Prod Postgres is unaffected (its column IS `manager_id` per `schema.sql`), and CI can miss it when fixtures create a different schema than the app itself creates.
> 
> ## Scope
> 
> Make ETL manager-id access dialect-correct so the SQLite-bootstrapped path works end-to-end.
> 
> ## Non-Goals
> 
> - Do NOT change the Postgres schema (`manager_id` stays canonical there).
> - Do NOT duplicate the resolver — reuse the existing `_manager_id_column` logic.
> 
> ## Tasks
> 
> - [ ] Export/reuse a shared manager-id column resolver and apply it in `etl/news_flow.py:71`, `etl/daily_diff_flow.py:138`, `diff_holdings.py:70`, `etl/activism_flow.py:124` (and any other hardcoded `manager_id` reader of the `managers` table). Alternatively, standardize the SQLite `managers` schema on `manager_id`.
> 
> ## Acceptance Criteria
> 
> - [ ] **New test (currently failing):** in a fresh SQLite DB, create a manager via the API path (`_ensure_manager_table()` + insert), then call an ETL reader (e.g. `_fetch_all_manager_ids(conn)` / `match_entities`); current code raises `no such column: manager_id`, fixed code returns the inserted manager id.
> - [ ] **Deliberate-break demonstration:** reverting ETL to hardcoded `manager_id` fails the new test; restoring the resolver passes.
> 
> ## Implementation Notes
> 
> Audit baseline: `main` @ `e5764a5` on 2026-06-28. Source-verified: API SQLite create at `managers.py:136`, resolver at `managers.py:33`, hardcoded ETL reads at the listed lines. Prod-Postgres path is unaffected; this is a SQLite/local-path wiring fix.
>