feat: rename generative slots -> generative stubs#801
Conversation
|
The PR description has been updated. Please fill out the template for your PR to be reviewed. |
d417e7d to
5ff79b5
Compare
There was a problem hiding this comment.
(testing out some things with this review. review was AI generated but human reviewed/approved)
Code Review
The rename is thorough and mechanically correct across ~40 files — nice work on the coverage. A few items to consider before merge:
1. No backward-compatibility shim for genslot import path
The module mellea.stdlib.components.genslot is deleted with no re-export. Multiple doc pages teach users from mellea.stdlib.components.genslot import PreconditionException — external code following current docs will get ModuleNotFoundError on upgrade.
Suggestion: Add a minimal genslot.py shim with a deprecation warning, or explicitly document this as a breaking change in the upcoming release's CHANGELOG (not by rewriting old entries).
2. docs/metrics/coverage-current.json not updated
Lines 1822, 1838, 1875 still reference mellea.stdlib.components.genslot. If this file is auto-generated and gets regenerated, no action needed — but if it's checked by CI it could break.
3. CHANGELOG history rewriting
Historical entries at lines 242/275 were changed from "genslot"/"gen slots" to "genstub"/"gen stubs". Standard practice is to leave historical entries untouched and add a rename note under the current version's section. I'd recommend reverting those edits and adding a new entry instead.
Minor
docs/docs/docs.json:448— Mintlify redirect still says/core-concept/generative-slots. This is arguably correct (preserves old bookmarks), but worth an explicit decision.- Notebook formatting — Two
.ipynbfiles had theirsourcearrays collapsed to single strings. Valid JSON but creates unnecessary diff noise.
Overall the rename itself is clean and consistent — template file + template_order are correctly in sync, all class/variable/error-message references are updated, and good call checking the tutorials repo too.
5ff79b5 to
f1da2ca
Compare
|
Makes sense!
I also added an |
|
@planetf1, can you please comment on:
I didn't see any code that references this and my understanding of the above statement is this is to make bookmarks keep working form the old version of the website. I think we should leave this as is? |
f1da2ca to
acc81ad
Compare
Yeah, I think so too (should've probably dropped that, was testing an automated PR review workflow, guess it could use a little tweaking still 😉 ) |
I think it may be too early to focus on keeping all our doc URLs stable.. I would stick to what we deem to be the main entry points (maybe just the docs themselves), otherwise we'll start making things complex to maintain. Most of the entries in docs.json are effectively 1:1 mappings for clarity - I'd mostly go for that approach unless we have a good reason not to. One thing to be careful of is any inbound links from the landing page (since that is a distinct repo). We could have a 'contract' where we keep that nominated list stable - but nothing formalized currently. That being said, the website doesn't currently point to generative-slots (or stubs) I'd therefore be inclined to update the source/destination to be in sync (which would break a bookmark) - but that's more driven by the lack of explicit policy to preserve links and keep things simple rather than any objection to the naming being used.... |
|
Sounds good. I will leave the bookmarks / links untouched for now. |
Misc PR
Type of PR
Description
We were requested to rename GenerativeSlots -> GenerativeStubs. I've gone through this repo and other mellea repos (tutorials, demos, contribs, website). The only other reference was in tutorials. I've opened a PR here.
Testing