Repository navigation
Let Facet save reading links on teaching records that have no prose - #19
Conversation
A teaching file with only id, slug, and blog is Further reading. structuralShape still required the prose counts, so adding, removing, or moving a reading link failed. Skip those counts when no prose field is present, and keep the reading list checks.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
🟢 Approval recommended
The narrow validation change preserves existing safeguards and has focused regression coverage.
0 open findings
What changed in this PR
Allows Facet to save reading-list changes on teaching records without prose, supporting the updated principles data.
Changes:
- Skip prose requirements only for records containing
id,slug, andblog. - Preserve reading-link validation and full teaching requirements for other records.
- Add regression coverage for list edits and invalid content.
| File | Description |
|---|---|
| tests/lists.test.mjs | Tests reading-only edits and validation safeguards. |
| lib/rules.js | Allows reading-only records through structural validation. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
kindelwww#296 pins principles at 961aa456. That revision adds Further reading for every company set. The new teaching files hold only
id,slug, andblog.Adding, removing, or moving a reading link runs
plan.js, which callsstructuralShape. That check still required full teaching prose, so a valid list was rejected with errors such as "Why must stay between 3 and 6 paragraphs." I reproduced it with the Armown-itrecord from that principles commit.A teaching file whose keys are only
id,slug, andblognow skips the prose counts. The reading list is still checked: it cannot be empty, each link still needs a title, an https URL, and a note, and a{lp:slug}token in a note still has to be listed onrelated. A file that includes any other field still has to carry the full teaching shape.kindelwww#296 needs this on facet main before it can repin facet and copy
lib/intoapi/editor-lib. I am not reverting the principles pin.Tests:
node --teston the patch, allowlist, guard, reuse, lists, insert, essay link, and analytics suites (167 pass), plusnode scripts/check-essay-links.jsandnode scripts/check-companies.js.