Code review fixes: Twig, console commands, on-disk config, and the last edition mechanism - #3403
Merged
Conversation
The spec says a page containing Twig markup is deleted, but "Sections kept" retains Page Builder, Form Builder and Site Factory as authoring features. Where the two collide the KEEP list wins: strip the Twig, keep the page. - Removed the Twig fences and their introducing prose from the nine pages that carried them, plus the Twig-template configuration residue on invitations.md, user_registration.md, imageassetfield.md, user_generated_content.md, workflow.md and cp_configuration.md. - Deleted customer_management/create_user_registration_form.md: stripping left only the user_registration configuration, which users/user_registration.md already documents. Inbound links and the nav entry now point there. - Deleted all 98 tracked .twig files and the four code samples orphaned by the strip; nothing under docs/, tools/ or tests/ references them any more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Applies the spec rule "a console command means its containing section is removed; if the page has nothing left, the page is deleted" to every remaining `bin/console`, `composer` and `yarn` instruction found by a fresh grep of docs/. Deleted: - users/update_basic_user_data.md: all five sections were `bin/console` recipes, so nothing survived. Its nav entry and the users.md card are gone; its only outbound link pointed at the passwords.md section also removed here. - snippets/page_block_cache_clear.md: the whole snippet was a cache-clearing console recipe, so its three include macros go with it. Stripped: users/passwords.md (Revoking passwords), configure_ai_actions.md (Initiate integration), mcp_usage.md, mcp_config.md, raptor_cdp_data_export.md, back_office_configuration.md, languages.md, set_up_translation_siteaccess.md, configure_translations_management.md, install_quable.md (Synchronize taxonomy), rest_api_usage.md and subitems_list.md. No UI replacement is invented for any removed capability; whether these return as UI actions is an open product decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Applies the spec rule "a package-installation step or an on-disk secrets/environment step is removed". YAML blocks that document a setting's shape are kept; only the prose that names a file to open is reframed to name the configuration key instead. Confirmed sites fixed: mcp_config.md (lexik_jwt_authentication.yaml and security.yaml), rest_responses.md (.env CORS_ALLOW_ORIGIN and nelmio_cors.yaml), connector_installation_configuration.md, fastly_io.md (.env variant of the variation handler), and a further ~15 pages found by a fresh grep of docs/ for `.env`, `config/packages`, `config/services` and `config/bundles.php`. Removed from multisite/site_factory/site_factory.md: - "Define domains", which was a Docker Compose, nginx vhost and /etc/hosts procedure for a dev environment. - "Disable Site Factory", which edited four config files and removed a dedicated database connection and cache pool. The page's own link to the removed section is gone. Left alone deliberately: administration/configuration/configuration.md, which the spec keeps and labels rather than rewriting, and YAML examples whose file paths are part of a setting's value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ipeline PILL_CLASS_TO_EDITION still mapped pill--lts-update, pill--experience and pill--headless to "LTS Update", "Experience" and "Headless", and appended them to headings in LLM-facing output — an edition mechanism the conversion missed, found by both axes of the code review. Renames the map to PILL_CLASS_TO_CATEGORY and keeps only pill--new-feature and pill--first-release, which are live release-note categories that main.py still emits. Restores the PEP 8 blank-line gaps. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
matrixfield.md ended on a sentence promising an example that had been removed. search_criteria_and_sort_clauses.md was never stripped and had become the Search section's only entry-point card while still linking Legacy and Solr criterion handlers on GitHub, telling the reader to develop a custom Criterion, and documenting service-container tags. Reduced to the concepts a SaaS reader can use, including the Content and Location search distinction restated without the PHP method table, plus the REST custom Field Criterion section. search.md promised aggregations and embeddings without linking either and rendered a single card; it now links what it promises and lists the surviving reference trees. user_authentication.md is deleted. It documented chaining Symfony user providers, injecting a repository user through a PHP listener and editing config/packages/security.yaml — none of which a tenant can do. Both OAuth pages were already deleted on the same reasoning, and restoring identity-provider capability is out of scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restates composer.json's description and keywords and the agent package's README in Cohesivo terms. The composer "name" field is left alone: it is the published package identity that consumers require, and renaming it would break them. Flattens the "### REST API" heading on 57 reference pages where it was the only child of "## Example" — a level left redundant when "### PHP" was cut. Verified beforehand that no page links to a "#example" or "#rest-api" anchor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Fixes every finding from the two-axis code review of the Cohesivo SaaS
conversion. Base:
saas-batch-3.211 files changed, 2,633 deletions, 96 insertions, in six commits — one per
finding group, worst first.
1. Twig survived the conversion (spec US13)
The spec says "A page containing Twig markup is deleted", but ticket 08 only
deleted the Templating section. Nine pages elsewhere still carried
html+twigfences or.twigincludes, and 98.twigfiles were stilltracked. No ticket owned Twig outside
docs/templating/, and no invariantchecked for it — the conversion measured PHP and forgot the other template
language.
Where the rule collided with "Sections kept" (Page Builder, Form Builder, Site
Factory are retained authoring features), the keep list wins: the Twig was
stripped, the pages kept. All 98
.twigfiles are gone.2. Console commands survived (spec US6)
13 pages still instructed
bin/console. This is the structural gap flaggedthree times during the conversion (residue R3, R13, R19): tickets 10 and 13
owned only pages whose entire subject was a command, so pages that merely
mention one had no owner.
3. On-disk configuration and secrets survived (spec US8)
Pages still told the reader to edit
config/packages/security.yaml, setCORS_ALLOW_ORIGINin.env, and edit.envfor a Docker compose file.Describing what a setting does is kept; instructing the reader to open a file on
disk is not.
4. An edition mechanism survived in the LLM pipeline (spec US29)
PILL_CLASS_TO_EDITIONstill mappedpill--lts-update,pill--experienceandpill--headlessto "LTS Update", "Experience" and "Headless" and appended themto headings in LLM-facing output. Ticket 04 kept the map because
_process_release_note_tagsuses it — correct as far as it went, but it neverchecked which entries were still live.
Renamed to
PILL_CLASS_TO_CATEGORY, keeping only the two live release-notecategories
main.pystill emits.5. Reader-visible defects the strip left
matrixfield.mdended on "…because the second row is empty:" with the examplegone.
search_criteria_and_sort_clauses.mdwas never stripped and had become theSearch section's only entry-point card while still linking Legacy and Solr
criterion handlers on GitHub, telling the reader to develop a custom Criterion,
and documenting service-container tags. It has no PHP fence, so every
invariant passed it — a page can be thoroughly on-premise without one code
sample. Reduced to what a SaaS reader can use, keeping the Content/Location
search distinction restated without the PHP method table.
search.mdpromised aggregations and embeddings without linking either andrendered a single card. It now links what it promises and lists the surviving
reference trees.
user_authentication.mddeleted. It documented chaining Symfony userproviders, injecting a repository user through a PHP listener, and editing
security.yaml. Both OAuth pages were already deleted on the same reasoning,and restoring identity-provider capability is explicitly out of scope.
6. Naming and cosmetics
composer.json's description and keywords and the agent package README now readCohesivo. The composer
namefield is deliberately unchanged — it is thepublished package identity consumers require, and renaming it would break them.
The redundant
### REST APIheading is flattened on 57 reference pages whereit was the only child of
## Example, left over from cutting### PHP.Verified beforehand that nothing links to a
#exampleor#rest-apianchor.Reviewed and deliberately not changed
saas_reviewfrontmatter on 38 pages. The Standards axis flagged itagainst
.agents/domain.md("agent-facing docs live outsidedocs/"), but thespec mandates exactly this (user stories 26-28, 51), and it was verified absent
from both
site/andllms-full.txt— genuinely author-facing.docs/index.md's second person and contractions. Consistent with theexisting corpus; a
WRITING.mdquestion, not a diff question.dump_class_paths.php'sJSON_FORCE_OBJECT, flagged as scope creep. Itfixes a latent crash where
json.loadsreturned a list into a.get()call.Verification
mkdocs build --strictexit 0 ·pytest -q85 passed, 0 warnings · nounresolved
[[= =]]·composer.jsonparses.Final self-check across
docs/**/*.md, all zero: PHP fences,html+twigfences,
.twig/.phpinclude macros,bin/console,composer require,edition:frontmatter,delete:frontmatter. Tracked.twigandcode_samples/*.phpfiles: 0.🤖 Generated with Claude Code