Repository navigation
Refuse to delete a teaching file while its principle remains - #20
Conversation
principles require_teaching_coverage fails a pull request that drops the teaching file for a principle that is still in the index. Facet now refuses that delete, including the catalog entry that would go with it. A reading-only file stays valid, and its reading list can still change.
|
@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. |
There was a problem hiding this comment.
🟢 Approval recommended
The server and editor checks match the retention requirement, with regression coverage for blocked deletions and permitted cleanup.
0 open findings
What changed in this PR
Protects required teaching files in Facet while preserving cleanup of files whose principles are no longer listed.
Changes:
- Rejects deletion of teaching files for listed principles.
- Hides the editor’s Delete teaching action for those files.
- Adds regression coverage and documents the retention rule.
| File | Description |
|---|---|
| tests/lists.test.mjs | Covers blocked deletions, orphan cleanup, and reading-only files. |
| tests/analytics.test.mjs | Checks editor deletion controls and orphan cleanup. |
| tests/allow.test.mjs | Tests index-based deletion eligibility. |
| lib/guard.js | Enforces the deletion check during save review. |
| lib/allow.js | Adds the teaching deletion eligibility helper. |
| js/facet.js | Prevents deletion of required teaching files in the editor. |
| AGENTS.md | Documents teaching-file retention and reading-list editing. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cb3b2ae5a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kindelwww#296 pins principles at 961aa456. That revision requires a teaching file for every principle. Facet still accepted a save that deleted the file and cleared its catalog entry. The principles pull request then failed with
data/teaching/<company>/<slug>.json: missing.The guard now refuses that delete while the principle is still in the index. The editor no longer offers Delete teaching in that case. The same refusal covers a reading-only file (
id,slug, andblog). Reading links on that file can still be added, removed, and reordered. A file whose principle is no longer in the index can still be removed, and that save still has to clear the catalog entry.kindelwww#296 will repin facet and copy
lib/intoapi/editor-libafter this is on main.Tests:
node --teston the patch, allowlist, guard, reuse, lists, insert, essay link, and analytics suites (169 pass), plusnode scripts/check-essay-links.jsandnode scripts/check-companies.js.