Scaled-Tech-Consulting/Gitea-Tools
2
0
Fork 0
Code Issues 93 Pull Requests 15 Actions Packages Projects Releases Wiki Activity

feat(webui): AI-provider connections and evidence-backed insights (Closes #650) #906

Open
jcwalker3 wants to merge 2 commits from feat/issue-650-providers-insights into master
pull from: feat/issue-650-providers-insights
merge into: :master
Conversation 6 Commits 2 Files Changed 6
+1412 -14
jcwalker3 commented 2026-07-25 15:54:06 -05:00
Owner
Copy Link
Copy Source

Summary

Implements Phase 4 AI-provider connections and evidence-backed operational insights for #650.

Surfaces

  • GET /providers + GET /api/v1/providers — declared connection status from the worker registry (no API keys, no live probe claims)
  • GET /insights + GET /api/v1/insights — advisory insight cards with mandatory evidence refs
  • Nav: Insights group is live (Insights + Providers)

Insight kinds (all require evidence; none claim actions completed)

  1. blocked_queue_pressure — traffic blocked items
  2. controller_attention — needs_controller items
  3. stale_runtime_risk — system-health mutation_safe / stale
  4. provider_without_workers — declared-available providers with zero workers
  5. analytics_failure_pressure — failure-status analytics events (when loaded)

Safety

  • Read-only routes (POST → 405)
  • advisory_only=true, claims_action_completed=false always
  • Missing sources listed under sources_unavailable
  • Redaction on free text; secrets never loaded from registry
  • Docs document interpretation limits

Test plan

  • pytest tests/test_webui_providers_insights.py — 19 passed
  • Docs in docs/webui-local-dev.md

Closes #650

Canonical handoff

STATE: ready-for-review
WHO_IS_NEXT: reviewer
NEXT_ACTION: Review PR for #650 providers + insights
## Summary Implements Phase 4 **AI-provider connections** and **evidence-backed operational insights** for #650. ### Surfaces - `GET /providers` + `GET /api/v1/providers` — declared connection status from the worker registry (no API keys, no live probe claims) - `GET /insights` + `GET /api/v1/insights` — advisory insight cards with mandatory evidence refs - Nav: Insights group is live (Insights + Providers) ### Insight kinds (all require evidence; none claim actions completed) 1. `blocked_queue_pressure` — traffic blocked items 2. `controller_attention` — needs_controller items 3. `stale_runtime_risk` — system-health mutation_safe / stale 4. `provider_without_workers` — declared-available providers with zero workers 5. `analytics_failure_pressure` — failure-status analytics events (when loaded) ### Safety - Read-only routes (POST → 405) - `advisory_only=true`, `claims_action_completed=false` always - Missing sources listed under `sources_unavailable` - Redaction on free text; secrets never loaded from registry - Docs document interpretation limits ## Test plan - [x] `pytest tests/test_webui_providers_insights.py` — 19 passed - [x] Docs in `docs/webui-local-dev.md` Closes #650 ## Canonical handoff ```text STATE: ready-for-review WHO_IS_NEXT: reviewer NEXT_ACTION: Review PR for #650 providers + insights ```
jcwalker3 added 1 commit 2026-07-25 15:54:06 -05:00
feat(webui): AI-provider connections and evidence-backed insights (Closes #650) 983e8ac2c7 
Add Phase 4 advisory surfaces for declared AI-provider connection status
(no secrets, no live probe claims) and operational insights derived only
from durable traffic, health, provider, and analytics evidence.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jcwalker3 commented 2026-07-25 15:54:20 -05:00
Author
Owner
Copy Link
Copy Source

Canonical Issue State

STATE: ready-for-review
WHO_IS_NEXT: reviewer
NEXT_ACTION: Review PR #906 for #650 provider connections and evidence-backed insights (read-only advisory surfaces)
NEXT_PROMPT:

Review PR #906 on prgs / Scaled-Tech-Consulting / Gitea-Tools (Closes #650). Use gitea-reviewer only. Scope: webui/insights_loader.py, insights_views.py, /providers and /insights routes, tests/test_webui_providers_insights.py, docs/webui-local-dev.md.

WHAT_HAPPENED: Implemented and published Phase 4 providers + insights. Commit 983e8ac on feat/issue-650-providers-insights; PR #906 opened; issue #650 labeled status:pr-open. Selected via allocator (controller + author both selected #650; merger had no unclaimed merge work for #904).
WHY: Allocator forbids self-select merge of #904; next permissible work was author implement #650.
RELATED_PRS: #906
BLOCKERS: none
VALIDATION: pytest tests/test_webui_providers_insights.py — 19 passed
LAST_UPDATED_BY: author/jcwalker3

## Canonical Issue State STATE: ready-for-review WHO_IS_NEXT: reviewer NEXT_ACTION: Review PR #906 for #650 provider connections and evidence-backed insights (read-only advisory surfaces) NEXT_PROMPT: ```text Review PR #906 on prgs / Scaled-Tech-Consulting / Gitea-Tools (Closes #650). Use gitea-reviewer only. Scope: webui/insights_loader.py, insights_views.py, /providers and /insights routes, tests/test_webui_providers_insights.py, docs/webui-local-dev.md. ``` WHAT_HAPPENED: Implemented and published Phase 4 providers + insights. Commit 983e8ac on feat/issue-650-providers-insights; PR #906 opened; issue #650 labeled status:pr-open. Selected via allocator (controller + author both selected #650; merger had no unclaimed merge work for #904). WHY: Allocator forbids self-select merge of #904; next permissible work was author implement #650. RELATED_PRS: #906 BLOCKERS: none VALIDATION: pytest tests/test_webui_providers_insights.py — 19 passed LAST_UPDATED_BY: author/jcwalker3
jcwalker3 commented 2026-07-25 16:53:33 -05:00
Author
Owner
Copy Link
Copy Source

pr: #906
branch: feat/issue-650-providers-insights
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/conflict-fix-pr-906-983e8ac
profile: prgs-author
session_id: unknown
phase: claimed
head_before: 983e8ac2c7
expires_at: 2026-07-25T23:53:33Z
reviewer_active: no

<!-- mcp-conflict-fix-lease:v1 --> pr: #906 branch: feat/issue-650-providers-insights worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/conflict-fix-pr-906-983e8ac profile: prgs-author session_id: unknown phase: claimed head_before: 983e8ac2c7d7752753c17b29139371b320bbab7c expires_at: 2026-07-25T23:53:33Z reviewer_active: no
jcwalker3 added 1 commit 2026-07-25 16:53:40 -05:00
merge(master): resolve conflicts for PR #906 providers/insights af70a27b01 
Merge prgs/master into feat/issue-650-providers-insights. Conflicts in
webui/app.py, webui/nav.py, and docs/webui-local-dev.md kept both:
- #650 providers/insights routes and docs
- #645 gitea linkage routes and docs

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jcwalker3 commented 2026-07-25 16:54:29 -05:00
Author
Owner
Copy Link
Copy Source

Canonical Issue State

STATE: ready-for-review
WHO_IS_NEXT: reviewer
NEXT_ACTION: Fresh review of PR #906 at head af70a27b01 after conflict-fix merge from master.
NEXT_PROMPT:

Act as reviewer on prgs/Scaled-Tech-Consulting/Gitea-Tools PR #906. Pin expected_head_sha=af70a27b018cfdc6b275c11d76bd608f95619ced. Scope is #650 providers/insights plus merge resolution that kept #645 gitea linkage routes from master. Validate in a branches/ worktree; do not merge.

WHAT_HAPPENED: Author conflict-fix on PR #906. Live mergeable was false at 983e8ac. Merged prgs/master into feat/issue-650-providers-insights; resolved conflicts in webui/app.py, webui/nav.py, docs/webui-local-dev.md by keeping both #650 providers/insights and #645 linkage. Pushed af70a27 (no force). Gitea now reports mergeable=true.
WHY: Master had landed #645 linkage console which touched the same app/nav/docs surfaces as #650.
RELATED_PRS: #906
BLOCKERS: none remaining for mergeability; formal review still required at new head
VALIDATION: WEBUI_TEST_OFFLINE=1 venv/bin/pytest tests/test_webui_providers_insights.py tests/test_webui_gitea_linkage.py -q from branches/conflict-fix-pr-906-983e8ac → 68 passed, 6 subtests passed; local merge-tree clean; master is ancestor of head
LAST_UPDATED_BY: jcwalker3 (prgs-author)

## Canonical Issue State STATE: ready-for-review WHO_IS_NEXT: reviewer NEXT_ACTION: Fresh review of PR #906 at head af70a27b018cfdc6b275c11d76bd608f95619ced after conflict-fix merge from master. NEXT_PROMPT: ```text Act as reviewer on prgs/Scaled-Tech-Consulting/Gitea-Tools PR #906. Pin expected_head_sha=af70a27b018cfdc6b275c11d76bd608f95619ced. Scope is #650 providers/insights plus merge resolution that kept #645 gitea linkage routes from master. Validate in a branches/ worktree; do not merge. ``` WHAT_HAPPENED: Author conflict-fix on PR #906. Live mergeable was false at 983e8ac. Merged prgs/master into feat/issue-650-providers-insights; resolved conflicts in webui/app.py, webui/nav.py, docs/webui-local-dev.md by keeping both #650 providers/insights and #645 linkage. Pushed af70a27 (no force). Gitea now reports mergeable=true. WHY: Master had landed #645 linkage console which touched the same app/nav/docs surfaces as #650. RELATED_PRS: #906 BLOCKERS: none remaining for mergeability; formal review still required at new head VALIDATION: WEBUI_TEST_OFFLINE=1 venv/bin/pytest tests/test_webui_providers_insights.py tests/test_webui_gitea_linkage.py -q from branches/conflict-fix-pr-906-983e8ac → 68 passed, 6 subtests passed; local merge-tree clean; master is ancestor of head LAST_UPDATED_BY: jcwalker3 (prgs-author)
sysadmin commented 2026-07-25 17:25:11 -05:00
Owner
Copy Link
Copy Source

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #906
issue: #650
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: prgs-reviewer-78651-cbbd1dba
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/conflict-fix-pr-906-983e8ac
phase: claimed
candidate_head: af70a27b01
target_branch: master
target_branch_sha: 26f54851d1
last_activity: 2026-07-25T22:25:11Z
expires_at: 2026-07-25T22:35:11Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #906 issue: #650 reviewer_identity: sysadmin profile: prgs-reviewer session_id: prgs-reviewer-78651-cbbd1dba worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/conflict-fix-pr-906-983e8ac phase: claimed candidate_head: af70a27b018cfdc6b275c11d76bd608f95619ced target_branch: master target_branch_sha: 26f54851d1d20eff92c339e778125f31c09fa95f last_activity: 2026-07-25T22:25:11Z expires_at: 2026-07-25T22:35:11Z blocker: none
sysadmin commented 2026-07-25 17:28:18 -05:00
Owner
Copy Link
Copy Source

Reviewer findings for PR #906 at head af70a27b01. Verdict held: REQUEST_CHANGES, one blocker.

This is posted as a discussion comment rather than a formal review verdict because gitea_mark_final_review_decision fail-closed on an unreleased author conflict-fix lease (comment 16866, profile prgs-author, phase=claimed, head_before 983e8ac2, expires_at 2026-07-25T23:53:33Z). The conflict fix it covers already succeeded — this head is the merge result and Gitea reports the PR mergeable — the claiming session simply never posted its release comment. A reviewer cannot clear a comment-only author lease. The formal REQUEST_CHANGES verdict will be recorded once that lease is released or its TTL expires. Until then, treat this comment as the review of record: REVIEW_STATUS on this PR is REQUEST_CHANGES, and it must not land at this head.

One blocker. The rest of this change is in good shape, and the conflict resolution that produced this head is correct — that was audited first and is reported below as a positive finding rather than left implicit.

What was audited and is sound

The conflict-fix merge kept both sides. This head is a merge commit with parents 983e8ac2c7d7752753c17b29139371b320bbab7c (the #650 work) and 9c69bfcd80f62b112d89acc875ec13c0af714c49 (master carrying #645). I diffed the merge result against each parent for all three hand-resolved files. Against the master parent the only deletions are webui/nav.py's /insights stub NavItem and its STUB_PAGES placeholder entry, plus the matching docs/webui-local-dev.md stub row — which is exactly what #650 is supposed to replace. Against the author parent the only deletion is one docstring line superseded by the combined wording. #645's linkage surface survives intact: NavItem("/gitea", "Issue/PR linkage"), the linkage_loader/linkage_views imports, and both /gitea and /api/v1/gitea/linkage routes are all present. Nothing was dropped by the resolution.

HTML escaping is thorough. Every interpolation in webui/insights_views.py that carries text passes through html.escape. The handful that do not are integers (worker_count, enabled_worker_count) or already-escaped fragments assembled upstream. I found no injection path.

Read-only posture holds. All four routes are registered methods=["GET"], so non-GET yields 405 structurally rather than by convention. Nothing in insights_loader.py writes to Gitea, the control plane, or the registry. Provider records come from the credential-free worker registry and to_dict hard-codes secrets_exposed: False; no key material is read or rendered.

Evidence discipline holds. _require_evidence raises rather than emitting an insight with no evidence refs, and every generator routes through it.

B1 — InsightsSnapshot.ok can never be False, so a total evidence blackout renders as healthy

webui/insights_loader.py:692:

ok = bool(used) or bool(merged_unavailable)

In generate_insights, each of the four sources (traffic, system_health, providers, analytics) appends to exactly one of used or unavailable on every path — there is no branch where a source contributes to neither. So len(used) + len(unavailable) == 4 invariantly, and ok is unconditionally True. It is not reachable as False for any input.

Demonstrated at this head:

$ WEBUI_TEST_OFFLINE=1 python -c "from webui.insights_loader import load_insights_snapshot; ..."
ok            = True
insights      = 0
sources_used  = ()
fetch_error   = 'no evidence sources produced a usable snapshot'
unavailable   = 4

HTTP status would be: 200

The snapshot simultaneously reports ok: true and fetch_error: "no evidence sources produced a usable snapshot". Those two statements contradict each other.

This leaves three deliberately-authored branches unreachable:

  1. webui/app.py — status_code=200 if snapshot.ok else 502 on /api/v1/insights. The 502 can never be returned.
  2. webui/insights_views.py:130 — if not snapshot.ok and not snapshot.insights: guards the prominent health-stale "Insights unavailable" banner. It can never render.
  3. The fetch_error value itself, which is only populated when used is empty — i.e. only in the state that ok refuses to report.

Why this is a blocker rather than a tidy-up: the module's own docstring states the rule this violates — "Qualified absence. When a source could not run, the insight list says so rather than inventing an empty-and-healthy fleet or zero blocked items." A consumer polling /api/v1/insights during a full observability blackout receives HTTP 200, ok: true, and insights: []. That is precisely an empty-and-healthy fleet. The existence of the unreachable 502 branch is itself evidence that a failure signal was intended here and is simply not wired up; this is unmet intent, not a deliberate contract choice. It also makes ok mean two different things across the two endpoints this PR adds, since ProviderSnapshot.ok is correctly False on registry failure and does drive a real 502.

Stated fairly, the mitigations: the payload still carries sources_unavailable and fetch_error, and the HTML page still lists unavailable sources in its meta line, so a careful consumer can recover the truth. The defect is that the two headline signals — the ok field and the HTTP status — both say "fine" when nothing was measured.

Fix is small: ok = bool(used), plus a test asserting that a snapshot with no usable source reports ok False and that /api/v1/insights answers 502 in that state.

Non-blocking observations

  • webui/insights_loader.py:367 — severity=SEVERITY_WARN if count else SEVERITY_INFO. The function returns early when needs is empty, so count is always at least 1 and the SEVERITY_INFO branch is dead.
  • Evidence lists are capped at 20 in insight_blocked_queue, insight_controller_attention, and insight_analytics_failures, but the cap is not disclosed in the emitted payload. The title still reports the full count, so a reader comparing "43 blocked items" against 20 evidence refs cannot tell truncation from missing evidence. Consider an explicit evidence_truncated flag.
  • webui/insights_loader.py:551-553 and the equivalent guards catch ValueError around each builder to absorb _require_evidence. That same handler also swallows a ValueError raised by the int(...) coercions on number, pr, and issue, so malformed upstream data silently drops an insight rather than surfacing. Narrow the guard, or coerce defensively before building.
  • EvidenceRef.to_dict redacts detail but leaves kind and ref unredacted, and ProviderConnection.provider_id is unredacted while every other string field on that record is redacted. Low risk given the current sources, but the asymmetry looks unintentional.
  • insight_blocked_queue keys insight_id on the count alone. Unique within a snapshot, so nothing is broken today, but the id is not stable across snapshots in a way a consumer could rely on.

Base state, for the record

This head took its master merge at 9c69bfcd80f62b112d89acc875ec13c0af714c49. Master has since advanced to 26f54851d1d20eff92c339e778125f31c09fa95f, so master is no longer an ancestor of this head. Gitea still reports the PR as mergeable at the time of this review. That is not a finding against the change — it is stated so the next actor does not mistake it for one.

Canonical PR State

STATE: PR-open
WHO_IS_NEXT: author
NEXT_ACTION: Make InsightsSnapshot.ok reachable as False when no evidence source contributed, add a regression test, then push and request re-review.
NEXT_PROMPT:

Address the REQUEST_CHANGES findings on PR #906 (Closes #650), prgs /
Scaled-Tech-Consulting / Gitea-Tools, reviewed at head
af70a27b018cfdc6b275c11d76bd608f95619ced.

B1 (webui/insights_loader.py:692) — ok = bool(used) or bool(merged_unavailable) is
unconditionally True, because generate_insights appends each of the four sources to
exactly one of used or unavailable, so len(used) + len(unavailable) == 4 always. A
total evidence blackout therefore returns ok true, insights [], HTTP 200, while
fetch_error reads "no evidence sources produced a usable snapshot". This makes three
authored branches unreachable: the 502 in webui/app.py on /api/v1/insights, the
"Insights unavailable" banner at webui/insights_views.py:130, and the fetch_error
value itself. It contradicts the module docstring's own "qualified absence" rule
against inventing an empty-and-healthy fleet.

Fix: ok = bool(used). Add a test asserting that a snapshot with no usable source
reports ok False and that /api/v1/insights answers 502 in that state.

Also release the stale conflict-fix lease at comment 16866 (phase=claimed,
head_before 983e8ac2) — the fix it covers already landed, and while it is held the
reviewer cannot record a formal verdict on this PR.

Non-blocking, worth clearing in the same push:
- insights_loader.py:367 — the SEVERITY_INFO branch is dead; count is always >= 1.
- Evidence lists cap at 20 in three generators with no truncation flag in the payload,
  while the title reports the full count.
- The except ValueError guard around each builder also swallows ValueError from the
  int() coercions on number/pr/issue, silently dropping an insight.
- EvidenceRef.kind/ref and ProviderConnection.provider_id are unredacted while
  neighbouring string fields are redacted.

Not in scope for the fix: the conflict resolution at this head was audited and is
correct — both #650 and #645 content survive, and the linkage routes and nav entry
are intact. Do not redo the merge.

WHAT_HAPPENED: PR #906 was reviewed in full at head af70a27b01, in a clean branches/ worktree pinned to that exact sha. The conflict-fix merge was audited against both parents and found correct. Escaping, read-only posture, secret handling, and evidence discipline were checked and found sound. One blocker was found and reproduced by execution: InsightsSnapshot.ok is unconditionally True, so a total evidence blackout presents as a healthy empty result and three authored failure branches are unreachable. Recording of the formal verdict was refused by the reviewer gate on account of an unreleased author conflict-fix lease, so the findings are placed here instead.
WHY: The blocker matters because this console surface exists to distinguish "measured, and there is nothing to report" from "nothing could be measured". The module docstring commits to that distinction explicitly. As written, the two signals a consumer reads first — the ok field and the HTTP status — collapse those two states into one, and the unreachable 502 branch shows a failure signal was intended rather than deliberately omitted. The remaining observations are genuine but narrow, and none of them alone would hold up the change.
ISSUE: #650
RELATED_PRS: #906
HEAD_SHA: af70a27b01
REVIEW_STATUS: REQUEST_CHANGES
MERGE_READY: false
BLOCKERS: B1 InsightsSnapshot.ok is unconditionally True, so a total evidence blackout returns ok true with HTTP 200 and leaves the 502 branch, the unavailable banner, and fetch_error all unreachable. Separately, the unreleased author conflict-fix lease at comment 16866 blocks recording of the formal verdict.
VALIDATION: Reviewed in the worktree /Users/jasonwalker/Development/Gitea-Tools/branches/conflict-fix-pr-906-983e8ac, verified clean and pinned at af70a27b01. Live head SHA before these findings: af70a27b01. Pushes occurred during validation: no. Merge audit: git diff against each parent for webui/app.py, webui/nav.py, and docs/webui-local-dev.md; master-side deletions limited to the /insights stub NavItem, its STUB_PAGES entry, and the matching docs row; author-side deletions limited to one superseded docstring line. B1 was reproduced by execution, not inferred: load_insights_snapshot(load_live=False) under WEBUI_TEST_OFFLINE=1 returned ok True, zero insights, empty sources_used, four entries in sources_unavailable, and fetch_error "no evidence sources produced a usable snapshot", which maps to HTTP 200. Focused suites: WEBUI_TEST_OFFLINE=1 pytest tests/test_webui_providers_insights.py tests/test_webui_gitea_linkage.py — 68 passed, 6 subtests passed, matching the author's reported figure and confirming the merge left #645 green. No full-suite run was performed at this head and none is claimed. Immediately before posting, the head was re-confirmed as unmoved at af70a27b01, the worktree re-confirmed clean, and B1 re-read at webui/insights_loader.py:692 where ok = bool(used) or bool(merged_unavailable) still stands unchanged. The one assertion that had gone stale between the review pass and this posting was the base-master sha, corrected above from 7bb5ff47 to 26f54851; no finding depended on it.
LAST_UPDATED_BY: sysadmin (prgs-reviewer)

NATIVE_REVIEW_PROOF: findings posted via gitea_create_issue_comment on the native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, at head af70a27b01. No offline, import, or helper path was used. No formal review verdict has been recorded on PR #906 at this head; this comment does not claim one.

[THREAD STATE LEDGER]

what is true now

PR #906 remains in open state at head af70a27b01. It carries the #650 providers and insights surfaces plus a conflict-fix merge of master taken at 9c69bfcd80.

Server-side decision state: no formal review verdict of any kind is recorded on PR #906. The reviewer verdict is held locally and stated in this comment; the gate refused to record it while the author conflict-fix lease at comment 16866 remains at phase=claimed.
Local verdict/state: REQUEST_CHANGES on one blocker, with the conflict resolution separately audited and found correct.

what changed

The review moved from unstarted to a recorded set of findings in the discussion thread, short of a formal verdict. The PR head did not move during validation, and has not moved since the review pass: it is still af70a27b01. Master advanced to 26f54851d1 after this head took its merge at 9c69bfcd, so master is no longer an ancestor of the head; Gitea still reported the PR mergeable during the review. A reviewer lease was acquired for this PR at comment 16936 and is released immediately after this comment.

what is blocked

Blocker classification: code blocker

B1 — webui/insights_loader.py:692 computes ok = bool(used) or bool(merged_unavailable), which is unconditionally True because each of the four evidence sources appends to exactly one of used or unavailable. A total evidence blackout therefore reports ok: true with HTTP 200 and an empty insights list, contradicting the module docstring's qualified-absence rule, and leaving the /api/v1/insights 502 branch, the "Insights unavailable" banner at webui/insights_views.py:130, and the fetch_error value all unreachable.

Separately, recording of the formal verdict is fenced by the unreleased author conflict-fix lease at comment 16866, which expires 2026-07-25T23:53:33Z.

who/what acts next

Next actor: author
Required action: Change ok to depend only on whether a source contributed, add a regression test covering the no-usable-source state and its 502, release the stale conflict-fix lease at comment 16866, and optionally clear the five non-blocking observations in the same push.
Do not do: Do not redo or revert the conflict-fix merge — it was audited against both parents and preserves both #650 and #645, including the linkage routes and nav entry. Do not treat the 68 passing focused tests as evidence against B1; no test in the branch exercises the no-usable-source state, which is why the defect survived. Do not read the absence of a formal verdict as a positive decision on this PR.

Reviewer findings for PR #906 at head af70a27b018cfdc6b275c11d76bd608f95619ced. Verdict held: REQUEST_CHANGES, one blocker. This is posted as a discussion comment rather than a formal review verdict because `gitea_mark_final_review_decision` fail-closed on an unreleased author conflict-fix lease (comment 16866, profile prgs-author, phase=claimed, head_before 983e8ac2, expires_at 2026-07-25T23:53:33Z). The conflict fix it covers already succeeded — this head *is* the merge result and Gitea reports the PR mergeable — the claiming session simply never posted its release comment. A reviewer cannot clear a comment-only author lease. The formal REQUEST_CHANGES verdict will be recorded once that lease is released or its TTL expires. Until then, treat this comment as the review of record: REVIEW_STATUS on this PR is REQUEST_CHANGES, and it must not land at this head. One blocker. The rest of this change is in good shape, and the conflict resolution that produced this head is correct — that was audited first and is reported below as a positive finding rather than left implicit. ## What was audited and is sound **The conflict-fix merge kept both sides.** This head is a merge commit with parents `983e8ac2c7d7752753c17b29139371b320bbab7c` (the #650 work) and `9c69bfcd80f62b112d89acc875ec13c0af714c49` (master carrying #645). I diffed the merge result against each parent for all three hand-resolved files. Against the master parent the only deletions are `webui/nav.py`'s `/insights` **stub** `NavItem` and its `STUB_PAGES` placeholder entry, plus the matching `docs/webui-local-dev.md` stub row — which is exactly what #650 is supposed to replace. Against the author parent the only deletion is one docstring line superseded by the combined wording. #645's linkage surface survives intact: `NavItem("/gitea", "Issue/PR linkage")`, the `linkage_loader`/`linkage_views` imports, and both `/gitea` and `/api/v1/gitea/linkage` routes are all present. Nothing was dropped by the resolution. **HTML escaping is thorough.** Every interpolation in `webui/insights_views.py` that carries text passes through `html.escape`. The handful that do not are integers (`worker_count`, `enabled_worker_count`) or already-escaped fragments assembled upstream. I found no injection path. **Read-only posture holds.** All four routes are registered `methods=["GET"]`, so non-GET yields 405 structurally rather than by convention. Nothing in `insights_loader.py` writes to Gitea, the control plane, or the registry. Provider records come from the credential-free worker registry and `to_dict` hard-codes `secrets_exposed: False`; no key material is read or rendered. **Evidence discipline holds.** `_require_evidence` raises rather than emitting an insight with no evidence refs, and every generator routes through it. ## B1 — `InsightsSnapshot.ok` can never be False, so a total evidence blackout renders as healthy `webui/insights_loader.py:692`: ```python ok = bool(used) or bool(merged_unavailable) ``` In `generate_insights`, each of the four sources (traffic, system_health, providers, analytics) appends to **exactly one** of `used` or `unavailable` on every path — there is no branch where a source contributes to neither. So `len(used) + len(unavailable) == 4` invariantly, and `ok` is unconditionally True. It is not reachable as False for any input. Demonstrated at this head: ``` $ WEBUI_TEST_OFFLINE=1 python -c "from webui.insights_loader import load_insights_snapshot; ..." ok = True insights = 0 sources_used = () fetch_error = 'no evidence sources produced a usable snapshot' unavailable = 4 HTTP status would be: 200 ``` The snapshot simultaneously reports `ok: true` and `fetch_error: "no evidence sources produced a usable snapshot"`. Those two statements contradict each other. This leaves three deliberately-authored branches unreachable: 1. `webui/app.py` — `status_code=200 if snapshot.ok else 502` on `/api/v1/insights`. The 502 can never be returned. 2. `webui/insights_views.py:130` — `if not snapshot.ok and not snapshot.insights:` guards the prominent `health-stale` "Insights unavailable" banner. It can never render. 3. The `fetch_error` value itself, which is only populated when `used` is empty — i.e. only in the state that `ok` refuses to report. Why this is a blocker rather than a tidy-up: the module's own docstring states the rule this violates — "**Qualified absence.** When a source could not run, the insight list says so rather than inventing an empty-and-healthy fleet or zero blocked items." A consumer polling `/api/v1/insights` during a full observability blackout receives HTTP 200, `ok: true`, and `insights: []`. That is precisely an empty-and-healthy fleet. The existence of the unreachable 502 branch is itself evidence that a failure signal was intended here and is simply not wired up; this is unmet intent, not a deliberate contract choice. It also makes `ok` mean two different things across the two endpoints this PR adds, since `ProviderSnapshot.ok` is correctly False on registry failure and does drive a real 502. Stated fairly, the mitigations: the payload still carries `sources_unavailable` and `fetch_error`, and the HTML page still lists unavailable sources in its meta line, so a careful consumer can recover the truth. The defect is that the two headline signals — the `ok` field and the HTTP status — both say "fine" when nothing was measured. Fix is small: `ok = bool(used)`, plus a test asserting that a snapshot with no usable source reports `ok False` and that `/api/v1/insights` answers 502 in that state. ## Non-blocking observations - `webui/insights_loader.py:367` — `severity=SEVERITY_WARN if count else SEVERITY_INFO`. The function returns early when `needs` is empty, so `count` is always at least 1 and the `SEVERITY_INFO` branch is dead. - Evidence lists are capped at 20 in `insight_blocked_queue`, `insight_controller_attention`, and `insight_analytics_failures`, but the cap is not disclosed in the emitted payload. The title still reports the full count, so a reader comparing "43 blocked items" against 20 evidence refs cannot tell truncation from missing evidence. Consider an explicit `evidence_truncated` flag. - `webui/insights_loader.py:551-553` and the equivalent guards catch `ValueError` around each builder to absorb `_require_evidence`. That same handler also swallows a `ValueError` raised by the `int(...)` coercions on `number`, `pr`, and `issue`, so malformed upstream data silently drops an insight rather than surfacing. Narrow the guard, or coerce defensively before building. - `EvidenceRef.to_dict` redacts `detail` but leaves `kind` and `ref` unredacted, and `ProviderConnection.provider_id` is unredacted while every other string field on that record is redacted. Low risk given the current sources, but the asymmetry looks unintentional. - `insight_blocked_queue` keys `insight_id` on the count alone. Unique within a snapshot, so nothing is broken today, but the id is not stable across snapshots in a way a consumer could rely on. ## Base state, for the record This head took its master merge at `9c69bfcd80f62b112d89acc875ec13c0af714c49`. Master has since advanced to `26f54851d1d20eff92c339e778125f31c09fa95f`, so master is no longer an ancestor of this head. Gitea still reports the PR as mergeable at the time of this review. That is not a finding against the change — it is stated so the next actor does not mistake it for one. ## Canonical PR State STATE: PR-open WHO_IS_NEXT: author NEXT_ACTION: Make InsightsSnapshot.ok reachable as False when no evidence source contributed, add a regression test, then push and request re-review. NEXT_PROMPT: ```text Address the REQUEST_CHANGES findings on PR #906 (Closes #650), prgs / Scaled-Tech-Consulting / Gitea-Tools, reviewed at head af70a27b018cfdc6b275c11d76bd608f95619ced. B1 (webui/insights_loader.py:692) — ok = bool(used) or bool(merged_unavailable) is unconditionally True, because generate_insights appends each of the four sources to exactly one of used or unavailable, so len(used) + len(unavailable) == 4 always. A total evidence blackout therefore returns ok true, insights [], HTTP 200, while fetch_error reads "no evidence sources produced a usable snapshot". This makes three authored branches unreachable: the 502 in webui/app.py on /api/v1/insights, the "Insights unavailable" banner at webui/insights_views.py:130, and the fetch_error value itself. It contradicts the module docstring's own "qualified absence" rule against inventing an empty-and-healthy fleet. Fix: ok = bool(used). Add a test asserting that a snapshot with no usable source reports ok False and that /api/v1/insights answers 502 in that state. Also release the stale conflict-fix lease at comment 16866 (phase=claimed, head_before 983e8ac2) — the fix it covers already landed, and while it is held the reviewer cannot record a formal verdict on this PR. Non-blocking, worth clearing in the same push: - insights_loader.py:367 — the SEVERITY_INFO branch is dead; count is always >= 1. - Evidence lists cap at 20 in three generators with no truncation flag in the payload, while the title reports the full count. - The except ValueError guard around each builder also swallows ValueError from the int() coercions on number/pr/issue, silently dropping an insight. - EvidenceRef.kind/ref and ProviderConnection.provider_id are unredacted while neighbouring string fields are redacted. Not in scope for the fix: the conflict resolution at this head was audited and is correct — both #650 and #645 content survive, and the linkage routes and nav entry are intact. Do not redo the merge. ``` WHAT_HAPPENED: PR #906 was reviewed in full at head af70a27b018cfdc6b275c11d76bd608f95619ced, in a clean branches/ worktree pinned to that exact sha. The conflict-fix merge was audited against both parents and found correct. Escaping, read-only posture, secret handling, and evidence discipline were checked and found sound. One blocker was found and reproduced by execution: InsightsSnapshot.ok is unconditionally True, so a total evidence blackout presents as a healthy empty result and three authored failure branches are unreachable. Recording of the formal verdict was refused by the reviewer gate on account of an unreleased author conflict-fix lease, so the findings are placed here instead. WHY: The blocker matters because this console surface exists to distinguish "measured, and there is nothing to report" from "nothing could be measured". The module docstring commits to that distinction explicitly. As written, the two signals a consumer reads first — the ok field and the HTTP status — collapse those two states into one, and the unreachable 502 branch shows a failure signal was intended rather than deliberately omitted. The remaining observations are genuine but narrow, and none of them alone would hold up the change. ISSUE: #650 RELATED_PRS: #906 HEAD_SHA: af70a27b018cfdc6b275c11d76bd608f95619ced REVIEW_STATUS: REQUEST_CHANGES MERGE_READY: false BLOCKERS: B1 InsightsSnapshot.ok is unconditionally True, so a total evidence blackout returns ok true with HTTP 200 and leaves the 502 branch, the unavailable banner, and fetch_error all unreachable. Separately, the unreleased author conflict-fix lease at comment 16866 blocks recording of the formal verdict. VALIDATION: Reviewed in the worktree /Users/jasonwalker/Development/Gitea-Tools/branches/conflict-fix-pr-906-983e8ac, verified clean and pinned at af70a27b018cfdc6b275c11d76bd608f95619ced. Live head SHA before these findings: af70a27b018cfdc6b275c11d76bd608f95619ced. Pushes occurred during validation: no. Merge audit: git diff against each parent for webui/app.py, webui/nav.py, and docs/webui-local-dev.md; master-side deletions limited to the /insights stub NavItem, its STUB_PAGES entry, and the matching docs row; author-side deletions limited to one superseded docstring line. B1 was reproduced by execution, not inferred: load_insights_snapshot(load_live=False) under WEBUI_TEST_OFFLINE=1 returned ok True, zero insights, empty sources_used, four entries in sources_unavailable, and fetch_error "no evidence sources produced a usable snapshot", which maps to HTTP 200. Focused suites: WEBUI_TEST_OFFLINE=1 pytest tests/test_webui_providers_insights.py tests/test_webui_gitea_linkage.py — 68 passed, 6 subtests passed, matching the author's reported figure and confirming the merge left #645 green. No full-suite run was performed at this head and none is claimed. Immediately before posting, the head was re-confirmed as unmoved at af70a27b018cfdc6b275c11d76bd608f95619ced, the worktree re-confirmed clean, and B1 re-read at webui/insights_loader.py:692 where `ok = bool(used) or bool(merged_unavailable)` still stands unchanged. The one assertion that had gone stale between the review pass and this posting was the base-master sha, corrected above from 7bb5ff47 to 26f54851; no finding depended on it. LAST_UPDATED_BY: sysadmin (prgs-reviewer) NATIVE_REVIEW_PROOF: findings posted via gitea_create_issue_comment on the native MCP namespace gitea-reviewer, profile prgs-reviewer, identity sysadmin, at head af70a27b018cfdc6b275c11d76bd608f95619ced. No offline, import, or helper path was used. No formal review verdict has been recorded on PR #906 at this head; this comment does not claim one. [THREAD STATE LEDGER] ### what is true now PR #906 remains in open state at head af70a27b018cfdc6b275c11d76bd608f95619ced. It carries the #650 providers and insights surfaces plus a conflict-fix merge of master taken at 9c69bfcd80f62b112d89acc875ec13c0af714c49. Server-side decision state: no formal review verdict of any kind is recorded on PR #906. The reviewer verdict is held locally and stated in this comment; the gate refused to record it while the author conflict-fix lease at comment 16866 remains at phase=claimed. Local verdict/state: REQUEST_CHANGES on one blocker, with the conflict resolution separately audited and found correct. ### what changed The review moved from unstarted to a recorded set of findings in the discussion thread, short of a formal verdict. The PR head did not move during validation, and has not moved since the review pass: it is still af70a27b018cfdc6b275c11d76bd608f95619ced. Master advanced to 26f54851d1d20eff92c339e778125f31c09fa95f after this head took its merge at 9c69bfcd, so master is no longer an ancestor of the head; Gitea still reported the PR mergeable during the review. A reviewer lease was acquired for this PR at comment 16936 and is released immediately after this comment. ### what is blocked Blocker classification: code blocker B1 — `webui/insights_loader.py:692` computes `ok = bool(used) or bool(merged_unavailable)`, which is unconditionally True because each of the four evidence sources appends to exactly one of `used` or `unavailable`. A total evidence blackout therefore reports `ok: true` with HTTP 200 and an empty insights list, contradicting the module docstring's qualified-absence rule, and leaving the `/api/v1/insights` 502 branch, the "Insights unavailable" banner at `webui/insights_views.py:130`, and the `fetch_error` value all unreachable. Separately, recording of the formal verdict is fenced by the unreleased author conflict-fix lease at comment 16866, which expires 2026-07-25T23:53:33Z. ### who/what acts next Next actor: author Required action: Change `ok` to depend only on whether a source contributed, add a regression test covering the no-usable-source state and its 502, release the stale conflict-fix lease at comment 16866, and optionally clear the five non-blocking observations in the same push. Do not do: Do not redo or revert the conflict-fix merge — it was audited against both parents and preserves both #650 and #645, including the linkage routes and nav entry. Do not treat the 68 passing focused tests as evidence against B1; no test in the branch exercises the no-usable-source state, which is why the defect survived. Do not read the absence of a formal verdict as a positive decision on this PR.
sysadmin commented 2026-07-25 17:28:28 -05:00
Owner
Copy Link
Copy Source

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #906
issue: #650
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: prgs-reviewer-78651-cbbd1dba
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/conflict-fix-pr-906-983e8ac
phase: released
candidate_head: af70a27b01
target_branch: master
target_branch_sha: 26f54851d1
last_activity: 2026-07-25T22:28:28Z
expires_at: 2026-07-25T22:38:28Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #906 issue: #650 reviewer_identity: sysadmin profile: prgs-reviewer session_id: prgs-reviewer-78651-cbbd1dba worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/conflict-fix-pr-906-983e8ac phase: released candidate_head: af70a27b018cfdc6b275c11d76bd608f95619ced target_branch: master target_branch_sha: 26f54851d1d20eff92c339e778125f31c09fa95f last_activity: 2026-07-25T22:28:28Z expires_at: 2026-07-25T22:38:28Z blocker: manual-release
You are not authorized to merge this pull request.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/issue-650-providers-insights:feat/issue-650-providers-insights
git checkout feat/issue-650-providers-insights
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
allocator
Controller-owned work allocator
 anti-stomp
Prevent concurrent LLM session stomping
 architecture
Architecture / structural design
 bug
chore
codex
OpenAI Codex client / workflow session surface
 concurrency
Concurrent session safety
 contamination
Workflow or session contamination incident
 control-plane
MCP control-plane coordination and allocation authority
 dashboard
MCP operational dashboard/queue view
 database
Internal coordination storage (SQLite/Postgres)
 design
Design / investigation, no implementation
 documentation
Docs / runbooks
 enhancement
New feature or improvement
 gitea
Gitea MCP workflow
 glitchtip
GlitchTip integration
 important
incident
Operational or process incident requiring durable audit trail
 incident-bridge
Sentry-to-Gitea incident bridging
 integration
Integration testing
 jenkins
Jenkins integration
 labels
Label taxonomy management
 leases
Lease adopt/release/expire lifecycle
 mcp
MCP server / tooling
 mcp-health
MCP namespace and runtime health
 mcp-menu
MCP menu surface
 multi-project
Work spanning multiple monitoring projects or Gitea repos
 mutating
Mutating action; requires gating
 nice-to-have
observability
Observability, metrics, traces, error reporting
 portability
Cross-platform / portability
 preflight
Shared preflight gates before mutation
 protected-branch
Protected branch / stable-branch policy concern
 queue
Work queue visibility and allocation
 read-only
Read-only, no mutation
 reconnect
MCP client reconnect/reload recovery path
 recovery
Recovery paths for stale/foreign leases
 refactor
Code refactor / restructure
 release
Release / versioning
 reliability
Reliability / failure handling
 resumable-review
Persist and resume prepared review verdicts across sessions
 reviewer
Reviewer workflow tooling
 roadmap
Roadmap / umbrella issue
 safety
Safety rails and fail-closed mutation guards
 security
Security / trust boundary
 self-hosted
Self-hosted infrastructure integration
 sentry
Sentry error monitoring integration
 stale-runtime
Stale backend daemon / runtime-vs-master parity failures
 status:blocked
Issue is blocked
 status:in-progress
Issue is being worked on
 status:pr-open
Issue has an open pull request
 status:ready
Issue is ready for work
 terminal-lock
Terminal review lock (#332) path
 testing
Tests / test coverage
 tracker
Issue tracker hygiene / meta
 type:bug
Bug or defect
 type:feature
Feature or enhancement
 type:feature
Feature or enhancement
 type:guardrail
Safety gate or guardrail
 visibility
Workflow state visibility for LLMs/operators
 workflow
Cross-tool workflow
 workflow-hardening
LLM workflow coordination hardening
 workflow-hardening
LLM workflow coordination hardening
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jcwalker3 sysadmin
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#906