Identify decision records by their GitHub tracking number, not a global counter - #114
Merged
Merged
Conversation
Replace the global DEC-NNNN counter with the tracking-number scheme adopted in exelearning/exelearning#2232: a decision is identified by the GitHub number of the change that produced it -- the issue when there is one, otherwise the pull request -- plus a two-digit sequence scoped to that number (DEC-<tracking-number>-<NN>-<decision-slug>.md). GitHub allocates issue and pull-request numbers from a single repository-wide sequence, so two branches can only collide when they share a tracking number. The retired counter could not offer that: this repository has already had to renumber records twice (DEC-0043 -> DEC-0045 in PR #35, and DEC-0065..0067 -> DEC-0069..0071 on the open secure-iframe branch). This commit only lands the policy: the decisiones README, the ADR template, the frontmatter schema and the research meta-docs. The records themselves are migrated next. The prefix stays DEC-, the prose stays Spanish and the frontmatter keeps its Spanish field names. Only `tracking_issue` and `legacy_id` are English, because they are the fields shared with the other eXeLearning repositories that follow this convention.
Rename every decision record whose GitHub tracking number could be established from the repository's own history, and rewrite the cross-references between records. The tracking number was resolved, in order of authority, from an explicit issue reference in the record itself, the `(#N)` subject of the squash commit that added the file (`git log --diff-filter=A --follow`), and the pull request title when the file landed as a direct push. Twelve records belong to umbrella issue #13 and one to issue #29; the other thirty-three take the number of the pull request that delivered them. Each migrated record gains `tracking_issue` and `legacy_id`, and every record -- migrated or not -- gains the mandatory `# <id>: <titulo>` H1, which the corpus had never carried. Slugs are preserved verbatim: the decision each one names is unchanged, and keeping them makes the id substitution also fix every filename reference. All Spanish prose, `agentes`, `fuentes`, `relacionados` and `herramienta_ia` provenance are untouched. Three one-sided supersession relationships are completed, as the policy now rejects them: - DEC-13-04 (ex DEC-0025) <-> DEC-13-05 (ex DEC-0026) - DEC-19-02 (ex DEC-0035) <-> DEC-25-01 (ex DEC-0038) - DEC-13-08 (ex DEC-0030) <-> DEC-111-01 (ex DEC-0068) The first two counterparts already declared `supersede`. For the third, the superseding record's own body states "Supersede a DEC-0030" and lists it under `relacionados`, so the relationship is documented, not inferred. Eighteen records keep the retired numbering: DEC-0001..DEC-0015, DEC-0019, DEC-0036 and DEC-0063 were pushed straight to `main` during the bootstrap period, reference no issue and no pull request, and have no verifiable tracking number. They are not renamed and no number is invented for them. Renames use git mv, so `git log --follow` still resolves the full history.
Rewrite the 2,038 references to the 46 migrated records across the repository:
PHP source comments, JavaScript, tests and Behat features, the docs/ suite, the
release scripts, the CI workflow step names, blueprint.json, AGENTS.md and the
research corpus (status.yaml, backlog, journal, analysis and source notes).
Because every slug is preserved, a reference to the record's filename
(`DEC-0030-version-sentinela-en-main.md`) is fixed by the same substitution as
the bare identifier.
Notes on the non-mechanical parts:
- Abbreviated group references (`DEC-0022/0037`) were first expanded to full
identifiers, since the shorthand cannot survive a numbering change.
- Range references (`DEC-0051..0054`) were rewritten by hand: a contiguous
range has no meaning once identifiers are tracking numbers.
- amd/build/fullscreen.min.js and its source map carry the comment verbatim
from amd/src/fullscreen.js. They are updated in step, which is byte-for-byte
what grunt regenerates: the identifier lives in a comment, so no mapping
segment moves.
- The AGENTS.md decision table is a curated editorial summary, not an index;
it keeps its commentary and now says so, and points at the generated index
as authoritative.
- The 2026-06-17 journal entry that records the DEC-0059/DEC-0063 collision
keeps its retired identifiers: it is the historical evidence for retiring
the global counter.
References to the eighteen records that keep the retired numbering are
unchanged, because those identifiers still resolve.
…index The map records every retired identifier, its current one, the tracking number and how that number was established, so a reference in an old PR, commit message or notebook can still be resolved. It also states plainly what was not migrated: the eighteen bootstrap records that have no verifiable tracking number, the evidence for that (their adding commits), and the options for closing the gap. No number was invented and no issue was opened to obtain one. build_indexes.py now sorts the decision index by tracking number and local sequence. Filename order would put DEC-106-01 before DEC-11-01 and DEC-4-01 after DEC-37-01, which is unreadable for an index whose whole job is to be scanned by a human.
Add research/tools/check_decisions.py, a stdlib-only Python 3 validator that
matches the existing research/tools/ toolchain. The reference implementation in
exelearning/exelearning is Bun + TypeScript; this repository has no Node/Bun
runtime of its own (bun is only fetched to build the embedded editor) and its
record corpus already ships two Python tools, so porting is the right call
rather than adding a runtime for one script.
It validates:
- the filename grammar DEC-<tracking-number>-<NN>-<decision-slug>.md, and
rejects any NEW record on the retired numbering. The eighteen bootstrap
records are a frozen allowlist, so the debt cannot grow;
- frontmatter: id matches the filename, tracking_issue matches the number,
valid status and date, agentes, and herramienta_ia provenance;
- the mandatory `# <id>: <titulo>` H1, and the absence of an `## Estado`
section duplicating the frontmatter;
- duplicate ids and duplicate local sequences;
- the supersession graph in both directions, including the superseded
record's status;
- design-document directories under research/decisiones/cambios/, when they
exist;
- retired identifiers anywhere in the tree, outside a documented allowlist.
Bootstrap identifiers are deliberately not flagged: those records still
exist under that name, so a reference to them resolves.
`list` prints the index from the frontmatter -- the index is derived, never
hand-maintained.
Wired into make (architecture-records / architecture-check) and into the CI job
release-workflow-check, which is the existing home for static guards: no matrix,
no Moodle bootstrap, and python3 is already present on the runner.
42 unit tests cover the checker, including three that assert the real corpus
satisfies its own rules.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #114 +/- ##
=========================================
Coverage 92.31% 92.31%
Complexity 824 824
=========================================
Files 51 51
Lines 3734 3734
=========================================
Hits 3447 3447
Misses 287 287
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…NNN ids Replaces `check_decisions.py` with the shared `architecture_records.py`, byte-identical to the copy in the other three repositories. Only the paths and the `DEC` prefix are configured; every rule is the same everywhere. The 18 bootstrap records that had no verifiable tracking number are now `DEC-0-01` … `DEC-0-18`. `0` is a sentinel for records that predate tracking: GitHub numbers issues and pull requests from 1, so it can never collide with a real one, and unlike `1` it does not claim a pull request that exists and is about something else. `check` no longer reports records on the retired numbering — there are none. Frontmatter keys are translated to English so the schema matches the other repositories exactly (`titulo` -> `title`, `estado` -> `status`, `relacionados` -> `related.adrs`, `herramienta_ia` -> `ai_assistance`, …), and status values with them. **The prose stays in Spanish**: this change is about the schema, not the language. `relacionados` mixed record ids with references to other research artifacts (AN-, RIE-, REPO-). Record ids became `related.adrs`, which the validator now checks; the rest moved to `see_also`, which it leaves alone. make architecture-check OK — 64 records (18 from repository bootstrap) unittest OK — 48 tests
…on port The validator is now one TypeScript file, byte-identical in every repository. It runs under Bun in core, where `bun test` covers it, and under the Node that ships on the CI image in the plugins, which need no setup step and no dependency. This replaces the Python port from the previous commit. Python was chosen when the alternative looked like installing Bun in four PHP pipelines. It is not: `setup-bun` appears only in release workflows that never run on a pull request, but Node is preinstalled on the runner image, and one source file runs under both runtimes once it avoids Bun-specific APIs. That keeps the tests where the scaffolding already is. Core keeps its 47 `bun test` cases with `expect()`; the plugins run the file itself on every pull request, which is what would surface a Node-specific regression. Repository-specific values live in `architecture-records.json` — paths and the record prefix only. Every rule is identical everywhere. bun run scripts/architecture-records.mts check OK node scripts/architecture-records.mts check OK (same output)
…sing Making `deciders` optional was the wrong reconciliation. It weakened the rule in every repository to accommodate five records that simply had not recorded who decided — a gap to fill, not a rule to relax. The field is required again, and the five records in wp-exelearning and omeka-s-exelearning now carry `@erseco` and `claude-code`, matching the pattern moodle-mod_exelearning already used for the same work. No model had to be inferred: `ai_assistance` was already present and consistent with each record's date (claude-fable-5 in July, claude-opus-5 in August). wp, omeka, nextcloud, moodle, core architecture-check OK core bun test 47 pass
Four files still named `check_decisions.py` and its test module, both removed when the validator became a shared TypeScript file copied from core. Also records that the copy here must not be edited locally. Refs exelearning/exelearning#2232
Five documents still pointed at `scripts/architecture-records.ts`, which no longer exists: the file became `.mts` so the same source runs under Bun in core and under Node in the plugin repositories. The script's own usage header said `bun run` unconditionally, which is wrong in the four repositories that carry a copy and run it with Node. It now states both, and why the file must avoid runtime-specific APIs. core architecture-check OK, bun test 47 pass, lint OK all 5 byte-identical copies, architecture-check OK
erseco
added a commit
that referenced
this pull request
Aug 5, 2026
main adopted tracking-number decision identifiers (#114) while this branch was open, so the merge collides on every file that names a record. Seven conflicts, resolved as follows. view.php, comment block above $emitsxapi: kept this branch's longer comment, which documents the channel choice for BOTH iframe modes; main's shorter version only knows the same-origin one. view.php, xapi_config() argument list: kept this branch's $hostorigin. The two sides close different calls. Main closes scorm_config(...) with its $emitsxapi argument and then adds the SCORM tracker boot and an xAPI listener block; this branch already carries all of that, restructured and moved earlier, and its own xAPI listener additionally handles opaque-origin mode by trusting event.source instead of event.origin. Verified after resolving: scorm_config() is still called with $emitsxapi, and the file has exactly one xAPI listener block. settings.php: kept this branch's cspprofile and embedmode settings — they are the feature — on top of main's editor-toggle comment. DEC-36-01: main renamed the file, this branch edited it. Applied this branch's superseded-in-part note and its extra related entry on top of main's renamed file and new frontmatter. research/status.yaml: RIE-001 keeps main's dated acceptance note and takes this branch's estado (mitigado, since this branch is the mitigation); its relacionados list is the union of both sides. docs/xapi-qa-checklist.md, AN-008: kept this branch's text, which is a superset of main's. research/docs/indices/adrs.yaml is generated; regenerated in a later commit.
erseco
added a commit
that referenced
this pull request
Aug 5, 2026
main retired the global DEC counter in #114: a record is now named after the GitHub number of the change that motivates it — the issue when there is one, otherwise the pull request. Issues are disabled here, so it is the PR: 80. The seven records this branch adds become DEC-80-01 … DEC-80-07, renamed with git mv so their history follows. Their frontmatter keys move to English to match what main now validates (the prose stays in Spanish), each gains tracking_issue and legacy_id, and the flat `relacionados` list splits into `related.adrs` for records and `see_also` for the other artefacts. The pre-migration corpus had no H1; each record gains the required `# <id>: <title>` heading. References are remapped repo-wide from the tree itself — every migrated record carries its legacy_id, so the retired-to-current mapping is derived, not typed. mapa-migracion-ids.md gains the seven rows and its "reserved in open branches" note becomes the record of what those identifiers ended up as. Three references to eXeLearning core ADRs were remapped the same way, reading the new identifiers out of core's own frontmatter on feature/preview-trust-boundary: ADR-0017 -> ADR-2199-08, ADR-0018 -> ADR-2199-09, ADR-0021 -> ADR-2199-12. That also makes the vendored js/exe_external_media/verify.mjs byte-identical to the copy core publishes again, which is the point of vendoring it. The two "ADR-0026/0027" mentions were never local identifiers: they name Procomún's own decision documents, in a repository that keeps its own numbering. They now cite those documents by path, so they no longer read as retired eXe identifiers.
erseco
added a commit
that referenced
this pull request
Aug 5, 2026
#114 moved the ADR frontmatter keys to English but left build_indexes.py reading `titulo`/`estado`/`fecha`, so the generator and the committed index no longer agreed: running it on main rewrote all 64 ADR entries to `titulo: '<filename>', estado: None, fecha: None`. The index survived only because nobody re-ran it. index_md() now reads the English key first and falls back to the Spanish one, so the same generator serves the migrated ADRs and the notes and sources that still use Spanish frontmatter. Regenerating with that fix changes adrs.yaml in exactly two ways: the seven DEC-80-* records appear, and 60 entries pick up the `estado` their frontmatter actually carries (Aceptada -> Accepted, Propuesta -> Proposed). No entry loses a title, a state or a date, and the other seven indexes come out unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Decision records in this repository were numbered from a global counter (
DEC-0001…DEC-0068). A global counter has one authority — "the highest number currently inmain" — and that authority does not exist on a branch. Two branches that both add a record both pick the same next number, and the collision is only discovered at merge time, when both records are already written, cross-referenced and cited from code comments.This is not hypothetical here. It happened twice, and the archaeology is in the repository:
9798390— "Renumber serve-time-transform ADR to DEC-0045 (DEC-0043 taken by GeoGebra Fix Geogebra grade item detection #30)". Two branches claimedDEC-0043; one had to be renamed after the fact.research/tareas/diario/2026-06-17-adr-validacion-xapi-y-2.0.yamlrecords the second: an xAPI record was opened asDEC-0059, discovered to collide with four records on the unmergedfeature/secure-iframe-scorm-bridgebranch (DEC-0059…DEC-0062), and renumbered toDEC-0063— file,id, and every reference instatus.yaml,AN-014,FTE-017and the journal.A renumber is expensive because an identifier is not just a filename: it is quoted in PHP comments, in
scripts/, in workflow step names, inblueprint.json, and in dozens of research documents. The counter also carries no information —DEC-0045tells you nothing about which change produced it.Identification model
Adopted from
exelearning/exelearning#2232, keeping this repository'sDEC-prefix and its Spanish prose.A decision is identified by the GitHub tracking number of the change that motivates it: the issue when the change has one, otherwise its pull request. GitHub allocates issue and PR numbers from a single repository-wide sequence — in GitHub's data model a pull request is an issue, which is why
/issues/<n>resolves to a PR — so the two can never collide.<tracking-number>has no leading zeros.<local-sequence>is two digits, scoped only to that tracking number, starting at01, present even when a change produces a single record — so adding a second one later never renames the first.<slug-de-la-decision>names the decision, not the topic.idequalsDEC-<number>-<NN>,tracking_issueholds the number,legacy_idkeeps the retired identifier.# <id>: <título>.There is no global counter and no next-free-number to compute. Two branches can only collide if they share a tracking number, in which case they are the same change.
tracking_issueandlegacy_idkeep their English names on purpose: they are the fields shared with the other eXeLearning repositories applying this convention. Prose, titles,agentes,fuentes,relacionadosandherramienta_iaremain in Spanish and are untouched.How each number was established
By order of authority, and re-derived independently for every record rather than trusted from an inventory:
(#N)subject of the squash commit that added the file (git log --diff-filter=A --follow -- <path>).mainand that PR names the record explicitly.32 of the 46 numbers match the adding commit's
(#N)exactly. The 14 that differ all do so for a documented reason: issue#13(12 records) and issue#29(1 record) are preferred over the PRs that implemented them, andDEC-16-01arrived via direct push in0f65dc5and is attributed to PR #16, whose title namesDEC-0027.Old → new mapping
46 records migrated, all with
git mv.DEC-0016DEC-4-01DEC-0017DEC-5-01DEC-0018DEC-6-01DEC-0020DEC-11-01DEC-0021DEC-12-01DEC-0022DEC-13-01DEC-0023DEC-13-02DEC-0024DEC-13-03DEC-0025DEC-13-04DEC-0026DEC-13-05DEC-0027DEC-16-01DEC-0028DEC-13-06DEC-0029DEC-13-07DEC-0030DEC-13-08DEC-0031DEC-13-09DEC-0032DEC-17-01DEC-0033DEC-18-01DEC-0034DEC-19-01DEC-0035DEC-19-02DEC-0037DEC-13-10DEC-0038DEC-25-01DEC-0039DEC-26-01DEC-0040DEC-26-02DEC-0041DEC-26-03DEC-0042DEC-13-11DEC-0043DEC-29-01DEC-0044DEC-34-01DEC-0045DEC-34-02DEC-0046DEC-36-01DEC-0047DEC-37-01DEC-0048DEC-66-01DEC-0049DEC-67-01DEC-0050DEC-13-12DEC-0051DEC-68-01DEC-0052DEC-69-01DEC-0053DEC-70-01DEC-0054DEC-71-01DEC-0055DEC-72-01DEC-0056DEC-74-01DEC-0057DEC-77-01DEC-0058DEC-78-01DEC-0064DEC-85-01DEC-0065DEC-106-01DEC-0066DEC-108-01DEC-0067DEC-110-01DEC-0068DEC-111-01Not migrated — 18 records, reported honestly
DEC-0001…DEC-0015,DEC-0019,DEC-0036,DEC-0063keep the retired numbering. All eighteen were pushed straight tomainduring the repository's bootstrap period:gh api repos/.../commits/<sha>/pullsreturns an empty list for every one of their adding commits. They have no verifiable tracking number, and no number was invented and no issue was opened to manufacture one.Leaving them is safer than a half-guess: those identifiers still resolve, because the files still exist under those names, so every existing reference to them remains correct. The list is frozen in
check_decisions.py— any newDEC-NNNN-*.mdfails validation, so the debt cannot grow. Their adding commits and three options for closing the gap are recorded in the migration map.Records and references
docs/,scripts/*.sh,.github/workflows/*step names,blueprint.json,Makefile,AGENTS.md,research/status.yaml, backlog, journal, analysis and source notes.Non-mechanical parts were handled by hand: abbreviated group references (
DEC-0022/0037) were expanded before rewriting, 9 range references (DEC-0051..0054) were rewritten as prose, andamd/build/fullscreen.min.js+.mapwere updated in step withamd/src/fullscreen.js. The identifier there lives in a comment, so the sourcemapmappingsstring is byte-identical before and after (558 chars both sides) — onlysourcesContentchanges.Every renamed record additionally gained the mandatory
# <id>: <título>H1; the corpus previously had none. No## Estadosections existed, so that cleanup was a no-op, but the validator now enforces it.Three one-sided supersession relationships were completed so the graph validates in both directions:
DEC-13-04 ↔ DEC-13-05,DEC-19-02 ↔ DEC-25-01, andDEC-13-08 ↔ DEC-111-01. The third was documented, not inferred —DEC-111-01's body already said "Supersede a DEC-0030" and listed it underrelacionados.How the index is generated
The index is derived from the records' own frontmatter and never hand-maintained.
make architecture-recordsprints it on demand (check_decisions.py list).research/docs/indices/adrs.yamlcontinues to be produced by the repository's existingresearch/tools/build_indexes.py, alongside the seven sibling indexes that were already generated this way.build_indexes.pynow sorts by tracking number and local sequence — plain filename order putDEC-106-01beforeDEC-11-01. Re-running it leaves the tree clean.The curated "Decisiones clave" table in
AGENTS.mdwas kept, not deleted: it carries editorial commentary that exists in no frontmatter field. It is now explicitly labelled a curated summary and points at the generated index as authoritative.What CI now validates
A new Validate decision records step in the existing matrix-free
release-workflow-checkjob, next to the other static guards. The checker's own unit tests run first, so a broken checker cannot silently pass everything.research/tools/check_decisions.pyvalidates:idmatches the filename,tracking_issuematches the number,legacy_idpresent on migrated records, status from a known set, plausible dates,herramienta_iaprovenance present;idandtitulo, and no duplicated## Estadosection exists;research/decisiones/cambios/<n>-<slug>/design directories, when any exist;legacy_idfield and a documented allowlist of historical journal entries.Tooling is Python 3, stdlib only, not Bun/TypeScript: this repository has no Node/Bun runtime of its own (bun is only fetched to build the embedded editor) and
research/tools/already ships Python tools, so no new runtime is introduced.research/tools/test_check_decisions.pycarries 42 unit tests, three of which assert that the real corpus passes.research/schemas/decision.schema.yaml,research/decisiones/README.md,research/plantillas/markdown/plantilla-adr.md,research/README.md,research/AGENTS.mdandresearch/tools/README.mdwere updated for the new policy.Commands run, with results
Independent verification performed against the branch, not against the diff's own claims:
git diff --find-renames main..HEADreports 46Rentries and zero add/delete pairs.git log --followtraverses each one, including through the pre-existingDEC-0043 → DEC-0045rename in9798390.mainagainst new file on the branch, for all 46. No field was dropped.agentes,fuentes,fecha,tituloandherramienta_iaare byte-identical in every record; the only changes areid, the addedtracking_issue/legacy_id, updatedrelacionados, and the three supersession completions above. Noestadovalue changed.id,tracking_issueand H1 was re-derived and cross-checked: 0 mismatches, 0 duplicate ids, 0 duplicate local sequences, every sequence starts at01and is contiguous, no leading zeros.git grepfor all 46 retired identifiers across the whole tree returns hits only in the migration map,legacy_idfields, the validator's own test fixtures, and one historical journal entry that is explicitly annotated as a historical record.[[wikilinks]]acrossresearch/anddocs/resolve: 0 broken.DEC-0030→exit=1; a newDEC-0099-*.md→exit=1; a duplicatedDEC-13-01→exit=1. Clean tree →exit=0.Not run
make test(PHPUnit) andmake behatrequire bringing up the Docker Moodle stack, which binds host port 80 and initialises Moodle and a PHPUnit database on first run. The PHP diff is comment-only except two diagnostic strings — adebugging(..., DEBUG_DEVELOPER)message inclasses/local/track.phpand a PHPUnit assertion message intests/lib_test.php. All 18assertDebuggingCalled()call sites in the suite are argument-less, so no test asserts either string.Known limitations
DEC-<pr>-01-…in a follow-up commit on this branch. Meanwhile the policy README citesexelearning/exelearning#2232as the source of the convention.exelearning/exelearning(#2058 and #2056). The local numbers are used because they remain permanently consumed in this repository's issue/PR sequence,https://github.com/exelearning/moodle-mod_exelearning/issues/13still returns a 302 to the new location (verified), and every existing document already says "issue #13". Adopting 2058 would import another repository's numbering and would eventually collide with this repository's own PR #2058. The rationale is recorded in the migration map.feature/secure-iframe-scorm-bridge) adds 7 further records on the retired scheme. It is deliberately untouched here and must migrate them on its own branch; the validator will fail it until it does.research/decisiones/cambios/does not exist yet. The validator supports change directories, but no design document in this repository currently warrants one, and empty scaffolding was not created.Moodle Playground Preview
The changes in this pull request can be previewed and tested using a Moodle Playground instance.
ℹ️ The eXeLearning editor is fetched from the shared release and unpacked into the plugin when the playground boots, so the first load may take a few extra seconds. ELPX upload, viewer and preview work normally.