Skip to content

docs(adr): retire every line citation in the corpus, and three of them were wrong - #860

Merged
CybotTM merged 2 commits into
mainfrom
docs/adr169-symbol-citations
Aug 21, 2026
Merged

docs(adr): retire every line citation in the corpus, and three of them were wrong#860
CybotTM merged 2 commits into
mainfrom
docs/adr169-symbol-citations

Conversation

@CybotTM

@CybotTM CybotTM commented Aug 21, 2026

Copy link
Copy Markdown
Member

Closes #793. Stacked on #858 — review that one first; this PR's diff against it is ADR-169, ADR-140, ADR-174 and the test.

Together with #858 this takes the corpus to zero line citations. The two left in Adr/Index.rst are the convention's own examples of what not to write.

ADR-169 alone carried 137, 92 of them bare :NNN continuations. Converting each one means reading what it points at, and that is where the value turned out to be.

Three citations no longer said what the record claims

Section 7 enumerates the 40 denyNonAdmin() sites, and one entry had drifted onto the wrong method:

Record said ToolPlaygroundController :155, :294, :367
The three gates are at :156, :295, :381
:367 resolves to resumeAction() — already the second entry
So the record named resumeAction() twice, never named submitInputAction(), and still totalled three

The list now names all 40 by method, and it reconciles exactly against the grep the section opens with:

real denyNonAdmin() sites:      40
named in the record:            40
  in code, missing from record: —
  in the record, not in code:   —

UseCasePackController was cited at :129, which is inside showAction(); installAction() begins at :141. The prose named the right method — the line number was the only wrong part, which is rather the point.

ext_conf_template.txt:132 is mcpOperationTimeout, cited for the routing policy mode. That key is routing.policyMode at :139. A citation seven rows off landed on an unrelated setting and read as precise.

And the method names throughout were uniformly missing their Action suffix — testConnection for testConnectionAction — so not one of them would have resolved as a symbol.

Most citations were deleted, not converted

Same finding as #858, more pronounced here:

:php:`DataHandler::isTableAllowedForThisPage()` (``DataHandler.php:7484-7503``)

The symbol is right there. Section 4's seven field citations are the clearest case — system_prompt_data_class (:222) — the field name is a better anchor than any line, and it is already in the sentence. ADR-140's six and ADR-174's one are all this shape.

The escape hatch that emptied, and PHPStan's objection

Twenty of ADR-169's citations reach into TYPO3 core. Those become symbols too: a core symbol survives a patch release, a core line number does not, and this repository does not pin the dependency whose line numbers those were.

That left the test's declared list of unverifiable citations — sixteen entries — completely empty, and PHPStan refused the result at level 10:

Call to function in_array() with arguments mixed, array{} and true
will always evaluate to false.

It is right, and the fix is not a suppression. The list, the branch consulting it and the test reconciling it are gone. everyCitationNamesAFileThatStillExists is now absolute and says what to do instead in its own failure message. An escape hatch nothing reaches for is a declaration nothing reads — and the convention that replaced it is what made it unreachable.

One thing that is load-bearing and looks incidental

The suite globs Adr*.rst, so Adr/Index.rst is not scanned. That is what lets the convention page quote ResumeCoordinator.php:204 and AgentRunController.php:267 as examples of rotted citations. Widening the glob to *.rst would fail the build on the page documenting the rule — now stated in the test docblock so nobody "fixes" it.

One caution for anyone repeating this

I first tried to resolve all 137 mechanically, carrying the "current file" forward across the bare :NNN continuations. That mapping was wrong for all 44 citations in section 7 — the numbers index the controller classes named in the surrounding prose, not the file the previous citation named, and the script happily resolved them against Configuration/Backend/Modules.php and produced 40 plausible #controllerActions anchors. The automation is usable as a lookup table; it is not usable as an applier, because the correct target is in the sentence, not in the syntax.

Verification

gate result
-s unit (full) 7285 tests, 24592 assertions, exit 0
-s phpstan No errors
-s cgl -n SUCCESS
line citations in Adr*.rst 0
anchors / :ref: / repo symbols all resolve
control: reintroduce ExtensionConfiguration.php:150 fails, naming the file and what to write instead

Assisted by claude-code:claude-opus-5 — Session

… wrong

ADR-169 is the corpus's densest citer: 137 line citations, 92 of them
bare :NNN continuations. Converting each one means reading what it points
at, and three no longer pointed at what the record says.

Section 7 enumerates the 40 denyNonAdmin() sites. ToolPlaygroundController
was cited at :367, fourteen lines short of the third gate at :381, so the
resolved method was resumeAction() — which the list already named. The
record therefore counted resumeAction() twice and never named
submitInputAction(), while still totalling three. It now names all 40 by
method, and the set reconciles exactly against the grep the section opens
with: 40 sites in the code, 40 named, nothing in either direction.

UseCasePackController was cited at :129, inside showAction();
installAction() begins at :141. The prose named the right method, so the
line number was the only wrong part — which is the argument for dropping
it.

ext_conf_template.txt:132 is mcpOperationTimeout, cited for the routing
policy mode. That key is routing.policyMode at :139, and the anchor now
names it rather than a line seven rows above it.

The method names were also uniformly missing their Action suffix
(testConnection for testConnectionAction), so none of them would have
resolved as a symbol.

Most citations were not converted but deleted. The prose already named
the symbol in the same sentence — hasGrant(), isTableAllowedForThisPage(),
getSecureFieldConfig() — and the line number carried nothing except a
surface to rot. Seven field citations in section 4 sit behind the field
names they cite, which are better anchors than any line.

