[#977] Fix stories incorrectly showing as Active#979
Conversation
…dline flag Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The getStoryStatus() fix itself looks aligned with the issue, but the PR is not merge-ready because CI is failing immediately on an out-of-sync lockfile. Both lint-and-typecheck and e2e stop at npm ci.
Findings
- [medium]
package-lock.jsonis not in sync withpackage.json, so required checks fail before any tests run. GitHub Actions reportsnpm cifailure withMissing: utf-8-validate@5.0.10 from lock file, which means this PR currently cannot pass the project's standard verification gate.- File:
package-lock.json - Suggestion: Re-sync the lockfile against the current dependency graph and push the updated
package-lock.jsonsonpm cisucceeds again.
- File:
Decision
Requesting changes because the PR's checks are red due to the lockfile mismatch, even though the code change itself appears directionally correct.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The follow-up did not resolve the blocking CI problem. Both required jobs still fail at npm ci with the same lockfile error, so the PR remains not merge-ready.
Findings
- [medium]
package-lock.jsonis still out of sync after the attempted regeneration. GitHub Actions is still failing withMissing: utf-8-validate@5.0.10 from lock filein bothlint-and-typecheckande2e, so the verification gate remains red.- File:
package-lock.json - Suggestion: Regenerate the lockfile in a way that preserves the missing dependency entry expected by
npm ci, then re-run checks before re-requesting review.
- File:
Decision
Keeping this in request-changes because the lockfile issue is still unresolved and required CI is still failing before tests run.
…all) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The status-logic fix in lib/story-status.ts addresses the root cause described in issue #977 by treating any story with an old last_plot_time as deadline-governed, regardless of the unreliable historical has_deadline flag. The blocking lockfile problem also appears resolved: CI is no longer failing immediately on npm ci, and the required checks are now running normally.
Findings
- No blocking findings.
Decision
Approving because the functional fix is correct and the previous CI blocker from the out-of-sync lockfile has been cleared. Checks were still pending at review time.
Summary
getStoryStatus()only checked deadline expiry whenhas_deadline = true, but older stories indexed before the flag existed havehas_deadline = falsedespite having the 7-day deadline mechanismlast_plot_time, regardless ofhas_deadlineflaghas_deadlineis an unreliable DB fieldFixes #977
Test plan
has_deadline = falsebut oldlast_plot_timeshow "Expired"last_plot_time) still show "Active"🤖 Generated with Claude Code