Enforce firstname-lastname.md naming for people files#46
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08320adac9
ℹ️ 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".
skills/librarian/SKILL.md
Outdated
| - People files: `firstname-lastname.md` (always include last name when known; never | ||
| `firstname.md` alone) |
There was a problem hiding this comment.
Define fallback when a person's last name is unknown
This rule now forbids firstname.md outright, but the workflow still says to create/update people files for recurring people and the same document includes firstname-only examples (memory/people/julianna.md, people/thomas.md), so agents can no longer derive a valid filename when only a first name is known. In that common case, runs will either violate this rule or skip creating required files, leading to inconsistent file names and wiki-link targets; please keep a documented fallback for unknown/mononym names (or update all related instructions together).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 35db709. Added firstname-unknown.md as the documented fallback when last name is genuinely unknown, and updated the example log entry from people/thomas.md to people/thomas-owen.md. The intent was never to break the unknown-last-name case — just to prevent workflows from using firstname-only when the last name was actually known.
There was a problem hiding this comment.
👎 Incorrect — the merged code already defines the fallback on lines 218-219: 'if last name unknown, use firstname-unknown.md'. The firstname-only examples (julianna.md, thomas.md) you referenced were replaced with proper firstname-lastname.md examples before merge. PR #48 adds disambiguation guidance for the same-first-name collision case (flagged separately by cursor[bot]).
…nly) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codex P2: the 'never firstname.md alone' rule was incomplete — no documented fallback when last name is genuinely unknown. Add firstname-unknown.md placeholder pattern. Also fix the example log entry that still referenced people/thomas.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
101932f to
dfc1bf1
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| - People files: `firstname-lastname.md` (or `firstname.md` if unambiguous) | ||
| - All filenames: **kebab-case** (`alex-chen.md`, not `Alex Chen.md`) | ||
| - People files: `firstname-lastname.md`; if last name unknown, use | ||
| `firstname-unknown.md` as a placeholder (never bare `firstname.md`) |
There was a problem hiding this comment.
Naming collision risk for multiple same-first-name unknowns
Low Severity
The firstname-unknown.md fallback creates a collision risk when two or more people share the same first name and both have unknown last names — e.g., two "Johns" would both map to john-unknown.md. The old firstname.md "if unambiguous" rule had an implicit guard against this. The new convention doesn't document how to disambiguate multiple unknowns with the same first name.
There was a problem hiding this comment.
Valid catch. Fixed in PR #48 — added descriptor pattern for disambiguation: , .
When multiple people share a first name and last names are unknown, document the descriptor pattern (john-unknown-contractor.md) to avoid file collisions. Addresses cursor[bot] feedback on PR #46. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>


Summary
firstname.md"if unambiguous" loophole from the librarian skill's file conventionsfirstname-lastname.mdwhen last name is knownSplit from #45 for focused review.
Why
The old rule allowed
firstname.mdif unambiguous, but this caused workflows like FRM to create files likebobby.mdinstead ofbobby-battista.md. As the people directory grows, firstname-only files become increasingly ambiguous. Closing the loophole prevents this class of naming issues.Test plan
firstname-lastname.mdfiles going forward🤖 Generated with Claude Code