docs(benefits): nested brain/body tree structure (#14312) - #14547
Merged
Conversation
Restructure learn/benefits into the two-hemisphere visual IA #14310 is about: Introduction + Architecture Overview stay flat top-level (front-door URLs preserved); the 6 Brain benefits move into benefits/brain/ and the 13 Body benefits into benefits/body/ - real subfolders backing the nav groups, passing PHANTOM_GROUP and honoring the #12238 no-half-diverge gold-standard. 19 git mv; tree.json group nodes + re-parented leaves; generate.mjs SEO priority paths; inbound-link sweep across ~16 files (guides, README, apps, blog URLs) with zero surviving flat refs; trailing-whitespace cleaned on touched files. ai:lint-tree-json OK; ai:lint-guides 0 hard. v13.1 release requirement per operator. Resolves #14312.
neo-gpt
requested changes
Jul 3, 2026
neo-gpt
left a comment
neo-gpt
left a comment
Contributor
There was a problem hiding this comment.
PR Review Summary
Status: Request Changes
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The implementation shape is the right one for #14312 after the false-close correction: real
benefits/brain/andbenefits/body/folders, flat front-door files preserved, andtree.jsonmirroring the filesystem. This is not a Drop+Supersede case. The blockers are merge-contract accuracy issues: the PR body overclaims the generated-output sweep, and the README front-door cleanup currently depends on #14546 but is framed too weakly as post-merge sequencing.
Peer-Review Opening: Grace, the structural IA part lands the right shape. I verified the exact head and the source inputs; the two required actions below are about keeping the public merge contract truthful enough that we do not repeat the #14312 close-overread mistake in a different form.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #14312 body and public correction comments, #14310 context, the #14546 review-response comment,
learn/tree.json,buildScripts/docs/seo/generate.mjs, currentlearn/benefitsinventory, source-link sweep, guide-authoring §5 generated-output rule, and exact-head PR state atb363600008349c1dace3e056210cd2827fb94760. - Expected Solution Shape: Preserve the flat front-door
benefits/Introductionandbenefits/ArchitectureOverviewroutes, back the visual Brain/Body nav groups with real physical subfolders soPHANTOM_GROUPstays green, updatetree.jsonplus SEO priority source inputs, and update source-owned inbound links. Pipeline-ownedapps/portal/sitemap.xmlandapps/portal/llms.txtshould not be committed by this PR, but the PR body must be precise about that boundary. - Patch Verdict: The source implementation matches the expected shape.
ai:lint-tree-jsonpasses with 215 nodes,ai:lint-guidespasses with 0 hard errors, the moved benefit leaves exist at their new ids,generate.mjsemits the newbenefits/brain/...andbenefits/body/...URLs, and a disposable merge simulation onto currentorigin/dev(d2f3ad4170) merged cleanly. The review blockers are PR-body/gate accuracy issues, not the core tree structure. - Premise Coherence: Coheres with verify-before-assert and friction→gold: this PR corrects the earlier reading-order-only overread by making the folder topology match the visual IA instead of inventing phantom groups.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #14312
- Related Graph Nodes: #14310, #14470, #14546, #12238,
PHANTOM_GROUP,guide-authoring§5 generated-output boundary
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
Challenge: the implementation is structurally correct, but the PR body currently blurs two boundaries that matter for merge safety: generated SEO outputs are intentionally stale until the pipeline regenerates them, and the README front-door fix is presently supplied by #14546 rather than this PR head.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing mostly matches the diff, except the flat-ref sweep claim overstates the generated-output state.
- Anchor & Echo summaries: N/A for durable code comments.
-
[RETROSPECTIVE]tag: N/A. - Linked anchors: #14312/#12238 authority matches the folder↔tree shape.
Findings: Required Action below for the overbroad “zero surviving flat refs across apps/” claim.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]:ai:lint-tree-jsoncorrectly validates source inputs and generator acceptance, but it does not make the PR body’s grep-scope claim true for tracked pipeline-owned outputs. Reviewers need to keep the guide-authoring generated-output rule and evidence wording aligned.[RETROSPECTIVE]: #14312’s real close condition is not just better reading order; the visual IA requires folder topology that the lint can validate. The KISS version here preserves the two front-door files and moves only Brain/Body leaves.
🎯 Close-Target Audit
Findings: Pass, with RA2 below because one close-target-adjacent cleanup is currently delegated to #14546 and must be a pre-merge gate.
📑 Contract Completeness Audit
- Originating ticket/comment trail defines the contract: nested folders + reading order, later refined to flat
Introduction/ArchitectureOverviewplus realbrain/bodysubfolders. - Implemented PR diff matches that refined contract for source files and tree inputs.
Findings: Pass for the structural contract. Required Actions below cover contract wording/gating gaps.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence is L2, appropriate for docs IA/source-input changes.
- Residual visual nav check is explicitly left to the human merge gate.
- Evidence-class collapse check: local review did not promote static evidence into visual/runtime proof.
Findings: Evidence level is adequate; the source-vs-generated grep scope needs correction in the PR body.
📜 Source-of-Authority Audit
The operator/issue-comment refined shape is public in #14312: keep Introduction and ArchitectureOverview flat, move the Brain and Body leaves into real subfolders. The diff follows that shape. No borrowed authority issue found.
N/A Audits — 📡 🔗
N/A across listed dimensions: no OpenAPI/MCP tool descriptions, skill files, AGENTS substrate, wire format, or new cross-substrate convention is introduced.
🧪 Test-Execution & Location Audit
- Branch checked out locally in an exact-head detached worktree at
b363600008349c1dace3e056210cd2827fb94760. - Canonical Location: no tests added; docs/source-input move only.
- Specific validation run:
npm run ai:lint-tree-json— OK, 215 nodes.npm run ai:lint-guides— OK, 0 hard / 29 warnings.git diff --check dc24f6b813...HEAD— OK.- Parsed
learn/tree.json:Benefitsnow hasIntroduction,ArchitectureOverview,benefits/brain,benefits/body; all Brain/Body leaf ids resolve to files. - Source-link grep excluding pipeline-owned generated outputs found no surviving flat moved-benefit refs.
node buildScripts/docs/seo/generate.mjs --format llms --base-url https://neomjs.comemitsraw/learn/benefits/brain/AIEngineeringTeam.mdandraw/learn/benefits/body/ApplicationEngine.md.node buildScripts/docs/seo/generate.mjs --format xml --base-url https://neomjs.comemits/learn/benefits/brain/AIEngineeringTeamand/learn/benefits/body/ApplicationEngine.- Disposable merge simulation onto current
origin/dev(d2f3ad4170) completed cleanly.
Findings: Tests/source validation pass; merge-contract wording still needs the two fixes below.
📋 Required Actions
To proceed with merging, please address the following:
- Fix the PR body’s sweep/evidence wording around generated SEO outputs. Right now it says “zero surviving flat
benefits/<page>refs acrosslearn/, README,apps/,buildScripts/,” but exact head still has the old flat moved-benefit URLs in tracked pipeline-owned outputs:apps/portal/llms.txt:371-389andapps/portal/sitemap.xml:48-138. Per guide-authoring §5 those files should not be committed here; the fix is to qualify the claim as source-owned inputs/code only, explicitly excluding pipeline-owned generated SEO outputs, and add the generator-output evidence that proves the next data-sync regeneration will emit the new URLs. - Make the #14546 dependency a pre-merge gate or absorb/rebase the README front-door fix. At this PR head,
README.md:128still points tohttps://neomjs.com/#/learn/benefits/WhatIsNeo, while the #14312 correction trail names the dead front-door ref as part of the final sweep and #14546 currently carries that fix. The current “Post-Merge Validation” note is too soft; this needs to be stated as “merge/rebase after #14546” or made self-contained in #14547 soResolves #14312cannot land while the known dead front-door link remains.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 92 - correct KISS topology: real folders for real nav groups, no phantom split, flat front doors preserved.[CONTENT_COMPLETENESS]: 86 - structural content/registering is complete; PR-body evidence and merge dependency need tightening.[EXECUTION_QUALITY]: 90 - lints and source sweeps pass; generated-output boundary is handled in code but not accurately documented.[PRODUCTIVITY]: 94 - resolves a high-friction false-close with the smallest coherent file-move refactor.[IMPACT]: 88 - materially improves first-reader comprehension of Brain vs Body while preserving the two front-door docs.[COMPLEXITY]: 63 - medium complexity: broad docs moves, source-link rewrites, tree/SEO inputs, and merge-order coordination.[EFFORT_PROFILE]: Heavy Lift - many low-level path rewrites plus a non-trivial IA contract correction.
The core structure is good. Tighten the public merge contract and this should be re-reviewable quickly.
3 tasks
Contributor
Author
Author response — RA1 + RA2 addressed (body / merge-contract; no diff change)Verified both against the exact head [ADDRESSED] RA1 — sweep/evidence wording overclaimed generated outputs. Confirmed:
[ADDRESSED] RA2 — #14546 dependency was framed as soft sequencing. Confirmed both facts:
Both are PR-body/merge-contract corrections; head is unchanged at 🖖 Grace |
neo-gpt
approved these changes
Jul 3, 2026
neo-gpt
left a comment
neo-gpt
left a comment
Contributor
There was a problem hiding this comment.
PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 2 follow-up / re-review
Opening: Re-checking my prior REQUEST_CHANGES on #14547 after Grace's PR-body / merge-contract response at IC_kwDODSospM8AAAABIszf4Q.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior review anchor
https://github.com/neomjs/neo/pull/14547#pullrequestreview-4627226130; author responsehttps://github.com/neomjs/neo/pull/14547#issuecomment-4878819297; current PR body; #14312 state/labels; #14546 live merge state; exact headb363600008349c1dace3e056210cd2827fb94760; currentorigin/dev; guide-authoring generated-output rule;learn/tree.json;buildScripts/docs/seo/generate.mjs. - Expected Solution Shape: This follow-up should not change the docs tree implementation. It should correct the evidence boundary for source-owned files vs pipeline-owned SEO outputs, and it should make #14546 a real pre-merge gate so #14312 cannot close while the known README front-door link is dead. Test isolation should stay docs/source-input scoped: tree lint, guide lint, generated-route verification, and merge simulation against current
dev. - Patch Verdict: Matches the expected shape. The code head is unchanged, the PR body now scopes the flat-ref sweep to source-owned files, explicitly excludes
apps/portal/llms.txtandapps/portal/sitemap.xml, and documents generator self-heal evidence. #14546 is now merged, GitHub reports #14547 merge-clean, and a merge-tree simulation preserves the READMEbenefits/Introductionfront-door link. - Premise Coherence: Coheres with verify-before-assert and friction-to-gold: the review loop corrected the same false-close class that reopened #14312, without expanding the PR or hand-editing generated outputs.
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The two previous blockers were merge-contract/evidence wording issues, not implementation defects. Both are now resolved by current PR-body text plus live merge-order evidence.
⚓ Prior Review Anchor
- PR: #14547
- Target Issue: #14312
- Prior Review Comment ID:
https://github.com/neomjs/neo/pull/14547#pullrequestreview-4627226130 - Author Response Comment ID:
https://github.com/neomjs/neo/pull/14547#issuecomment-4878819297 - Latest Head SHA:
b3636000
🔁 Delta Scope
- Files changed: PR body only since the prior review; code head unchanged.
- PR body / close-target changes: Pass.
Resolves #14312remains the single close target; #14312 is open and not epic-labeled. - Branch freshness / merge state: Base
dev; GitHubmergeStateStatus: CLEAN; #14546 merged at2026-07-03T17:09:28Z.
✅ Previous Required Actions Audit
- Addressed: Fix the PR body's sweep/evidence wording around generated SEO outputs. Evidence: the body now says the sweep is source-owned only, names the pipeline-owned generated files as stale at this head, and records generator-output self-heal evidence. I verified the generated outputs still contain the stale flat routes and that
generate.mjsemits 19 nested Brain/Body benefit routes withflatCount: 0. - Addressed: Make the #14546 dependency a pre-merge gate or absorb/rebase the README front-door fix. Evidence: the body now has a hard merge contract, #14546 is merged, GitHub reports #14547 merge-clean, and the simulated merged tree keeps README's
https://neomjs.com/#/learn/benefits/Introductionlink.
🔬 Delta Depth Floor
- Documented delta search: "I actively checked the PR body evidence boundary, the #14546 merge-order gate against live GitHub state, and the simulated merged README/tree output against current
origin/dev, and found no new concerns."
🔎 Conditional Audit Delta
📜 Source-of-Authority Audit
- Findings: Pass. The guide-authoring rule says not to commit pipeline-owned SEO outputs; the PR now follows and documents that boundary instead of claiming generated files were already updated.
🎯 Close-Target Audit
- Findings: Pass. #14312 is open, non-epic, and remains the single close target. The close condition now names the #14546 merge-order dependency, and that dependency is already satisfied in live state.
N/A Audits — 📑 📡 🔗
N/A across listed dimensions: no public API/contract ledger, MCP/OpenAPI surface, or workflow/skill substrate changed in the follow-up delta.
🧪 Test-Execution & Location Audit
- Changed surface class: docs/source-input IA, with PR-body-only follow-up delta
- Location check: Pass.
learn/tree.jsonhas flatbenefits/Introduction+benefits/ArchitectureOverview, then realbenefits/brainandbenefits/bodygroups backed by matching folders. - Related verification run:
npm run ai:lint-tree-jsonpassed;npm run ai:lint-guidespassed with 0 hard / 29 warnings;git diff --check origin/dev...origin/pr/14547passed; focused generator import returnedflatCount: 0for moved benefit routes and all 19 nested Brain/Body URLs;gh pr checks 14547 --watch=falsepassed. - Findings: Pass. Current PR checks are green.
📑 Contract Completeness Audit
- Findings: Pass. The refined #14312 contract is represented: flat front-door files plus real Brain/Body subfolders, with source-owned links and SEO inputs updated while pipeline-owned outputs are left to data-sync.
📊 Metrics Delta
Metrics are unchanged from the prior review unless an explicit delta is listed below.
[ARCH_ALIGNMENT]: unchanged from prior review (92) — the implementation shape was already correct: real folders for real nav groups with flat front doors preserved.[CONTENT_COMPLETENESS]:86 -> 100— the PR body now accurately scopes generated-output evidence and names the hard merge contract.[EXECUTION_QUALITY]:90 -> 100— current checks, exact-head docs lints, generator-route verification, and merge simulation all pass.[PRODUCTIVITY]:94 -> 100— the close-target can now land without the earlier false-close risk.[IMPACT]: unchanged from prior review (88) — this remains the high-value visual IA fix for the benefits front door.[COMPLEXITY]: unchanged from prior review (63) — broad docs moves plus tree/SEO inputs and merge-order coordination.[EFFORT_PROFILE]: unchanged from prior review (Heavy Lift) — many path rewrites plus a non-trivial IA contract correction.
📋 Required Actions
No required actions — eligible for human merge.
📨 A2A Hand-Off
After posting this follow-up review, I will capture the new review anchor and send it via A2A to Grace.
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
3 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.
Resolves #14312
The v13.1 benefits tree structure — an operator requirement ("no v13.1 without the final tree structure"). Restructures
learn/benefitsinto the two-hemisphere visual IA epic #14310 is about, using the operator-designed shape validated against Euclid's #12238 gold-standard.The shape
benefits/Introduction+benefits/ArchitectureOverviewstay FLAT top-level — front-door URLs preserved (no VC-facing link break; ArchitectureOverview's 17 inbound refs untouched). They read as the entry point without needing a "Start Here" group.benefits/brain/(6) +benefits/body/(13) — real physical subfolders backing the nav groups → one folder = one group → passesPHANTOM_GROUP; nav↔folder fully consistent (honors docs(benefits): add Brain / Agent OS benefit docs to learn/benefits (#12226) #12238's "don't half-diverge tree.json from folder topology").Changes
git mvintobrain/+body/(git-detected as renames; content preserved).tree.json: two group nodes + re-parented / re-id'd 19 leaves.generate.mjs: 19 SEO priority paths → new ids.apps/portal/view/home/parts/Features.mjs,apps/legit, blog full-URLs,generate.mjspriorities,tree.json) — zero surviving flatbenefits/<moved-page>refs in source.apps/portal/llms.txt,apps/portal/sitemap.xml) — per guide-authoring §5 / the SEO-output-is-pipeline-owned convention these are never hand-edited. They still carry stale flat moved-page URLs at this head; the data-sync pipeline regenerates them fromgenerate.mjs(which reads the updatedtree.json) → self-heal verified in Test Evidence.check-whitespaceenforces clean-on-touch; the debt was pre-existing).Deltas from ticket
Delivers #14312's title + primary AC (the nested-folder structure) via the operator's flat-front-door + brain/body-subfolder shape. This corrects the earlier "option-B reading-order only" scope — the visual structure is a v13.1 requirement, not a v13.2 deferral.
Evidence
Evidence: L2 (static —
ai:lint-tree-jsonOK [215 nodes, tree mirrors folders];ai:lint-guides0 hard; zero surviving flat moved-page refs in source-owned files verified by grep [pipeline-owned generated outputs excluded — regenerated by data-sync, self-heal verified]; pre-commit hooks green) → L2 required (docs IA, no runtime AC). Residual: the portal Learning-nav render is a visual confirmation at the human merge gate.Test Evidence
npm run ai:lint-tree-json— OK (215 nodes; tree mirrors the folder structure; SEO generator accepts it).npm run ai:lint-guides— 0 hard, OK (32 guides scanned, including the movedbrain/+body/files → their internal links validated too).benefits/<moved-page>refs acrosslearn/, README,apps/portal/*.mjs,apps/legit,buildScripts/,tree.json.apps/portal/llms.txt/apps/portal/sitemap.xmlstill hold stale flat moved-page URLs at this head (benefits/ApplicationEngine, …), correctly uncommitted per guide-authoring §5.generate.mjsat head (reading the updatedtree.json) emits the new ids to stdout —benefits/brain/AIEngineeringTeam,benefits/body/ApplicationEngine, … with 0 flat moved-page refs in the output — so the next data-sync regen overwrites the stale committed outputs. (Independently reproduced at headb3636000; matches the cross-family review's generator verification.)check-whitespace,check-shorthand,check-jsdoc-types,check-block-alignmentall green.Merge contract (hard gate — addresses review RA2)
Resolves #14312closes only when both land: this PR (the brain/body tree structure — #14312's headline AC) and #14546 (which carries the deadbenefits/WhatIsNeo→benefits/IntroductionREADME:128 fix — a #14470-removed-file ref that #14312's validated sweep names, per issue commentIC_kwDODSospM8AAAABIr3gVg).README.md:128still shows the deadbenefits/WhatIsNeolink; docs(portal): 6 Agent OS boxes on the landing page + README link fix (#14545) #14546 fixes exactly that line. docs(benefits): nested-folder tree structure + reading-order WEIGHTING (Start-Here / Brain / Body) #14312 must not close with that link live (the false-close, one level down).benefits/<page>→brain|bodymove refs; docs(portal): 6 Agent OS boxes on the landing page + README link fix (#14545) #14546: line 128 WhatIsNeo). Gate, don't absorb.dev(disposable merge sim ontodevd2f3ad4170was clean per review). First data-sync after merge self-heals the SEO outputs — no manual step.Post-Merge Validation
Introduction·Architecture Overview· The Brain (6) · The Body (13); no dead routes.Resolves #14312cannot land while README:128's dead front-door link is live.Authored by Grace (@neo-opus-grace, Claude Opus 4.8).