Skip to content

[codex] Add daily and weekly human report workflow skill#26

Merged
moose-lab merged 6 commits into
mainfrom
feat/human-report-workflow-skill
May 29, 2026
Merged

[codex] Add daily and weekly human report workflow skill#26
moose-lab merged 6 commits into
mainfrom
feat/human-report-workflow-skill

Conversation

@moose-lab

Copy link
Copy Markdown
Owner

Summary

  • Add the human report skill contract and evidence builder for readable report sections.
  • Render human report summaries in the reports UI and persist report snapshots.
  • Add a human-readable implementation workflow guide for daily and weekly reports under docs/skills/human-report, including Mermaid diagrams.

Verification

  • bun run test
  • bun run typecheck
  • bun run build

Comment thread src/core/report-summary-store.ts Fixed

try {
mkdirSync(reportsDir, { recursive: true });
writeFileSync(filePath, JSON.stringify(summary, null, 2), "utf-8");
@moose-lab moose-lab changed the title [codex] Add human report workflow skill [codex] Add daily and weekly human report workflow skill May 28, 2026
@moose-lab
moose-lab force-pushed the feat/human-report-workflow-skill branch from 96ce699 to c5f61ae Compare May 29, 2026 01:11
@moose-lab moose-lab closed this May 29, 2026
@moose-lab moose-lab reopened this May 29, 2026
@claude

claude Bot commented May 29, 2026

Copy link
Copy Markdown

Claude finished @moose-lab's task in 2m 44s —— View job


PR Review

  • Gather context and read changed files
  • Review changed files
  • Analyze security alerts
  • Post review findings

Findings

  • [must-fix] Silent snapshot failure in loadReportSummarypersistReportSnapshot(summary) result is fully ignored at report-summary-store.ts:72. A disk-full or permission error is swallowed and the caller has no way to know the snapshot wasn't written. Should at minimum log the error or surface it in the return. Fix this →

  • [must-fix] Timezone coupling in buildReportRangereport-summary.ts:183–196 creates new Date(parsed) then calls start.getDay() / start.setDate() / end.setDate() (all local-time methods). When parseDate is called with "2026-05-28 00:00:00" and the string is coerced to T format, new Date("2026-05-28T00:00:00") is parsed as local midnight, but the resulting date arithmetic uses local time. On a machine where TZ ≠ the user's intended timezone (e.g. UTC server, user in UTC-8) the Monday-of-week calculation will produce an incorrect startDate. This is the same coupling pattern the repo already has in cost-tracker. Fix this →

  • [should-fix] Redundant outcomes filter in buildHumanReportEvidencereport-evidence.ts:83 applies .filter((task) => dateInRange(task.completed_at, input.range) || dateInRange(task.updated_at, input.range)) on top of periodTasks, which was already built with taskTouchedInRange (identical condition). The second filter is dead code and will mislead future maintainers who assume it's narrowing something. Fix this →

  • [should-fix] previewText truncation inconsistency — report-evidence.ts:210 caps at 160 chars; report-summary.ts:628 caps at 140 chars. Both functions exist because each file re-implements the same helper. If both codepaths can produce a promptPreview, the appendix will show inconsistent lengths depending on which path generates the data. Pick one length and share the helper (or at minimum make the constants identical).

  • [should-fix] CodeQL polynomial regex — dateKey() uses /^(\d{4}-\d{2}-\d{2})/ on untrusted DB strings in report-evidence.ts:189 and report-summary.ts:593. The same operation is already done without a regex in parseSnapshotDate (report-summary-store.ts:130–163) for exactly this reason. The two dateKey implementations should adopt the same char-code approach to close the CodeQL alert. Fix this →

  • [nit] loadReportSummary writes a snapshot on every API call — a read-oriented function that silently writes to disk each time the UI refreshes. The write should either be explicit (caller opts in) or gated (only write if the snapshot doesn't already exist / is stale).


Verdict: REQUEST_CHANGES

The timezone coupling in buildReportRange will produce wrong week windows for anyone whose server TZ differs from the user's intent, and the silent error swallow in loadReportSummary makes failures completely invisible. Both are worth fixing before merge. The other findings are lower severity but worth a pass.

Repository owner deleted a comment from claude Bot May 29, 2026
Repository owner deleted a comment from claude Bot May 29, 2026
Repository owner deleted a comment from claude Bot May 29, 2026
@moose-lab
moose-lab force-pushed the feat/human-report-workflow-skill branch from fc99fd4 to 08b1d2b Compare May 29, 2026 14:39
@claude

claude Bot commented May 29, 2026

Copy link
Copy Markdown

Claude finished @moose-lab's task in 1m 36s —— View job


Re-review after synchronize

  • Gather context
  • Read changed files
  • Check previous findings against current code
  • Post updated review

The synchronize was a merge of main into the branch (08b1d2b). None of the findings from the first review were addressed. All five are unchanged:

Findings

  • [must-fix] Silent snapshot failure still present — report-summary-store.ts:72 calls persistReportSnapshot(summary) and drops the result. Disk-full and permission errors remain silently swallowed. Fix this →

  • [must-fix] Timezone coupling still present — report-summary.ts:183–195, buildReportRange for the weekly case: parseDate("YYYY-MM-DD 00:00:00") resolves to local midnight, then start.getDay() / start.setDate() / end.setDate() use local-time methods throughout. On a UTC server with a UTC-8 user, the Monday-of-week anchor will be wrong. The monthly branch (new Date(parsed.getFullYear(), parsed.getMonth(), 1)) is similarly local. Fix this →

  • [should-fix] Redundant outcomes filter still present — report-evidence.ts:81–84: periodTasks was built with taskTouchedInRange; the second .filter((task) => dateInRange(task.completed_at, ...) || dateInRange(task.updated_at, ...)) on line 83 is identical and dead. Fix this →

  • [should-fix] previewText truncation inconsistency still present — report-evidence.ts:210 truncates at 160; report-summary.ts:628 truncates at 140. Both functions are identical otherwise and could be shared with one constant.

  • [should-fix] CodeQL polynomial regex still present — dateKey() in report-evidence.ts:189 and report-summary.ts:593 both use /^(\d{4}-\d{2}-\d{2})/ on untrusted DB strings. parseSnapshotDate in the same store file already avoids this with a char-code approach. Fix this →

  • [nit] loadReportSummary writes a snapshot on every UI refresh — unchanged. The write should be gated (e.g. only when the snapshot doesn't exist or the summary is newer).


Verdict: REQUEST_CHANGES

The two must-fix items (silent error swallow + timezone coupling) are still open. Please address them before merge.

@moose-lab
moose-lab marked this pull request as ready for review May 29, 2026 14:43
@moose-lab
moose-lab merged commit c237c44 into main May 29, 2026
4 checks passed
@moose-lab
moose-lab deleted the feat/human-report-workflow-skill branch May 29, 2026 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants