|
| 1 | +<!doctype html> |
| 2 | +<html lang="en"> |
| 3 | +<head> |
| 4 | + <meta charset="utf-8"> |
| 5 | + <title>Implementation Plan: Subjective Check-in Calendar SLA</title> |
| 6 | + <meta name="viewport" content="width=device-width, initial-scale=1"> |
| 7 | + <style> |
| 8 | + body { margin: 0; font-family: system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; background: #f6f7f9; color: #1f2937; line-height: 1.5; } |
| 9 | + main { max-width: 1120px; margin: 0 auto; padding: 32px; } |
| 10 | + section { background: #fff; border: 1px solid #e5e7eb; border-radius: 8px; padding: 20px; margin: 16px 0; } |
| 11 | + h1, h2, h3 { line-height: 1.2; margin-top: 0; } |
| 12 | + code { background: #f3f4f6; padding: 2px 5px; border-radius: 4px; } |
| 13 | + ul, ol { padding-left: 22px; } |
| 14 | + li { margin: 6px 0; } |
| 15 | + .badge { display: inline-block; padding: 4px 10px; border-radius: 999px; background: #e5e7eb; font-size: 12px; font-weight: 600; } |
| 16 | + .meta { color: #6b7280; margin: 6px 0 0; } |
| 17 | + .callout { border-left: 4px solid #2563eb; } |
| 18 | + .risk { border-left: 4px solid #dc2626; } |
| 19 | + .approval { border-left: 4px solid #d97706; } |
| 20 | + .grid { display: grid; grid-template-columns: repeat(auto-fit, minmax(260px, 1fr)); gap: 12px; } |
| 21 | + .mini { background: #f9fafb; border: 1px solid #e5e7eb; border-radius: 8px; padding: 12px; } |
| 22 | + </style> |
| 23 | +</head> |
| 24 | +<body> |
| 25 | +<main> |
| 26 | + <h1>Implementation Plan: Subjective Check-in Calendar SLA</h1> |
| 27 | + <span class="badge">Approval required before implementation</span> |
| 28 | + <p class="meta">Created 2026-06-18 for the overdue Todoist follow-up: <code>Subjective check-in — calendar SLA after explicit enabled-since date</code>.</p> |
| 29 | + |
| 30 | + <section> |
| 31 | + <h2>Task Summary</h2> |
| 32 | + <p> |
| 33 | + Add an explicit per-tenant date from which subjective Telegram check-in prompts are expected, |
| 34 | + then expose a calendar-day SLA view that marks missing expected prompt days as real failures. |
| 35 | + Keep the current latest-prompts view because it remains useful for operator history and response latency. |
| 36 | + </p> |
| 37 | + </section> |
| 38 | + |
| 39 | + <section> |
| 40 | + <h2>Current Behavior</h2> |
| 41 | + <ul> |
| 42 | + <li><code>internal/storage/subjective_checkin.go</code> owns <code>subjective_checkins</code>, with one row per <code>(date, source)</code>.</li> |
| 43 | + <li><code>GetCheckinCoverage(today, source, days)</code> deliberately returns the latest N actual rows only. It does not synthesize missing calendar dates.</li> |
| 44 | + <li>The code comment says this was intentional during rollout, because pre-rollout calendar gaps would create false failures.</li> |
| 45 | + <li><code>CheckinStatusMissing</code> already exists, and the admin UI already has CSS/status labels for <code>missing</code>, but no storage path emits missing rows today.</li> |
| 46 | + <li><code>/api/admin/checkin-coverage?days=14</code> is admin-only, resolves the active tenant, computes tenant-local today, and returns the current read model.</li> |
| 47 | + <li><code>internal/ui/templates/pages/admin.html</code> renders one Subjective check-in coverage table from that response.</li> |
| 48 | + <li>The feature-enabled check for the morning gate is currently webhook-secret based in <code>cmd/server/main.go::morningCheckinEnabled</code>; it is not persisted as a calendar SLA start date.</li> |
| 49 | + </ul> |
| 50 | + </section> |
| 51 | + |
| 52 | + <section> |
| 53 | + <h2>Desired Behavior</h2> |
| 54 | + <div class="grid"> |
| 55 | + <div class="mini"> |
| 56 | + <h3>Explicit Start</h3> |
| 57 | + <p>Store a tenant-local <code>YYYY-MM-DD</code> setting such as <code>subjective_checkin_enabled_since</code>. Empty means calendar SLA is not active yet.</p> |
| 58 | + </div> |
| 59 | + <div class="mini"> |
| 60 | + <h3>Calendar SLA</h3> |
| 61 | + <p>For dates from <code>max(enabled_since, today-days+1)</code> through today, synthesize one row per day. Missing DB rows become <code>missing</code>.</p> |
| 62 | + </div> |
| 63 | + <div class="mini"> |
| 64 | + <h3>History Preserved</h3> |
| 65 | + <p>Continue returning/rendering latest actual prompt rows separately so response latency and late answers remain easy to inspect.</p> |
| 66 | + </div> |
| 67 | + </div> |
| 68 | + </section> |
| 69 | + |
| 70 | + <section> |
| 71 | + <h2>Assumptions and Unknowns</h2> |
| 72 | + <ul> |
| 73 | + <li>Use the existing per-tenant <code>settings</code> table instead of a schema migration. The table already supports arbitrary key/value tenant settings.</li> |
| 74 | + <li>The enabled-since date should be set by an admin/operator, not inferred from the first prompt row, because the requirement says explicit enabled-since date.</li> |
| 75 | + <li>Calendar missing means no row exists for <code>(date, telegram)</code>. A row with <code>expired</code> is not missing; it means the prompt was sent and not answered before cap.</li> |
| 76 | + <li>Open question: should the implementation provide a one-click "set to first prompt date" helper, or only a manual date input?</li> |
| 77 | + <li>Open question: should today count as missing before the morning check-in window has elapsed? The conservative default is to include today only after the tenant-local morning cap has passed, or mark it as pending if the day is still in progress.</li> |
| 78 | + </ul> |
| 79 | + </section> |
| 80 | + |
| 81 | + <section> |
| 82 | + <h2>Files Likely to Change</h2> |
| 83 | + <ul> |
| 84 | + <li><code>internal/storage/subjective_checkin.go</code> - add enabled-since helpers and a calendar SLA read model that overlays actual rows on expected dates.</li> |
| 85 | + <li><code>internal/storage/subjective_checkin_test.go</code> - unit tests for calendar synthesis, missing rows, lower bound at enabled-since, and current latest-row behavior.</li> |
| 86 | + <li><code>internal/ui/handler.go</code> - extend <code>adminCheckinCoverage</code> or add a small companion admin endpoint to save/read <code>subjective_checkin_enabled_since</code>.</li> |
| 87 | + <li><code>internal/ui/admin_checkin_coverage_test.go</code> - API JSON shape tests for calendar rows and invalid date handling.</li> |
| 88 | + <li><code>internal/ui/templates/pages/admin.html</code> - add date input/save control and render calendar SLA plus latest prompt history.</li> |
| 89 | + <li><code>internal/ui/i18n_en.go</code>, <code>internal/ui/i18n_ru.go</code>, <code>internal/ui/i18n_sr.go</code> - labels for enabled-since, SLA mode, history table, pending/missing explanation if needed.</li> |
| 90 | + <li><code>internal/ui/style.go</code> - small layout adjustments if the two-table admin section needs clearer spacing.</li> |
| 91 | + </ul> |
| 92 | + </section> |
| 93 | + |
| 94 | + <section> |
| 95 | + <h2>Proposed Implementation Steps</h2> |
| 96 | + <ol> |
| 97 | + <li>Add constants and validation for <code>subjective_checkin_enabled_since</code> in storage. Accept empty or valid <code>YYYY-MM-DD</code>.</li> |
| 98 | + <li>Add <code>GetCheckinEnabledSince</code> and <code>SaveCheckinEnabledSince</code> wrappers over <code>settings</code>.</li> |
| 99 | + <li>Keep <code>GetCheckinCoverage</code> as the latest-actual history path, or rename internally to make that explicit while preserving the API shape.</li> |
| 100 | + <li>Add a calendar builder that receives <code>today</code>, <code>enabledSince</code>, <code>days</code>, and actual rows, then returns expected rows newest-first with synthesized <code>missing</code> rows.</li> |
| 101 | + <li>Extend the admin response with <code>enabled_since</code>, <code>sla_active</code>, <code>calendar_rows</code>, and <code>calendar_summary</code>, while keeping existing <code>rows</code> and <code>summary</code> for history.</li> |
| 102 | + <li>Add admin POST handling for the enabled-since date. Keep it tenant-scoped through the existing <code>resolveAdminTenantScope</code> path.</li> |
| 103 | + <li>Update the admin UI to show a date control, a clear inactive state when no date is set, a calendar SLA table, and the existing latest prompt history table.</li> |
| 104 | + <li>Update this plan after implementation with actual changes, checks run, limitations, and follow-up recommendations.</li> |
| 105 | + </ol> |
| 106 | + </section> |
| 107 | + |
| 108 | + <section class="callout"> |
| 109 | + <h2>Data, API, Auth, and UX Impact</h2> |
| 110 | + <ul> |
| 111 | + <li>Data: no new table required if the enabled-since value lives in <code>settings</code>. No destructive migration.</li> |
| 112 | + <li>API: admin-only check-in coverage response gains fields. Existing fields should remain stable to reduce template and test churn.</li> |
| 113 | + <li>Auth: no change. Reads and writes stay behind <code>adminGuard</code> and tenant scope resolution.</li> |
| 114 | + <li>UX: the admin page will distinguish "SLA not active" from "SLA active and missing prompt rows". This avoids false alarms before explicit enablement.</li> |
| 115 | + <li>Deployment: safe deploy before setting the date. Calendar missing rows only appear after the admin setting is saved.</li> |
| 116 | + </ul> |
| 117 | + </section> |
| 118 | + |
| 119 | + <section> |
| 120 | + <h2>Test Plan</h2> |
| 121 | + <ul> |
| 122 | + <li><code>go test ./internal/storage -run "Test.*Checkin|Test.*CheckinCoverage" -count=1</code></li> |
| 123 | + <li><code>go test ./internal/ui -run TestAdminCheckinCoverage -count=1</code></li> |
| 124 | + <li>If DB-backed behavior is touched beyond pure builders, run the targeted DB-enabled UI/storage tests using the repo's usual DB environment.</li> |
| 125 | + <li>Run <code>gofmt -w</code> on changed Go files.</li> |
| 126 | + <li>Manual JSON check: call <code>/api/admin/checkin-coverage?days=14</code> for a tenant with and without <code>subjective_checkin_enabled_since</code>.</li> |
| 127 | + </ul> |
| 128 | + </section> |
| 129 | + |
| 130 | + <section> |
| 131 | + <h2>Manual QA Checklist</h2> |
| 132 | + <ul> |
| 133 | + <li>Admin page shows check-in coverage without JavaScript errors when enabled-since is empty.</li> |
| 134 | + <li>Saving an enabled-since date persists for the active tenant only.</li> |
| 135 | + <li>Calendar SLA starts no earlier than enabled-since and no earlier than the selected day window.</li> |
| 136 | + <li>Dates with actual <code>answered</code>, <code>late_answered</code>, <code>expired</code>, or <code>prompted</code> rows keep those statuses.</li> |
| 137 | + <li>Dates with no row after enabled-since render as <code>missing</code>.</li> |
| 138 | + <li>Latest prompt history still shows actual rows only and still reports average response latency from answered rows.</li> |
| 139 | + <li>Multi-tenant admin tab switching reloads the correct tenant's SLA date and rows.</li> |
| 140 | + </ul> |
| 141 | + </section> |
| 142 | + |
| 143 | + <section class="risk"> |
| 144 | + <h2>Risks and Edge Cases</h2> |
| 145 | + <ul> |
| 146 | + <li>Counting today too early could create noisy false missing rows. The implementation should either exclude in-progress today until cap or label it pending.</li> |
| 147 | + <li>Timezone drift matters because dates are tenant-local. Use the same <code>tenantLocalToday</code> path as the current endpoint.</li> |
| 148 | + <li>Manual date mistakes can create many historical missing rows. Keep the setting visible and easy to correct.</li> |
| 149 | + <li>Calendar rows and latest rows have different denominators. The UI labels must make that difference explicit.</li> |
| 150 | + <li>Do not make missing rows affect health scoring, illness evidence, or report generation. This is admin observability only.</li> |
| 151 | + </ul> |
| 152 | + </section> |
| 153 | + |
| 154 | + <section> |
| 155 | + <h2>Rollback Plan</h2> |
| 156 | + <ul> |
| 157 | + <li>Clear or delete <code>settings.subjective_checkin_enabled_since</code> to disable calendar SLA output without changing prompt delivery.</li> |
| 158 | + <li>Revert the code changes if needed; no derived health data or raw health records are modified by this feature.</li> |
| 159 | + <li>If the response extension causes UI trouble, keep the storage helper and temporarily render only the existing latest history table.</li> |
| 160 | + </ul> |
| 161 | + </section> |
| 162 | + |
| 163 | + <section> |
| 164 | + <h2>Open Questions</h2> |
| 165 | + <ul> |
| 166 | + <li>Should the first production enabled-since date be set to the actual rollout date from PR 2, the first prompt row, or a manually chosen date?</li> |
| 167 | + <li>Should in-progress today be excluded, shown as <code>prompted</code> when a prompt exists, or shown as a separate <code>pending</code> state when no prompt exists yet?</li> |
| 168 | + <li>Do we want a Todoist task close/update after implementation, or should that remain manual?</li> |
| 169 | + </ul> |
| 170 | + </section> |
| 171 | + |
| 172 | + <section> |
| 173 | + <h2>Implemented Result</h2> |
| 174 | + <ul> |
| 175 | + <li>Created GitHub issue <code>#186</code>: <code>Add calendar-day SLA for subjective check-in prompts</code>.</li> |
| 176 | + <li>Added tenant-scoped setting <code>subjective_checkin_enabled_since</code> through the existing <code>settings</code> table.</li> |
| 177 | + <li>Extended <code>/api/admin/checkin-coverage</code>: <code>GET</code> returns existing latest history plus new SLA fields; <code>POST</code> saves <code>enabled_since</code> for the active tenant and returns refreshed coverage.</li> |
| 178 | + <li>Kept existing <code>summary</code> and <code>rows</code> as latest actual prompt history for compatibility.</li> |
| 179 | + <li>Added <code>sla_summary</code> and <code>sla_rows</code> for calendar-day coverage after enabled-since.</li> |
| 180 | + <li>Added <code>pending</code> for the current day when no row exists yet, so today is not prematurely counted as <code>missing</code>.</li> |
| 181 | + <li>Updated Admin UI with an enabled-since date input, Calendar SLA table, and Latest prompt history table.</li> |
| 182 | + <li>Added English, Russian, and Serbian labels plus minimal CSS for the new UI.</li> |
| 183 | + </ul> |
| 184 | + </section> |
| 185 | + |
| 186 | + <section> |
| 187 | + <h2>Checks Run</h2> |
| 188 | + <ul> |
| 189 | + <li><code>gofmt -w internal\storage\subjective_checkin.go internal\storage\subjective_checkin_test.go internal\ui\handler.go internal\ui\admin_checkin_coverage_test.go internal\ui\i18n_en.go internal\ui\i18n_ru.go internal\ui\i18n_sr.go</code></li> |
| 190 | + <li><code>go test ./internal/storage -run "Test.*Checkin|Test.*CheckinCoverage" -count=1</code> - passed.</li> |
| 191 | + <li><code>go test ./internal/ui -run TestAdminCheckinCoverage -count=1</code> - passed.</li> |
| 192 | + </ul> |
| 193 | + </section> |
| 194 | + |
| 195 | + <section> |
| 196 | + <h2>Known Limitations</h2> |
| 197 | + <ul> |
| 198 | + <li>The current day with no prompt row is shown as <code>pending</code> for the full day. This is deliberately conservative; a later refinement can switch it to <code>missing</code> after the tenant's morning cap passes.</li> |
| 199 | + <li>Clearing enabled-since stores an empty settings value rather than deleting the row; existing settings helpers treat that as inactive.</li> |
| 200 | + <li>No production setting was changed during implementation. Enabling SLA for a tenant is still an explicit admin action.</li> |
| 201 | + </ul> |
| 202 | + </section> |
| 203 | + |
| 204 | + <section class="approval"> |
| 205 | + <h2>Approval Gate</h2> |
| 206 | + <p> |
| 207 | + Implementation must not start until the user explicitly approves this plan. |
| 208 | + Suggested approval phrase: <code>погнали</code>. |
| 209 | + </p> |
| 210 | + </section> |
| 211 | +</main> |
| 212 | +</body> |
| 213 | +</html> |
0 commit comments