Fix Optimize Earth root row showing zero delay metrics#81
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR updates agent and CLAUDE guidance, adds local hook scripts and dev-server checks, improves copy-preview tooling, adjusts seed data and tests (Optimize Earth and legacy grant keys), simplifies a referendum vote component, adds host-scoped redirects with tests, and expands TODOs and plan documents. ChangesPrimary changes UI copy & small component edits
Plans & TODOs
Estimated code review effort: 🎯 3 (Moderate) | ⏱️ ~20 minutes
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
This PR fixes delay-metric rendering for the root “Optimize Earth” managed task by ensuring it has a non-null dueAt, so delay-derived stats (wasted-by-delay, deaths-from-delay) no longer compute to zero on /tasks.
Changes:
- Set a concrete
dueAton the Optimize Earth root managed task to make it “overdue” for delay-metric calculations. - Added a regression test to guard the managed-task data shape for the Optimize Earth root.
- Updated internal triage documentation and the repository TODO list with follow-up items.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| TODO.md | Adds triage-cycle TODOs and a rename plan note. |
| packages/db/src/managed-data/optimize-earth-task-tree.ts | Sets dueAt on the Optimize Earth root task (and documents the motivation). |
| packages/db/src/managed-data/optimize-earth-task-tree.test.ts | New regression test asserting the root task exists and has an overdue dueAt. |
| .claude/agents/pr-comment-triager.md | Updates bot-comment triage instructions to reply per-thread/per-comment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/db/src/managed-data/optimize-earth-task-tree.test.ts (1)
19-22: ⚡ Quick winConsider checking exact dueAt value instead of using Date.now().
While the current check is unlikely to be flaky (the date is well in the past), testing guidelines discourage using
Date.now()in assertions. A more explicit check against the expected value would be more robust and self-documenting.♻️ Proposed refactor
- it("has an overdue dueAt so the root row shows non-zero cost of delay", () => { - expect(root?.dueAt).toBeInstanceOf(Date); - expect(root?.dueAt!.getTime()).toBeLessThan(Date.now()); + it("has dueAt set to 2024-12-31 (overdue) so the root row shows non-zero cost of delay", () => { + expect(root?.dueAt).toEqual(new Date("2024-12-31T00:00:00.000Z")); });This verifies both that the date exists AND that it's the expected value, making the test more precise.
As per coding guidelines, tests should avoid
Date.now()in assertions to prevent flakiness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/managed-data/optimize-earth-task-tree.test.ts` around lines 19 - 22, The test currently asserts root?.dueAt is in the past using Date.now(), which is discouraged; instead assert the exact expected timestamp/value for root?.dueAt to make the test deterministic—locate the assertion in optimize-earth-task-tree.test.ts that references root?.dueAt and replace the Date.now() comparison with a check against the known expected Date or timestamp (e.g., compare to a specific Date string or numeric epoch value) while still ensuring the value is an instance of Date via the existing expect(root?.dueAt).toBeInstanceOf(Date) check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/db/src/managed-data/optimize-earth-task-tree.ts`:
- Around line 66-72: The comment above the hard-coded dueAt date incorrectly
references END_WAR_AND_DISEASE_TASK_ID; update the comment to reference
TREATY_PARENT_TASK_ID (which contains the matching 2024-12-31 dueAt) so the
explanation correctly points to the matching task; ensure you keep the existing
rationale about getTaskDelayStats/currentDelayDays and that the date matches
TREATY_PARENT_TASK_ID.
---
Nitpick comments:
In `@packages/db/src/managed-data/optimize-earth-task-tree.test.ts`:
- Around line 19-22: The test currently asserts root?.dueAt is in the past using
Date.now(), which is discouraged; instead assert the exact expected
timestamp/value for root?.dueAt to make the test deterministic—locate the
assertion in optimize-earth-task-tree.test.ts that references root?.dueAt and
replace the Date.now() comparison with a check against the known expected Date
or timestamp (e.g., compare to a specific Date string or numeric epoch value)
while still ensuring the value is an instance of Date via the existing
expect(root?.dueAt).toBeInstanceOf(Date) check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b44ad747-9c88-41dd-b7c9-513b077a2dcd
📒 Files selected for processing (4)
.claude/agents/pr-comment-triager.mdTODO.mdpackages/db/src/managed-data/optimize-earth-task-tree.test.tspackages/db/src/managed-data/optimize-earth-task-tree.ts
Preview deploy — one-click review linksLatest preview: https://optimitron-bvvwyorlm-mike-p-sinns-projects.vercel.app
|
Code reviewTwo CLAUDE.md violations found: 1. Flaky test: real wall clock in assertionFile:
Fix: replace expect(root?.dueAt!.getTime()).toBeLessThan(new Date("2026-01-01").getTime());2. Magic number: inline date literal duplicated without a named constantFile:
Fix: extract to a named constant at the top of the file (e.g. |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎ Awaiting the start of a new Argos build… |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/db/src/managed-data/managed-seed-data.ts (1)
1280-1285: ⚡ Quick winDocument the deliberate
icewad→IC2EWDsplit.The
["ice", "wad"].join("")trick is meaningful — it preserves the legacyicewad-*/icewad:*DB keys (existing rows, idempotent upserts) while letting the new TypeScript identifiers move toIC2EWD_*as part of the bundled rename plan. Without a one-line comment that intent is invisible: the next reader will assume the array-join is accidental and "fix" it back to a string literal, which would re-introduce the very identifier the rename is removing.Also consider hoisting
IC2EWD_GRANT_DALYS_PER_USD/IC2EWD_GRANT_ECON_VALUE_PER_USDto module scope (they're pure constants and the description prose at line 1361–1364 quotes the same565 DALYs / $1figure — co-locating them keeps the number and the marketing copy from drifting).♻️ Suggested clarification
- const IC2EWD_GRANT_DALYS_PER_USD = 564.972; - const IC2EWD_GRANT_ECON_VALUE_PER_USD = IC2EWD_GRANT_DALYS_PER_USD * 150_000; - const legacyCampaignKeyStem = ["ice", "wad"].join(""); - const legacyGrantTaskIdPrefix = `${legacyCampaignKeyStem}-grant`; - const legacyGrantTaskKeyPrefix = `${legacyCampaignKeyStem}:grant`; - const legacyGrantMethodologyKey = `${legacyCampaignKeyStem}-one-dollar-grant`; + // Legacy DB key stem — kept as runtime-constructed "icewad" so existing + // task ids, taskKeys, interestTags, and methodologyKey strings round-trip + // unchanged across the ICEWAD → IC2EWD identifier rename (see TODO.md). + // Do not collapse to a string literal — that re-introduces the identifier + // the rename is intentionally removing. + const legacyCampaignKeyStem = ["ice", "wad"].join(""); + const legacyGrantTaskIdPrefix = `${legacyCampaignKeyStem}-grant`; + const legacyGrantTaskKeyPrefix = `${legacyCampaignKeyStem}:grant`; + const legacyGrantMethodologyKey = `${legacyCampaignKeyStem}-one-dollar-grant`;As per coding guidelines: "Replace magic numbers with named constants citing the paper" — the named-constant half is now satisfied; a one-line citation to
manual.warondisease.org/knowledge/economics/1-pct-treaty-impact.htmlnext toIC2EWD_GRANT_DALYS_PER_USD = 564.972would close the loop.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/managed-data/managed-seed-data.ts` around lines 1280 - 1285, Add a one-line comment explaining that legacyCampaignKeyStem = ["ice","wad"].join("") intentionally preserves existing DB keys (icewad-*/icewad:*) while new TypeScript identifiers use the IC2EWD_* naming; place this comment adjacent to legacyCampaignKeyStem and the related legacyGrantTaskIdPrefix / legacyGrantTaskKeyPrefix / legacyGrantMethodologyKey declarations so future editors don't “simplify” the join. Also hoist IC2EWD_GRANT_DALYS_PER_USD and IC2EWD_GRANT_ECON_VALUE_PER_USD to module scope (they are pure constants) and add a short citation comment (manual.warondisease.org/knowledge/economics/1-pct-treaty-impact.html) next to IC2EWD_GRANT_DALYS_PER_USD to document the source of the 564.972 value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/hooks/dev-server-check.mjs:
- Around line 33-36: The curl call uses a Windows-only "-o NUL"; update the
execSync invocation in dev-server-check.mjs (the const out = execSync(...)
block) to compute a platform-aware null device (e.g. const nullDevice =
process.platform === "win32" ? "NUL" : "/dev/null" or obtain platform via
node:os) and substitute that value into the curl command instead of the literal
"NUL" so output is discarded correctly on Unix/macOS and Windows.
In `@CLAUDE.md`:
- Around line 51-52: Trim the snapshot rule in CLAUDE.md to remove the
parenthetical implementation details: keep the core instruction that developers
must not hand-edit page.logged-out.md / *.email.md snapshots and that they
should run the commands `pnpm --filter `@optimitron/web` copy:preview` (or
`copy:preview:all` / `email:preview-md` for emails) to regenerate; delete the
explanatory parenthetical mentioning scripts/affected-routes.mjs and the
static-import walker so the rule is concise and follows the "minimum words"
guideline.
---
Nitpick comments:
In `@packages/db/src/managed-data/managed-seed-data.ts`:
- Around line 1280-1285: Add a one-line comment explaining that
legacyCampaignKeyStem = ["ice","wad"].join("") intentionally preserves existing
DB keys (icewad-*/icewad:*) while new TypeScript identifiers use the IC2EWD_*
naming; place this comment adjacent to legacyCampaignKeyStem and the related
legacyGrantTaskIdPrefix / legacyGrantTaskKeyPrefix / legacyGrantMethodologyKey
declarations so future editors don't “simplify” the join. Also hoist
IC2EWD_GRANT_DALYS_PER_USD and IC2EWD_GRANT_ECON_VALUE_PER_USD to module scope
(they are pure constants) and add a short citation comment
(manual.warondisease.org/knowledge/economics/1-pct-treaty-impact.html) next to
IC2EWD_GRANT_DALYS_PER_USD to document the source of the 564.972 value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee7c9705-6604-46f3-a095-3b254c60d8b0
📒 Files selected for processing (19)
.claude/codex-delegation.md.claude/hooks/block-codex-rescue-agent.mjs.claude/hooks/block-snapshot-handedit.mjs.claude/hooks/dev-server-check.mjs.claude/settings.json.husky/pre-commitCLAUDE.mdTODO.mdpackages/db/src/__tests__/seed.integration.test.tspackages/db/src/managed-data/managed-seed-data.tspackages/web/package.jsonpackages/web/scripts/copy-preview-smart.mjspackages/web/scripts/soft-delete-funding-tasks.tspackages/web/src/app/humanity-v-government/HumanityVGovernmentVerdictVote.test.tsxpackages/web/src/app/humanity-v-government/HumanityVGovernmentVerdictVote.tsxpackages/web/src/app/humanity-v-government/page.logged-out.mdpackages/web/src/app/humanity-v-government/page.tsxpackages/web/src/lib/__tests__/redirects.test.tspackages/web/src/lib/redirects.js
💤 Files with no reviewable changes (4)
- packages/web/src/app/humanity-v-government/HumanityVGovernmentVerdictVote.test.tsx
- packages/web/src/app/humanity-v-government/page.tsx
- packages/web/src/app/humanity-v-government/page.logged-out.md
- packages/web/src/app/humanity-v-government/HumanityVGovernmentVerdictVote.tsx
✅ Files skipped from review due to trivial changes (2)
- .claude/settings.json
- packages/web/scripts/soft-delete-funding-tasks.ts
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
TODO.md (2)
299-317: ⚡ Quick winMissing cross-reference to email-template-audit plan.
Lines 299-317 describe the
share-templates.tscanonical source directive and the audit task for email modules, but the PR also includes.claude/plans/email-template-audit.mdwhich covers this exact migration.📝 Recommended cross-reference
### `share-templates.ts` is the source of truth for ALL reminder copy - `packages/web/src/lib/tasks/share-templates.ts` is the canonical voice-variant registry: ~26 named templates (Trump versions, office memo, performance review, polite reminder, etc.) keyed by `recipientModes` (`leader | humanity | one_human | peer`) with token-based interpolation. - Today only `TreatyReminderComposer` reads from it. `monthly-chain-digest`, `post-vote-share`, `referral-first-conversion`, and `task-comment-notification` emails hand-roll their own reminder copy — confirmed for monthly-chain-digest, audit needed for the others. - Migration: every email module that includes reminder/share copy should pull recipient-appropriate templates from `share-templates.ts` (filtered by the email's `recipientModes`), interpolate via `renderTemplate`, and pick a default variant. Hand-rolled copy stays only when no template fits AND a new template would be too narrow to reuse. -- Audit task: grep all `packages/web/src/lib/email/*.ts` and - `*-react-email.tsx` for hardcoded "Sign now"/"Vote"/"You haven't voted yet"- - shaped prose and replace with template lookups. +- Full migration plan at `.claude/plans/email-template-audit.md`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@TODO.md` around lines 299 - 317, The TODO entry describing share-templates.ts as the canonical source and the email audit should be updated to reference the existing audit plan file (.claude/plans/email-template-audit.md); edit the paragraph that mentions the "Audit task" to add a brief cross-reference pointing to that plan and note it covers migration steps for modules like TreatyReminderComposer, monthly-chain-digest, post-vote-share, referral-first-conversion, and task-comment-notification; mention that modules should use share-templates.ts (filtered by recipientModes and rendered with renderTemplate) and defer to the email-template-audit plan for the detailed migration checklist.
79-216: ⚡ Quick winVerify cross-reference accuracy for the apocalypse framing plan.
Lines 159-215 describe the "apocalypse" framing standardization work in detail, including candidate phrasings and surface locations. This duplicates content that should live in the plan document at
.claude/plans/apocalypse-framing-standardization.md.The PR includes
.claude/plans/apocalypse-framing-standardization.md, which covers the same scope. Keeping parallel copies in both TODO and the plan increases maintenance cost and drift risk.📝 Recommended consolidation
Replace lines 159-215 with a brief directive pointing to the plan:
-- **Standardize "apocalypse" framing across the project.** Ivy (real- - user feedback) said *"a hundred of them ends civilization is a - confusing sentence."* The word "apocalypse" treats civilization- - ending event as a countable unit, and "122 apocalypses" / "trade - one apocalypse" doesn't land for people who haven't been told the - causal chain (~100 warheads → nuclear winter → food system collapse - → civilizational collapse; we stockpiled ~12,200 → 122x overkill). - Pick ONE standardized phrasing, parameter-back it, sweep all - surfaces. - - **User-facing surfaces to update (one consistent phrasing):** - [... 40 more lines ...] +- **Standardize "apocalypse" framing across the project.** Ivy feedback: + "a hundred of them ends civilization" is confusing. Implementation + plan at `.claude/plans/apocalypse-framing-standardization.md`.Keep detailed surface lists and candidate phrasings only in the plan document.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@TODO.md` around lines 79 - 216, Duplicate "Standardize 'apocalypse' framing" content in the TODO should be consolidated: remove the detailed block that duplicates the dedicated plan and replace it with a one-line pointer referencing the separate apocalypse-framing plan (keep candidate labels A/B/C and surface lists only in that plan), and ensure the TODO contains a short directive like "See apocalypse-framing-standardization plan for full details" so future readers are guided to the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/plans/apocalypse-framing-standardization.md:
- Around line 171-200: The core templates use camelCase tokens (e.g.
NUCLEAR_OVERKILL_FRAMING_TEMPLATES.coreFull with {overkillLayers},
{spareOverkillLayers}, {globalWarheads}) while share-templates.ts and
ShareTokenKey use snake_case, which will break interpolation; fix by making
token casing consistent—preferably convert all template tokens in
NUCLEAR_OVERKILL_FRAMING_TEMPLATES.coreFull and the listed replacement strings
(Footer.tsx, donate/page.tsx, endorse/page.tsx,
DonationCalculationNarrative.tsx, TreatyVoteFlow.tsx,
TreatyPostVoteShareFlow.tsx, managed-task-triggers.ts, managed-grandma-kay.ts,
TreatyTradeThesis.tsx, site.ts keys, routes.ts templates, war-on-disease.json,
one-percent-treaty.ts, SignatoriesLeaderboard.tsx, RepresentedPersonForm.tsx,
ManageRepresentedPeopleClient.tsx, humanity-manager-promotion-content.tsx,
ProgramTaskSection.tsx, share-templates.ts) to snake_case (e.g.
{overkill_layers}, {spare_overkill_layers}, {global_warheads},
{winter_threshold_warheads}, {dfda_years}, {status_quo_years},
{trial_capacity_multiplier}), or alternatively implement a single conversion in
renderNuclearOverkillTemplate to map camelCase→snake_case before rendering;
choose one approach and update tests (one-percent-treaty.test.ts) and regenerate
any generated files instead of hand-editing.
In @.claude/plans/email-template-audit.md:
- Around line 179-253: The plan defers choosing between
`lumbergh-one-human`/`lumbergh` for monthly digest reminders without a decision
criterion; update the checklist in the migration entry for
`monthly-chain-digest-react-email.tsx`/`monthly-chain-digest-email.ts`/`monthly-chain-digest.server.ts`
to either (A) select the variant now (e.g., use `lumbergh-one-human` for
employee reminders and `lumbergh` for president reminders) and note why, or (B)
add a concrete pre-deploy verification step that programmatically checks the
available token set (e.g., via `loadOverduePresidentSnapshot()` or a small
token-building helper) and defines a deterministic fallback to a lower-token
registry variant when required tokens are missing; include the verification task
and expected pass/fail behavior in the checklist so implementers have a clear
rule to follow.
In @.claude/plans/login-form-ux-fixes.md:
- Around line 283-293: Update the verification step to require explicit mobile
viewport dimensions to test the proposed compact spacing in AuthForm.tsx and its
callers (TreatyVoteFlow.tsx and HumanityVGovernmentVerdictVote.tsx);
specifically capture before/after screenshots at these viewports: 360x800 (small
Android), 375x812 (iPhone X/11 Pro), 390x844 (iPhone 12/13), 412x915 (larger
Android), and 768x1024 (portrait tablet) and confirm the submit button remains
above the fold (also note if py-6 sm:py-8 is needed in TreatyVoteFlow.tsx or
mt-4 in HumanityVGovernmentVerdictVote.tsx); include pass/fail result per
viewport in the plan.
In @.claude/plans/task-impact-backfill.md:
- Around line 144-145: Update the task description to explicitly state that only
hierarchy-based inheritance should be removed: say "disable or remove
buildParentInheritedImpactFrame (hierarchy/tree parent inheritance) so tasks
without a direct current estimate render no selected frame", and clearly note
that buildDownstreamUnlockedImpactFrame (edge-based causal modeling using
edge.probabilityDeltaBase and edge.timeDeltaDaysBase) must be preserved and kept
separate; ensure the task text references both functions by name
(buildParentInheritedImpactFrame and buildDownstreamUnlockedImpactFrame) and
instructs verifying that any retained buildDownstreamUnlockedImpactFrame logic
only reflects explicit task edges, not parent hierarchy.
---
Nitpick comments:
In `@TODO.md`:
- Around line 299-317: The TODO entry describing share-templates.ts as the
canonical source and the email audit should be updated to reference the existing
audit plan file (.claude/plans/email-template-audit.md); edit the paragraph that
mentions the "Audit task" to add a brief cross-reference pointing to that plan
and note it covers migration steps for modules like TreatyReminderComposer,
monthly-chain-digest, post-vote-share, referral-first-conversion, and
task-comment-notification; mention that modules should use share-templates.ts
(filtered by recipientModes and rendered with renderTemplate) and defer to the
email-template-audit plan for the detailed migration checklist.
- Around line 79-216: Duplicate "Standardize 'apocalypse' framing" content in
the TODO should be consolidated: remove the detailed block that duplicates the
dedicated plan and replace it with a one-line pointer referencing the separate
apocalypse-framing plan (keep candidate labels A/B/C and surface lists only in
that plan), and ensure the TODO contains a short directive like "See
apocalypse-framing-standardization plan for full details" so future readers are
guided to the single source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2be0036b-2f21-4cfe-8ae8-3f0fec4413c9
📒 Files selected for processing (11)
.claude/codex-delegation.md.claude/hooks/codex-dispatch-blather.mjs.claude/hooks/enforce-codex-protocol.mjs.claude/plans/README.md.claude/plans/anonymized-db-preview.md.claude/plans/apocalypse-framing-standardization.md.claude/plans/email-template-audit.md.claude/plans/login-form-ux-fixes.md.claude/plans/task-impact-backfill.md.claude/settings.jsonTODO.md
✅ Files skipped from review due to trivial changes (1)
- .claude/plans/README.md
|
Re: claude[bot] code review flagging |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Drop unsafe widening cast in dashboard.server.ts; `findUnique` with `include` already returns all User scalars including `downstreamConversionCount`, so the cast was both unnecessary and a type-system lie (it claimed the field might be absent when in fact it is present and typed as `number`). - Export `DownstreamCacheClient` / `DownstreamCacheTransaction` so the test can declare its mock against the real interface instead of reaching for `as never`. The test's `$queryRaw` is now a typed generic method on the mock, no `any` and no `as never`. - Drop the `as unknown as DownstreamCacheClient` double-cast in the source by typing the local `db` variable as the interface directly; PrismaClient assigns to it structurally. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The "overdue dueAt" regression test compared `dueAt.getTime()` to `Date.now()`, which violates the testing rule against real wall-clock in assertions. The dueAt is a fixed 2024-12-31 anchor; use a fixed 2025-01-01 reference so the assertion can't drift if time freezes, travels backwards, or the runner clock skews. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
test |
Code reviewNo bugs found. Three CLAUDE.md compliance issues flagged. Issue 1 —
|
Code reviewThree CLAUDE.md compliance issues found in new files: 1. Hardcoded hex colors in
|
Code review1 issue found.
|
Code reviewOne CLAUDE.md style issue found. No bugs or security issues.
The Fix: add a comment mirroring // #ffffff / #000000: qrcode.react's bgColor/fgColor only accept
// plain strings — same rationale as @media print blocks in page.tsx.
<QRCodeSVG
bgColor="#ffffff"
fgColor="#000000"
...
/>See: optimitron/packages/web/src/app/poster/poster-client.tsx Lines 19 to 27 in 1d2dc8a |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Code reviewTwo CLAUDE.md violations found. Checked for bugs and CLAUDE.md compliance. Issue 1 — Magic number: use existing named constantFile:
// packages/web/src/lib/tasks/treaty-signer-network.ts (line 32)
const TREATY_DUE_AT = new Date("2024-12-31T00:00:00.000Z");CLAUDE.md rule: "Magic numbers — named constants citing the paper." The same raw literal already appears twice in this file (lines 72 and 273), which is exactly the duplication risk the rule targets. Fix: export Issue 2 — Touched line retains hardcoded color tokensFile: The PR edits the CLAUDE.md rule: "When touching public UI, remove neobrutalist styling instead of copying it forward." + "Never use: Hardcoded Fix: - ? "w-full border border-black bg-white p-4 text-black shadow-none"
+ ? "w-full border border-foreground bg-background p-4 text-foreground shadow-none" |
|
Triaged the latest claude-bot review (#81 (comment)). Both findings declined as out-of-scope / architecturally wrong: Issue 1 — magic date literal in Issue 2 — No code changes pushed. CI is clean (9 pass / 5 skip), all 29 inline review threads already resolved, PR ready for Mike's merge. |
Summary
Optimize Earthmanaged task was missingdueAt, sogetTaskDelayStatsreturnedcurrentDelayDays = 0and the/tasksrow showed $0 wasted / 0 deaths-from-delay even though the per-day rates were wired correctly.dueAt = 2024-12-31to matchEND_WAR_AND_DISEASE_TASK_IDso parent and child agree on "we should be done by now."optimize-earth-task-tree.test.tsas a data-shape regression guard.Bundled
share-templates.tsas the canonical source for all reminder copy;ICEWAD→IC2EWDrename plan; impact-inheritance design decision (don't inherit by default — inheritance creates double-counting when aggregating); column hiding on task tables when no row has a value; unify "Your Tasks" empty state with the task-row component.pr-comment-triageragent: post per-comment replies on bot threads instead of one summary, so it's obvious every bot comment was addressed when scrolling the PR.Test plan
pnpm --filter @optimitron/db test— 137/137 passing (12 files; new test file included)./tasksand confirm the Humanity's Tasks → Optimize Earth row shows non-zero "Wasted By Delay" and "Deaths From Delay".🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Bug Fixes
New Features
UI
Chores