feat: add generate-slides-retro-simple skill - #28
Conversation
Nieuwe skill voor compacte sprint review presentaties georganiseerd per domein (instroom/uitval/tech/project/overig) met: - CEDA Board integratie via GraphQL (iteratie, status, issue types) - Functie-metric (20-100 regels) als proxy voor substantieve logica - Kanban board voor pitches (Todo/In Progress/Done/On Hold) - Afgeronde tasks slide voor losse taken - Terminal summary output Voorbeeld output: cedanl/clidev-presentaties@1fb594c Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Slide 6 title and references updated to "Iteratie Output" - Added per-column leeswijzer description - Use "geen output" instead of "geen activiteit" for inactive domains Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Genereert inderdaad slides. Alleen kreeg in html code op de slides te zien. Oorzaak:
Het probleem was dat ingesprongen HTML (4+ spaties) door Slidev's markdown-parser als tekst/code wordt behandeld in plaats van als HTML. De fix:
- Alle HTML-tags staan nu op kolom 0 (geen inspringing)
- Lege regels scheiden HTML-blokken correct
&is vervangen door& in HTML-context<ul>/<li>vervangen door<p>met• (betrouwbaarder in Slidev)
Controle oid opnemen in de skill?
|
Willen we ook nog iets met sprintdoel? |
|
|
student-signal hoort bij 'uitval'. Misschien kan skill ook automatisch runnen obv iteratie-schema? |
|
feedback:
|
Adds Marp presentation skill with Npuls theme (npuls.css, .marprc.yml, _template.md). Builds Marp decks in CEDA/Npuls house style with render → PDF visual overflow-check loop. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CorneeldH
left a comment
There was a problem hiding this comment.
Review: generate-slides-retro-simple
Leuke skill — de CEDA Board-integratie zit goed in elkaar en de "scope per pitch"-analyse is een echt idee, geen metriek om de metriek. Maar er zitten een paar dingen in die eerst opgelost moeten worden, en er is een probleem met de PR zelf.
0. Eerst: deze PR kan niet gemerged worden — hij moet dicht
De inhoud staat al op main, in een nieuwere vorm dan wat hier in de PR zit:
PR head (4a1a046) |
origin/main |
|
|---|---|---|
config.json |
42 repo's | 56 repo's |
| Slide 6 | "Toevoegingen" | "Iteratie Output" (hernoemd in a6e07d3) |
De drie commits (d12b1a7, a6e07d3, 7b75654) zitten met andere SHA's op main, dus de branch is ergens gerebased of gecherry-pickt en deze PR is blijven hangen. mergeable_state staat op dirty. Als je 'm alsnog merget draai je main terug naar de oudere repolijst en de oude slidenaam.
Voorstel: PR sluiten met een verwijzing naar de commit op main, en de punten hieronder als issues oppakken. Ze gelden onverkort tegen de huidige main — daar heb ik ze ook tegen getoetst.
0b. De PR bevat twee skills, niet één
Naast generate-slides-retro-simple zitten hier ook build-marp-deck/SKILL.md + drie assets in (npuls.css, .marprc.yml, _template.md), samen 565 regels. Dat is een losstaande skill over een ander presentatietool. Los reviewen, los mergen — nu is niet te zien welke review-opmerking bij welke skill hoort.
Blokkerend
1. De skill vraagt om een PAT in de chat, en heeft die niet nodig
Stap 1 laat de gebruiker een classic token met repo + read:org + project aanmaken en in het chatvenster plakken, gevolgd door export GITHUB_TOKEN="<geplakt-token>".
Dat is een org-breed, langlevend token met schrijfrechten dat daarmee in het sessietranscript belandt, in de shell history, en in wat er verder van die sessie bewaard wordt. Voor een skill die alleen leest.
En het is overbodig: elke andere call in deze skill gebruikt al gh api, dat authenticeert zelf via de bestaande gh-login. De enige call die curl + $GITHUB_TOKEN gebruikt is het ophalen van de repolijst — die kan gewoon gh api orgs/cedanl/repos zijn.
Vervang het hele tokenblok door:
gh auth status || gh auth loginen zet de repolijst-call om naar gh api. Scheelt ook nog eens 20 regels.
2. sha=main is hardcoded, maar niet elke repo heeft main
gh api "repos/cedanl/{repo}/commits?sha=main&since=..."Gecheckt tegen de org: selectie-evaluatietool staat op master. Die call geeft een 404 en de repo valt stil uit de retro — zonder foutmelding in het deck.
Extra: de leeswijzer die de skill zelf laat genereren op slide 6 zegt letterlijk "commits = op default branch". De implementatie doet dat dus niet. Haal default_branch uit de repos-call die je toch al doet en vul die in.
3. De categoriemapping klopt niet met de echte repolijst
Dit is de kern van de skill — "georganiseerd per domein" staat in de description — en dat werkt op dit moment niet. Ik heb de tabel uit stap 2 tegen de 66 huidige publieke cedanl-repo's gedraaid:
- 25 van de 66 repo's (38%) vallen door naar
overig. Daarmee isoverighet grootste domein, terwijl het in het deck een halve slide krijgt. Onder andere:wisselstroom,studentjourney-mbo,rio-onderwijsdata,onderwijsdata-chat,synthetische-onderwijsdata,duo-mbo-datafiles,mbo-bekostiging-bestanden,ceda-innovatiefunnel. UitnodigingsregelenUitnodigingsregel_datapreparatiematchen niet. Het patroon is*uitnodiging*(kleine letter), de repo begint met een hoofdletter, en globmatching is hoofdlettergevoelig. Dat is uitgerekend de vlaggenschip-repo van het uitval-domein die inoverigterechtkomt. Zelfde risico bijAssistentie(die matcht wél, want letterlijk zo gespeld) enAI-Alignment.docker_1chomatcht twee domeinen tegelijk:*1cho*(uitval) én de expliciete naam onder tech. Er is geen voorrangsregel, dus in welk domein hij landt hangt af van hoe het model de tabel toevallig afloopt.- Drie patronen matchen niets meer:
*uitval*,ceda-scoop,regiobijeenkomsten(de echte repo heetregiobijeenkomst-tool).
Wat ik zou doen: hoofdletterongevoelig matchen, een expliciete voorrangsregel opschrijven (exacte naam wint van glob, anders eerste match in tabelvolgorde), en de skill laten printen welke actieve repo's niet gemapt konden worden. Dan degradeert de lijst zichtbaar in plaats van stil.
4. De overlap met generate_slides_retro is niet opgelost — en onze eigen standaard eist dat wél
Beide descriptions beginnen met "Genereer een … Slidev sprint review presentatie voor CEDA". generate_slides_retro zegt "op basis van actuele GitHub data" — maar deze skill is óók GitHub-data-based, dus er staat niets in dat de een van de ander uitsluit. Geen van beide heeft een Gebruik wanneer …-triggerzin of een LET OP-clause, terwijl clidev en build-marp-deck die wel hebben.
skills-ontology/SKILL.md:158 en create-skill/references/description-schrijven.md zeggen hier allebei hetzelfde over:
voeg je een skill toe die overlapt met een bestaande, dan hoort de description van de bestaande skill in dezelfde PR mee te veranderen.
En description-schrijven.md noemt dit paar zelfs bij naam als bekend probleemgeval. Dus: twee descriptions aanpassen, in één wijziging. Bijvoorbeeld deze richting — deze krijgt "LET OP — voor de uitgebreide variant met velocity, backlog health en interactieve slides, gebruik generate_slides_retro", en die krijgt de spiegel ervan.
(Dit sluit ook aan op je eigen comment "onderscheid tussen planning en retro" — dat onderscheid hoort precies in die twee descriptions te landen.)
Graag oplossen, niet blokkerend
5. Frontmatter mist het CEDA-schema
Alleen name + description. validate-skill.py geeft OK, maar met de aantekening "nog niet gemigreerd naar het CEDA-schema" — de skill wordt dus als legacy geboren. Ontbreken: allowed-tools (hier Read Write Edit Bash Glob Grep), compatibility (deze skill heeft gh, node en npm echt nodig, en dat staat nu nergens), en het hele metadata:-blok.
Kanttekening over de volgorde: create-skill v2 en de validator staan zelf nog in #68 en zijn nog niet op main. Dus dit is redelijkerwijs een "zodra #68 landt"-punt, geen verwijt achteraf. Maar dan wel meteen meenemen.
6. theme: ./theme is achterhaald
De skill schrijft theme: ./theme voor op twee plekken (stap 0 en stap 5) en noemt <clidev-pad>/theme/ als huisstijllocatie. clidev/SKILL.md markeert precies dat als verouderd:
Verouderd: oudere presentaties gebruiken
theme: ./theme… Dat is vervangen doortheme: default+style.css. Volg voor nieuwe presentaties altijd het nieuwe systeem.
Elk deck dat deze skill vanaf nu genereert komt dus op het oude systeem terecht.
7. Stap 0 dupliceert clidev in plaats van 'm aan te roepen
De 14 regels clone/find/npm install zijn vrijwel letterlijk overgenomen uit clidev, met twee verschillen: hier wordt ~/clidev-presentaties hardcoded (clidev vraagt waar), en de check op de installatie van de upstream slidev-skill ontbreekt. Punt 6 hierboven is precies wat er gebeurt als je zo'n stap kopieert: de kopie loopt achter op het origineel.
De rest van de repo doet dit al anders:
build-marp-deck/SKILL.md:12— "Wil iemand Slidev? → gebruikclidev."npuls-huisstijl/README.md:59— "Voor Slidev-presentaties is er al declidevskill … Die skill gebruikt de brand-assets en tokens uit deze skill als bron — niet duplicaten."
Vervang stap 0 door een verwijzing naar clidev voor de projectsetup. Dan verdwijnt punt 6 vanzelf, en ook:
8. find ~ -type f -name "_template.md" scant de hele home directory
Bij elke run, voor iedereen. Op een volle laptop is dat tientallen seconden tot minuten. Overgeërfd van clidev — nog een reden om te delegeren in plaats van te kopiëren.
9. config.json is een cache die niemand nodig heeft
281 regels gecachte repolijst, in git, die de skill volgens zijn eigen documentatie "bijwerkt bij elke run". Gevolgen:
- Hij is nu al verouderd: 56 entries tegenover 66 live publieke repo's.
- Elke run maakt de skills-repo dirty met een diff die niemand wil reviewen.
- De domeinmapping staat in
SKILL.mden de repolijst inconfig.json— twee plekken die uit elkaar lopen, wat je terugziet in punt 3. - De gecachte velden (
pushed_at,description) worden élke run opnieuw opgehaald, dus de cache levert niets op.
Ik zou 'm gewoon weglaten. Stap 1 haalt de lijst toch live op.
10. De functiemetriek meet iets anders dan de kop belooft
De kop zegt "alleen TOEGEVOEGDE code". Vier dingen die dat ondermijnen:
compare/{FIRST_SHA}...mainheeft geen bovengrens. De commit-query filtert netjes opsinceénuntil, maar de compare loopt door tot de huidigemain. Draai je de retro een week na sprint-einde, dan tellen de functies van daarna gewoon mee. Bound 'm op de laatste commit vóórSPRINT_END.- De compare-endpoint kapt af op 300 bestanden en laat
patchweg bij grote diffs. Stille onderschatting, staat nergens vermeld. - "tel regels tot de volgende
def" werkt niet betrouwbaar op een diff-patch. Patches zijn hunks met contextregels; een functie die over twee hunks verdeeld is telt dubbel of half, en een gewijzigde functie verschijnt als toegevoegde regels. - Renames en verplaatsingen tellen als volledig nieuwe functies.
Voor een proxy is dat te leven mee, maar het deck presenteert deze getallen per domein als "substantieve logica toegevoegd". Ik zou het "functies aangeraakt" noemen en de kanttekeningen in de leeswijzer zetten — die staat er toch al.
11. Paginering is beschreven maar niet uitgevoerd
De GraphQL-query heeft items(first: 100, after: $cursor) en de tekst zegt "pagineer met after: $endCursor", maar het getoonde commando geeft nooit een -F cursor= mee. Het CEDA Board zit met 100+ items zomaar over die grens. Zelfde patroon bij de repolijst: per_page=100 zonder paginering — met 66 repo's gaat dat nu net goed.
Klein
- De domeinkleuren zijn GitHub-labelkleuren, geen Npuls-huisstijl (
#1D76DB,#D93F0B,#5319E7,#0E8A16zijn de GitHub-defaults). Voor een CEDA-deck zou ik ze uitnpuls-huisstijlhalen, dat is daar de bron van waarheid voor. type=publicbetekent dat werk in interne/private repo's onzichtbaar is in de retro. Bewuste keuze? Zet 'm dan in de leeswijzer, anders leest een leeg domein als "er is niks gedaan".- Kanban-volgorde op slide 5: Todo staat vooraan in rood (
#C62828), dezelfde kleur als de Bug-badge. Rood voor "nog niet begonnen" leest als alarm; grijs of blauw past beter bij de "geen waardeoordeel"-lijn die je verderop expliciet aanhoudt. - Sectie
## Bestandenheet in het schema## Gebundelde bestanden, met per bestand de conditie waaronder het gelezen moet worden. Valt weg alsconfig.jsonverdwijnt (punt 9). - De PR-omschrijving is verouderd: slide 6 heet daar nog "Toevoegingen".
Wat ik hier goed aan vind
Even expliciet, want de lijst hierboven is lang en dit zijn de dingen die ik zou overnemen:
- De GraphQL klopt precies op de plekken waar het pijn doet. De aliassen
status:eniteratie:mét de uitleg waarom ze nodig zijn,ProjectV2IterationFieldom de sprintperiode op te halen in plaats van data hardcoden, de terugval op 2 weken als het veld ontbreekt, en de opmerking dat het veld "Iteratie" heet en niet "Iteration". Dat is exact het soort detail dat je één keer uitzoekt en daarna nooit meer wilt uitzoeken — daar is een skill voor. - Scope per pitch via tasklist-counting. Een board laat alleen Done/niet-Done zien; dit laat zien dat een pitch op 1/4 sub-items staat. Dat is de meest bruikbare slide van het hele deck.
- De toon rond de metriek. "Geen waardeoordeel, alleen context", en
"geen output"in plaats van"geen activiteit"met de expliciete motivatie erbij. Iemand heeft nagedacht over hoe deze cijfers in een teamvergadering gelezen worden. Dat is precies waarom punt 10 me wel dwarszit: de zorgvuldigheid in de presentatie verdient een even zorgvuldige meting eronder. - De waarschuwing over
<script>/<style>in Slidev-markdown — echte valkuil, goed dat 'ie er staat.
Je eigen openstaande comments
- "student-signal hoort bij uitval" — staat inmiddels in de mapping op
main. ✅ - "Willen we ook nog iets met sprintdoel?" / "assignee naast onderwerp" / "grotere letter" — nog niet verwerkt, zie ik nergens terug in de huidige
main-versie. - "onderscheid tussen planning en retro" — dat is punt 4 hierboven; dat onderscheid hoort in de twee descriptions.
- "Misschien kan skill ook automatisch runnen obv iteratie-schema?" — kan met
ceda-activation: scheduledin het nieuwe frontmatter-schema, dus dat sluit mooi aan op punt 5.
Samengevat: PR sluiten (de inhoud staat al nieuwer op main), build-marp-deck apart oppakken, en van de inhoudelijke punten zijn 1 (token in de chat), 2 (sha=main) en 3 (mapping) de dingen die nu daadwerkelijk verkeerde of onvolledige decks opleveren.
|
Aanvulling op mijn review hierboven — ik had de review van @Muhammet369 van 8 april gemist toen ik 'm schreef, en zijn bevinding staat nog steeds open. Zijn punt (ingesprongen HTML wordt door Slidev's markdown-parser als tekst gerenderd in plaats van als HTML, plus Sterker nog: de snippets in Op zijn vraag "controle oid opnemen in de skill?" — ja, en ik zou het tweeledig doen:
Dat is goedkoper en betrouwbaarder dan een controle achteraf. Wil je er tóch een check bij, dan is de render-en-visueel-controleren-loop uit Daarmee komt het totaal op vier dingen die daadwerkelijk kapotte of onvolledige decks opleveren: token in de chat, |
|
laatste comment is van agent. |
Summary
260408_sprint_review_simple.mdSlide structuur
Test plan
/generate-slides-retro-simplein een nieuwe sessie.mdopent metnpx slidev🤖 Generated with Claude Code