From 9ead10b87f443fd91b2eeb364a4d80411ad9af14 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 18 Aug 2026 08:16:20 +0200 Subject: [PATCH 1/2] docs(adr): anchor the builtin-tool count where it carries an argument ADR-108 and ADR-094 both state the builtin count, and both were wrong by five. In ADR-094 the number is not colour: "covers all 41 builtins without 41 edits" is the reason the contract change was deferred, so a reader re-deriving that trade-off worked from the wrong denominator. ADR-108's first mention needed no number at all and now has none. The two that carry a magnitude keep one and gain the command that produces it, the way ADR-169 does: 46 today via `grep -l ToolInterface Classes/Service/Tool/Builtin/*.php | wc -l`, with 41 kept as what the record said when written. The second number in ADR-094's sentence was checked too, since correcting one and leaving the other is the defect this fixes: the seven explicit declarations are still exactly seven (`grep -l 'function getDataClass' Classes/Service/Tool/Builtin/*.php`). Neither decision changes. getDataClass() is still not on the contract, and 46 edits are still more than seven declarations. Closes #792 Signed-off-by: Sebastian Mendel --- Documentation/Adr/Adr094ToolDataClassTrustZone.rst | 11 ++++++++--- .../Adr/Adr108TypedToolResultWithArtifacts.rst | 8 ++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Documentation/Adr/Adr094ToolDataClassTrustZone.rst b/Documentation/Adr/Adr094ToolDataClassTrustZone.rst index 7ba5aa57e..a3e8f2222 100644 --- a/Documentation/Adr/Adr094ToolDataClassTrustZone.rst +++ b/Documentation/Adr/Adr094ToolDataClassTrustZone.rst @@ -110,9 +110,14 @@ Consequences external ceiling. That is correct — the run really can reach that provider — but it will surprise operators who added a fallback purely for availability. - ``ToolInterface`` is **not** changed yet. Classifying by group plus seven - explicit declarations covers all 41 builtins without 41 edits; promoting - ``getDataClass()`` onto the contract is a later, announced breaking change, - once observe-mode evidence exists. + explicit declarations covers every builtin without one edit per builtin. + The ratio is the argument, not the number, and it has only widened since: + 41 builtins when this was written against 46 today + (``grep -l ToolInterface Classes/Service/Tool/Builtin/*.php | wc -l``), + while the declarations are still seven + (``grep -l 'function getDataClass' Classes/Service/Tool/Builtin/*.php | wc -l``). + Promoting ``getDataClass()`` onto the contract is a later, announced + breaking change, once observe-mode evidence exists. - ``isEnabledByDefault()`` and the "never-toggled group is enabled" default stay as they are. Flipping them would make a fresh install offer zero tools and buy nothing the ceiling does not already buy. The fail-closed default belongs on diff --git a/Documentation/Adr/Adr108TypedToolResultWithArtifacts.rst b/Documentation/Adr/Adr108TypedToolResultWithArtifacts.rst index 88abed9e0..33417aaa8 100644 --- a/Documentation/Adr/Adr108TypedToolResultWithArtifacts.rst +++ b/Documentation/Adr/Adr108TypedToolResultWithArtifacts.rst @@ -82,7 +82,7 @@ emitter) is intentionally deferred rather than shipped empty. The sole v1 emitter is ``ReadRecordsTool``, which builds its ``TABLE`` rows from the SAME already-redacted ``formatValue()`` cells its text lines use, in one pass — the artifact can never drift from, or re-expose more than, the text -egress. The other 40 builtins ship text-parity via a mechanical +egress. Every other builtin ships text-parity via a mechanical ``ToolResult::text($string)`` wrap. Fail-closed bounding @@ -123,7 +123,11 @@ This is intentional for the admin-only module, not a bypass. Consequences ============ -- ``ToolInterface`` is a breaking change across all 41 builtins. Pre-1.0 +- ``ToolInterface`` is a breaking change across every builtin. That was 41 of + them when this was written; the live count is + ``grep -l ToolInterface Classes/Service/Tool/Builtin/*.php | wc -l``, which + answers 46 today. The magnitude is the point here, so it is anchored to the + command rather than left as a number nothing re-derives. Pre-1.0 (:ref:`ADR-090 `) this is acceptable and announced; third-party tools discovered via the ``nr_llm.tool`` tag must return a ``ToolResult`` (the ``ToolResult::text()`` factory keeps the trivial case a one-line change). From 9b902993dfa6e0b12f67799b635e606b0c554f2c Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 18 Aug 2026 08:16:20 +0200 Subject: [PATCH 2/2] docs(adr): correct the module inventory ADR-119 opens with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The record enumerated twelve submodules; fourteen are parented to `nrllm`. The two it never named are use-case packs and MCP servers. The count is repeated three more times in the record and all four are corrected, with `grep -c "'parent' => 'nrllm'"` given as the source to re-derive rather than the sentence to trust. nrllm_aitasks is named explicitly as NOT part of the number. It is an nr_llm backend module parented to `web` on purpose (ADR-131), and the obvious way for a later reader to "fix" fourteen is to fold it in. The cost paragraph carried three more figures and every one had drifted: two setShortcutContext calls are 16, 33 references in 20 documentation files are 93 in 40, roughly 17 screenshots are 20. They are dropped rather than restated, because the record already discounts that argument — cost answers what it takes, not what is right. What they said is kept as history so the paragraph still reads as a record. The decision is untouched. ADR-119 stays Accepted (deferred) and its Revisit section still owns whether the modules move. Closes #791 Signed-off-by: Sebastian Mendel --- .../Adr/Adr119BackendModulePlacement.rst | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/Documentation/Adr/Adr119BackendModulePlacement.rst b/Documentation/Adr/Adr119BackendModulePlacement.rst index ae0333b50..1ea2dc9e6 100644 --- a/Documentation/Adr/Adr119BackendModulePlacement.rst +++ b/Documentation/Adr/Adr119BackendModulePlacement.rst @@ -15,10 +15,17 @@ ADR-119: Where the backend modules live — Administration, for now Context ======= -nr_llm registers a parent module ``nrllm`` with twelve submodules — providers, -models, configurations, tasks, snippets, skills, tools, playground, agent runs, -analytics, setup wizard and overview — under TYPO3's **Administration** -section. The question raised: should this become its own top-level section, +nr_llm registers a parent module ``nrllm`` with fourteen submodules — overview, +providers, models, configurations, tasks, snippets, use-case packs, setup +wizard, skills, tools, MCP servers, playground, agent runs and analytics — +under TYPO3's **Administration** section. +``grep -c "'parent' => 'nrllm'" Configuration/Backend/Modules.php`` answers +fourteen, and is the count to re-derive rather than trust this sentence. + +A fifteenth nr_llm module, ``nrllm_aitasks``, is parented to ``web`` on purpose +(:ref:`ADR-131 `) because the menu hides a child whose parent's access +check fails. It is not a submodule of this tree and must not be folded into the +number by a later reader correcting it. The question raised: should this become its own top-level section, a sibling of Content, Media, Sites, Administration and System, and should it be called "LLM" or "AI"? @@ -32,11 +39,14 @@ admin-only modules."** Access level is not the grouping principle. In the core, ``systemMaintainer`` — stricter than admin, not the same. Sections mix access levels, so this explains nothing. -**"A move is expensive."** It is — the ``nrllm`` route and its bookmarks, two -``setShortcutContext`` calls, the docheader submodule dropdown, 33 references -across 20 documentation files, roughly 17 backend screenshots, and -``t3_cowriter``'s ``position => ['after' => 'nrllm']`` anchor. But cost answers -"what does it take", not "what is right". The two must not be confused. +**"A move is expensive."** It is — the ``nrllm`` route and its bookmarks, the +``setShortcutContext`` calls, the docheader submodule dropdown, the module +identifier spread across the documentation and its backend screenshots, and +``t3_cowriter``'s ``position => ['after' => 'nrllm']`` anchor. This record +originally counted them: two calls, 33 references in 20 files, roughly 17 +screenshots. Every one of those has since grown. The figures are dropped rather +than maintained, because cost answers "what does it take", not "what is right". +The two must not be confused. .. _adr-119-context-principle: @@ -117,7 +127,7 @@ When it is reopened, these are settled in advance: identifier is a shared namespace with no owner: the label and icon would depend on package load order, and removing the owning extension would strip the routes of any foreign submodules parented to it. -- **Twelve flat entries do not move as they are.** They read as a dumping +- **Fourteen flat entries do not move as they are.** They read as a dumping ground at any level. Group them by subject first — setup (provider, model, configuration), authoring (tasks, skills, snippets), operation (tools, playground, runs, analytics) — and let the section hold three or four @@ -136,9 +146,9 @@ Consequences - No code changes. The placement, identifiers, routes and documentation stay as they are. - The discoverability problem is real and remains: TYPO3's module menu renders - two levels, and nr_llm's twelve submodules sit at the third, so they are + two levels, and nr_llm's fourteen submodules sit at the third, so they are invisible from the main menu. That is worth fixing on its own terms — by - strengthening the Overview as the hub, or by grouping the twelve — and does + strengthening the Overview as the hub, or by grouping the fourteen — and does not require the top level. - If the editor surfaces are built without reopening this ADR, they will land in an admin-only section where their users cannot reach them. The revisit