Twenty citations reach into TYPO3 core. Those become symbols too: a core
symbol survives a patch release, a core line number does not, and this
repository does not pin the dependency whose line numbers those were. The
declared list of unverifiable citations drops from fifteen entries to
none for this record, and the control was run — reintroducing
DataHandler.php:7484 now fails as an undeclared citation where it
previously passed as a listed one.

Refs #793

Assisted-by: claude-code:claude-opus-5
Agent-Session: https://claude.ai/code/session_01MNg1MysJVugv1xo2husknU
Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
@CybotTM
CybotTM requested a review from a team as a code owner August 21, 2026 05:37
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests Test-related changes labels Aug 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.69%. Comparing base (d37804e) to head (83b3536).

Additional details and impacted files

Impacted file tree graph

@@                       Coverage Diff                       @@
##             docs/adr171-symbol-citations     #860   +/-   ##
===============================================================
  Coverage                           86.69%   86.69%           
  Complexity                           9919     9919           
===============================================================
  Files                                 574      574           
  Lines                               32064    32064           
===============================================================
  Hits                                27799    27799           
  Misses                               4265     4265           
Flag Coverage Δ
unit 65.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…ches for

ADR-140's six and ADR-174's one are all the redundant shape: the symbol is
named in the same sentence and the line number sat beside it carrying
nothing but a surface to rot. They are deleted, not converted.

That takes the corpus to zero line citations in Adr*.rst. The two left in
Adr/Index.rst are the convention's own examples of what not to write, and
they survive only because the suite globs Adr*.rst — which is now stated
in the test, because widening that glob to *.rst would fail the build on
the page documenting the rule.

With no record citing .Build/vendor by line, the declared list of
unverifiable citations emptied, and PHPStan would not accept the result:
in_array() against an empty array always evaluates to false. It is right,
and the fix is not a suppression. The list, the branch that consulted it
and the test that reconciled it are gone; everyCitationNamesAFileThat-
StillExists is now absolute and says why in its failure message.

Sixteen entries existed there when the corpus cited core by line. An
escape hatch nothing reaches for is a declaration nothing reads, and the
convention that replaced it — cite the symbol, not the line — is what
made it unreachable.

The control was run on the simplified assertion: a reintroduced
ExtensionConfiguration.php:150 fails, naming the file and what to write
instead.

Refs #793

Assisted-by: claude-code:claude-opus-5
Agent-Session: https://claude.ai/code/session_01MNg1MysJVugv1xo2husknU
Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
@CybotTM CybotTM changed the title docs(adr): retire the last 137 line citations, and three of them were wrong docs(adr): retire every line citation in the corpus, and three of them were wrong Aug 21, 2026
@sonarqubecloud

Copy link
Copy Markdown

github-actions[bot]
github-actions Bot previously approved these changes Aug 21, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

@CybotTM

CybotTM commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

Review record — no Copilot review exists (monthly, account-wide quota), so the green gate is not a read of this change.

What the gate cannot see here, and what I did instead. A docs PR that rewrites 137 citations can silently point them somewhere else, and only one of the four resulting forms is machine-checkable. So every citation was resolved against the tree before it was written: each :php: symbol verified to exist as function <name>, each anchor with grep -F, each :ref: against the corpus's actual labels. That pass is what produced the three findings in the body — none of them came from reading the prose.

The one I would flag to a reviewer as the load-bearing claim: the section-7 enumeration now reconciles 1:1 against grep -rn 'denyNonAdmin()) instanceof' Classes/Controller/ — 40 sites in the code, 40 named, nothing extra in either direction. Before this change it named resumeAction() twice and submitInputAction() never, while still totalling three. That reconciliation is a script I ran, not an impression.

A dead end worth knowing. I first tried to resolve all 137 mechanically. The mapping was wrong for all 44 citations in section 7 — the numbers index the controller classes named in the surrounding prose, not the file the previous citation named — and the script produced 40 plausible-looking anchors against the wrong file. Anything that reads like automation output in this diff was checked by hand afterwards.

The guard change was controlled. Emptying the declared list of unverifiable citations is a weakening unless something still fails: reintroducing ExtensionConfiguration.php:150 fails with a message naming the file and what to write instead. Run, observed, restored.

PHPStan objected to the result and was right. With the list empty, in_array() against array{} is provably always false. The list, the branch consulting it and the test reconciling it are gone rather than suppressed — an escape hatch nothing reaches for is a declaration nothing reads.

Stacked on #858, so queue=false until that one merges. Not merging on this record — I hold no merge authorisation for this PR.

Assisted by claude-code:claude-opus-5 — Session

Base automatically changed from docs/adr171-symbol-citations to main August 21, 2026 11:11
@CybotTM
CybotTM dismissed github-actions[bot]’s stale review August 21, 2026 11:11

The base branch was changed.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

@CybotTM

CybotTM commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

Self-review: 83b3536

The review this pull request demands is unsatisfiable (Copilot quota wall or repeated bot failures on this head). Per the documented fallback, the diff on this head was reviewed by the PR author; this comment is the on-the-record attestation the merge gate reads back. It stops matching on the next push.

@CybotTM
CybotTM added this pull request to the merge queue Aug 21, 2026
Merged via the queue into main with commit 4b73c4c Aug 21, 2026
109 checks passed
@CybotTM
CybotTM deleted the docs/adr169-symbol-citations branch August 21, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation tests Test-related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ADRs cite code by line number and the citations rot silently — nothing checks them

1 participant