Reader prerequisites: consume canonical Lync 0.4.3 presentation - #80
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdedaf809d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| {timeSeam ? ( | ||
| <span | ||
| className="story-time-seam" | ||
| title={`${new Date(previous!.createdAt!).toISOString()} → ${new Date(segment.createdAt!).toISOString()}`} |
There was a problem hiding this comment.
Validate timestamps before formatting the time seam
Reject or suppress out-of-range timestamps before calling toISOString(). A kept-conversation Markdown manifest can currently supply any numeric turn.createdAt because parseKeptConversationMarkdown validates only the top-level arrays; with two portable turns such as timestamps 0 and 9e99, the elapsed-time condition passes and this expression throws RangeError: Invalid time value, crashing the reader instead of rejecting the malformed import.
Useful? React with 👍 / 👎.
| const elapsedMs = | ||
| previous?.createdAt !== undefined && segment.createdAt !== undefined | ||
| ? segment.createdAt - previous.createdAt | ||
| : null; | ||
| const timeSeam = | ||
| showTurnBoundaries && elapsedMs !== null && elapsedMs >= STORY_TIME_SEAM_MS | ||
| ? elapsedTimeLabel(elapsedMs) |
There was a problem hiding this comment.
Limit elapsed markers to timestamps with matching semantics
Only calculate this marker when both adjacent values are known source-event times. On a reopened kept-context artifact, projectRawLyncFile assigns a carried local turn's createdAt from local.keepEvent.at, while its source parent retains the source event time; this code therefore labels the interval between the source event and a later curation action as if it were the elapsed time before the local turn, potentially showing a large but fabricated narrative discontinuity.
Useful? React with 👍 / 👎.
| directly here through `@deepfates/lync/presentation`. The exact local package is | ||
| from Lync commit `860aa549727bdec7fda2ba1571c0a6be1c787aa8` and is checksum-bound in | ||
| `vendor/LYNC-PROVENANCE.md`. |
There was a problem hiding this comment.
Record the actual commit for the vendored Lync package
Update this exact-package claim to 0a511b6f918acf87800bf433f34311ee47a9c48b. This commit replaces the archive with deepfates-lync-0.4.3-0a511b6.tgz, and vendor/LYNC-PROVENANCE.md attributes its verified checksum to 0a511b6, so naming 860aa549... here gives readers contradictory provenance for the package that actually supplies the presentation code.
Useful? React with 👍 / 👎.
Purpose
Land the four already-reviewed reader/corpus repairs that the documentation change depends on. This is a product prerequisite PR, separate from the documentation PR. It does not publish Textile or Lync.
Exact commit stack
2d87e28— consume canonical Lync presentation12cba6f— preserve habitat time and browser scale173cdbc— show resident null cognition honestlyfdedaf8— show held-focus placement attemptsThe replacement checksum-pinned
@deepfates/lync0.4.3 tarball is the complete candidate at Lync0a511b6, including the indexed-union shared helper and additive file-cursor/checkpoint subpaths, not a presenter-only overlay.Clean-checkout evidence
bun install --frozen-lockfile: passbun run verify: pass (228 tests, one deliberately skipped retained scale fixture; lint and production build pass)Known unrelated gate defects
origin/main; one storybook curation timing assertion also fails. The four-commit diff does not modify those mocks or the generation response contract.verify:corpus-loopcannot start its Lync source build because the current Lyncpnpm-workspace.yamlhas nopackagesfield. The packed dependency itself is covered above.Please review and land this dependency before retargeting documentation PR #79 to
main.