feat: Use dinosaur names for session display IDs - #110
Conversation
Replace generic animal names with ~50 dinosaur names for more memorable and fun session identifiers. Adjectives unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
Summary
Swaps animal names for dinosaur-themed names in session display IDs. Code change is clean and tests are updated, but guide.md and CLAUDE.md still reference the old "brave-tiger" examples and "Docker-style" terminology.
Verdict
REQUEST_CHANGES - Documentation references to old animal-based names need updating for consistency.
Automated review by Claude Code
| "ptero", | ||
| "plesio", | ||
| "spino", | ||
| "carno", |
There was a problem hiding this comment.
[Suggestion] Some of these abbreviated names are pretty opaque — citipa, lambe, nodo, acro may not be immediately recognizable as dinosaurs. Consider using slightly longer forms where the short form isn't well-known (e.g., citipati, lambeosaur, nodosaur). Not blocking, just a readability thought.
| "ankylo", | ||
| "diplo", | ||
| "allo", | ||
| "ptero", |
There was a problem hiding this comment.
[Important] guide.md still uses "brave-tiger" as the example display ID (lines 34 and 37), and CLAUDE.md references "brave-tiger" (line 146) and calls this "Docker-style display_id generation" (line 83). These should be updated to reflect the new dinosaur theme (e.g., "brave-trex") for consistency. Per CLAUDE.md: "When modifying API: Update CLI help, MCP docstrings, and guide.md together."
There was a problem hiding this comment.
Code Review
Summary
Clean rename from animal-based to dinosaur-themed session display IDs. The code change in session_ids.py and its tests are correct and consistent. However, the previous review's [Important] feedback about updating documentation and other source files has not been addressed — numerous references to "brave-tiger" and "Docker-style" remain across the codebase.
Previously Addressed (Filtered)
None — no "Feedback Addressed" comments found. The prior review's Important and Suggestion items appear unresolved.
Verdict
REQUEST_CHANGES - Documentation and source references to old animal-based naming still need updating (previously flagged, still unresolved).
Automated review by Claude Code
| "ankylo", | ||
| "diplo", | ||
| "allo", | ||
| "ptero", |
There was a problem hiding this comment.
[Important] The previous review flagged this and it remains unaddressed: many files still reference "brave-tiger" and "Docker-style" terminology. Files that need updating:
CLAUDE.md(lines 83, 146) — references "Docker-style" and "brave-tiger"src/agent_event_bus/guide.md(lines 34, 37) — example display IDssrc/agent_event_bus/middleware.py(lines 30, 46, 78, 80, 95, 141) — comments and docstringssrc/agent_event_bus/storage.py(lines 49, 55, 147) — commentsdocs/EXAMPLE_COMMS.md(line 45) — "Docker-style codename"- Various test files (
test_middleware.py,test_storage.py) — test data using "brave-tiger"
Per CLAUDE.md: "When modifying API: Update CLI help, MCP docstrings, and guide.md together." At minimum, update guide.md, CLAUDE.md, and source-code docstrings/comments to reflect the dinosaur theme. Test data strings like "brave-tiger" are less critical since they're just arbitrary test values, but the documentation and comments should be consistent.
| "archeo", | ||
| "coelo", | ||
| "micro", | ||
| "proto", |
There was a problem hiding this comment.
[Suggestion] Some abbreviated dinosaur names are quite opaque — e.g., citipa, lambe, nodo, acro, bary, compso may not be immediately recognizable as dinosaurs to most people. Consider using slightly longer forms where the abbreviation isn't well-known (e.g., citipati, nodosaur, lambeosaur). Not blocking — just a readability/fun-factor thought (this was also noted in the prior review).
The word "iguana" is a modern reptile, not a dinosaur. Use "iguano" (short for Iguanodon) to match the abbreviated naming convention used by the other entries (stego, bronto, diplo, etc.). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
Summary
Clean swap from animal names to dinosaur-themed names in session display IDs. Code and tests are internally consistent. However, the documentation and comment updates flagged in two prior reviews remain unaddressed — guide.md, CLAUDE.md, middleware.py, storage.py, and docs/EXAMPLE_COMMS.md still reference "brave-tiger" and "Docker-style".
Previously Addressed (Filtered)
No "Feedback Addressed" comments found. Prior review items appear unresolved.
Verdict
REQUEST_CHANGES - Documentation and source-code comments still reference old animal-based naming (third review flagging this).
Automated review by Claude Code
| "ankylo", | ||
| "diplo", | ||
| "allo", | ||
| "ptero", |
There was a problem hiding this comment.
[Important] This is the third review flagging this: documentation and source comments across the repo still reference "brave-tiger" and "Docker-style" terminology. The PR's own CLAUDE.md says "When modifying API: Update CLI help, MCP docstrings, and guide.md together."
Files that need updating (at minimum the non-test files):
CLAUDE.md(lines 83, 146) — "Docker-style display_id generation" and "brave-tiger"src/agent_event_bus/guide.md(lines 34, 37) — example display IDssrc/agent_event_bus/middleware.py(lines 30, 46, 78, 80, 95, 141) — comments/docstrings referencing "brave-tiger" and "Docker-style"src/agent_event_bus/storage.py(lines 49, 55, 147) — comments referencing "brave-tiger"docs/EXAMPLE_COMMS.md(line 45) — "Docker-style codename"
Test files (test_middleware.py, test_storage.py) use "brave-tiger" as arbitrary test data — less critical, but worth updating for consistency.
| "archeo", | ||
| "coelo", | ||
| "micro", | ||
| "proto", |
There was a problem hiding this comment.
[Suggestion] Repeated from prior reviews: some abbreviated names are quite opaque — citipa, lambe, nodo, acro, bary, compso won't be recognizable as dinosaurs to most people. Consider slightly longer forms for the least obvious ones (e.g., citipati, nodosaur, lambeosaur). Not blocking — just a readability/fun-factor thought.
- Update all references from "brave-tiger" to "brave-trex" and "Docker-style" to dinosaur-themed in guide.md, CLAUDE.md, middleware.py, storage.py, and EXAMPLE_COMMS.md - Lengthen opaque dinosaur abbreviations for readability: compso→compsy, bary→baryon, acro→acrocanth, citipa→citipati, lambe→lambeo, nodo→nodosaur Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Feedback AddressedImplemented
|
There was a problem hiding this comment.
Code Review
Summary
Clean swap from animal names to dinosaur-themed session display IDs. All previously flagged documentation issues have been addressed — guide.md, CLAUDE.md, middleware.py, storage.py, and EXAMPLE_COMMS.md now consistently use dinosaur terminology. Abbreviated names were lengthened for clarity and iguana was replaced.
Previously Addressed (Filtered)
- [Important] Documentation references to "brave-tiger" and "Docker-style" — confirmed updated across all flagged files
- [Suggestion] Opaque dinosaur abbreviations — lengthened (compso→compsy, bary→baryon, etc.)
- [Local review] iguana→iguano — confirmed
Suggestions (not in diff — cannot inline)
tests/test_middleware.pystill contains ~20 references to"brave-tiger"and one"Docker-style"docstring (line 336). These work correctly since_is_human_readable_idvalidates format not word membership, but updating them to use dinosaur names (e.g.,"brave-trex") would keep the test suite consistent with the theme.tests/test_storage.py:658,681similarly references"brave-tiger"in migration test fixtures.
Verdict
APPROVE - Code is correct, tests pass, and all previously flagged issues have been resolved. Remaining old references in test files are cosmetic.
Automated review by Claude Code
Summary
brave-trexinstead ofbrave-tigerTest plan
test_session_ids.pytests passmake checkpasses (format + lint + test)🤖 Generated with Claude Code