get_skills reports a cross-check it never performed, because fetch_history records failures instead of raising #287

New issue

Closed

opened 2026-08-13 22:39:33 +00:00 by coilyco-ops · 0 comments

coilyco-ops commented

2026-08-13 22:39:33 +00:00

Member

Slice of #269, filed so PR #271 closes what it actually closes. Angie (ENG, claude seat).

269 has two halves. This is the first, already built. The second — the six specialties against a reachable eco.coilysiren.me:3001 — needs the live server and stays on 269.

The defect

get_skills(server="not-a-real-eco-server.invalid:3001")
  -> skillsCrossChecked: true
     skillsInUseNotInGraph: []

A hostname with no DNS record. get_server_status on the same string in the same session returns ConnectError.

Cause

The caller looks correct — it wraps fetch_history in except (httpx.HTTPError, OSError) and sets the flag false. That branch is unreachable. fetch_history catches per action and records into history.warnings rather than raising, so an unreachable server arrives as a well-formed ProgressionHistory with an empty by_specialty, and annotate_skills_coverage then sets the flag True unconditionally.

Not a swallowed exception. An exception never thrown.

Acceptance

skillsCrossChecked is false when no exporter answered, with the transport error surfaced rather than discarded.
A reachable server with a genuinely empty specialty set still reports true with []. Nothing missing is not the same as nothing checked.
Both paths covered, plus the modded case that lists what the graph omits.
Fails under a mutation restoring the unconditional annotate.

per_action_counts is the signal used — populated only where an exporter answered, and its own comment already says it exists so a reader can tell fetched-but-empty from errored.

**Slice of https://forgejo.coilysiren.me/coilyco-gaming/eco-app/issues/269, filed so PR https://forgejo.coilysiren.me/coilyco-gaming/eco-app/pulls/271 closes what it actually closes.** Angie (ENG, `claude` seat). 269 has two halves. This is the first, already built. **The second — the six specialties against a reachable `eco.coilysiren.me:3001` — needs the live server and stays on 269.** ## The defect ``` get_skills(server="not-a-real-eco-server.invalid:3001") -> skillsCrossChecked: true skillsInUseNotInGraph: [] ``` A hostname with no DNS record. `get_server_status` on the same string in the same session returns `ConnectError`. ## Cause The caller looks correct — it wraps `fetch_history` in `except (httpx.HTTPError, OSError)` and sets the flag false. **That branch is unreachable.** `fetch_history` catches per action and records into `history.warnings` rather than raising, so an unreachable server arrives as a well-formed `ProgressionHistory` with an empty `by_specialty`, and `annotate_skills_coverage` then sets the flag `True` unconditionally. Not a swallowed exception. An exception never thrown. ## Acceptance - `skillsCrossChecked` is false when no exporter answered, with the transport error surfaced rather than discarded. - A reachable server with a genuinely empty specialty set still reports true with `[]`. Nothing missing is not the same as nothing checked. - Both paths covered, plus the modded case that lists what the graph omits. - Fails under a mutation restoring the unconditional annotate. `per_action_counts` is the signal used — populated only where an exporter answered, and its own comment already says it exists so a reader can tell fetched-but-empty from errored.

coilyco-ops referenced this issue from a pull request that will close it,

2026-08-13 22:39:40 +00:00

fix(skills): report a cross-check only when the server actually answered #271