diff --git a/.deploy/mta.yaml b/.deploy/mta.yaml index 9bc46237b..9708446d6 100644 --- a/.deploy/mta.yaml +++ b/.deploy/mta.yaml @@ -10,7 +10,7 @@ ID: tutorials-ims # Bump this on each release you deploy — it's the version shown by `cf mtas` # and in the mtar filename (tutorials-ims_.mtar). Deploy is manual: # `cd .deploy && mbt build && cf deploy mta_archives/tutorials-ims_.mtar -e ../deploy/.mtaext -f`. -version: 1.44.0 +version: 1.44.1 # Top-level parameters (overridable per-env via deploy/.mtaext). parameters: diff --git a/CLAUDE.md b/CLAUDE.md index 49dfaa6ed..755f2ea74 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -152,7 +152,7 @@ The load-bearing few. **Full detail for every relocated item → [tutorials-ims- - **`@cap-js/ai` for RPT-1 ValueList recommendations (#959)** — `AICore-mocked` locally, `AICore-btp` in hybrid/prod; per-field opt-out `@UI.RecommendationState: 0`. Ref: [cap-ai-plugin.md](docs/developers/reference/cap-ai-plugin.md). - **Knowledge-graph feature flags (`KG_*`), all default OFF + DEV-only, all fail-open** — PageRank #916, WCC isolation #918, on-demand extraction #948, Louvain communities #917, orphan retirement #1115, community peers/labels #1126, community search weight #1171, coverage nudge #1172, cluster Q&A #1173. Toggles, nightly jobs, and fail-open specifics → gotchas.md "Knowledge graph feature flags". - **HCQL protocol adapter (#2247, CAP 10.1.0)** — `POST /hcql/` paths (NOT co-mounted on OData URLs) on 5 authenticated services only (`AdminService`, `AuthorService`, `AnalyticsService`, `ExportsService`, `ConsolidationService`); requires `@sap/cds >= 10.1.0`. Malformed-CQN process-exit DoS from 10.0.3 **fixed in 10.1.0** (now returns 400, server stays up). Auth enforced before CQN parse. Kill: delete `srv/hcql-enablement.cds` + drop the `hcql` entry from `AdminService`'s `@protocol` list in `srv/admin-service-mcp.cds`, then `cds build --production` + redeploy. Ref: [hcql-support.md](docs/developers/reference/hcql-support.md). -- **cds-caching CDS-DB store + metrics ON (#1222)** — `store:"cds"` in hybrid/prod, `memory` in base/unit. CF resolve-guard needs BOTH the baked csn entities AND `srv/lib/strip-precompiled-plugin-roots.js` (both load-bearing). Ref: [cds-caching-store.md](docs/developers/reference/cds-caching-store.md). +- **cds-caching CDS-DB store + metrics ON (#1222)** — `store:"cds"` in hybrid/prod, `memory` in base/unit. CF resolve-guard needs BOTH the baked csn entities AND `srv/lib/strip-precompiled-plugin-roots.js` (both load-bearing). A committed `patch-package` patch (`patches/cds-caching+2.1.0.patch`, `postinstall`) makes `KeyvCDS.set` an atomic UPSERT to kill PROD PK-collision error noise on concurrent `kg:rl:*` writes (#3102); **re-verify the patch on any `cds-caching` bump** (patch-package fails install loudly on drift). Ref: [cds-caching-store.md](docs/developers/reference/cds-caching-store.md). - **User-facing UI changes want a committed e2e spec** — advisory PR nudge on `app/**`/`hugo/**` changes; real coverage runs in the post-DEV-deploy `e2e` job. Ref: [e2e-coverage-pattern.md](docs/developers/reference/e2e-coverage-pattern.md). - **`test:e2e` is post-deploy only, not on PRs** — self-skips without `SMOKE_BASE_URL`. Served tutorials render `
`+`

`, NOT `
`. Runbook: `test/e2e/README.md`. - **Freshness detector grounding needs the corpus-embedding backfill** — until `srv/jobs/freshness-corpus-embedding-job.js` runs, every API-obsolescence claim degrades to `confidence: Low`. Tutorial source from `ContentFiles.sourceContent` via `getTutorialSource(slug)`, NOT `Steps.description`. → gotchas.md "Freshness detector". diff --git a/docs/developers/reference/cds-caching-store.md b/docs/developers/reference/cds-caching-store.md index b325a8634..4aaaeea95 100644 --- a/docs/developers/reference/cds-caching-store.md +++ b/docs/developers/reference/cds-caching-store.md @@ -356,3 +356,62 @@ command-palette concept search): `@cache: { ttl: 300000, tags: [{ value: re-enabled metrics actually accumulate against the deployed DEV container (clear the stale zeroed rows first), then let PROD traffic post-cutover give the baseline before annotating more surfaces. + +## `KeyvCDS.set` UPSERT patch (#3102) + +**Symptom (PROD, 2026-11-11):** recurring `level:"error"` lines from the +`cds-caching` logger: + +``` +KeyvCDS set error for key kg:rl:api:auth:auth:: unique constraint violated: +Table(PLUGIN_CDS_CACHING_CACHESTORE), Index(_SYS_TREE_CS_#..._#P0) ... key: ID=kg:rl:... +``` + +Two namespaces seen — both the KG rate-limiter (`srv/lib/rate-limit/shared-limiter.js`, +key prefix `kg:rl:`): `kg:rl:api:auth:auth:` and `kg:rl:homepage:anon:`. + +**Root cause:** the plugin's CDS-DB store adapter, +`node_modules/cds-caching/lib/support/KeyvCDS.js`, implemented `set()` as a +**read-then-create/update** (TOCTOU): + +```js +const existing = await db.read(entity, key); +if (existing) { await db.update(entity, key).with({ value, expiresAt }); } +else { await db.create(entity, { ID: key, value, expiresAt }); } +``` + +Under multi-instance (HA) CF, two instances both read "no row" for the same +`kg:rl:*` counter, both take the `else` branch, and the second `create` violates +the `CacheStore` PK unique index. The adapter's own `catch` logs at **error** +level and swallows the exception — so the collision never reaches our +`shared-limiter.js` fail-open (the limiter already over-allows under races, which +is acceptable), but it spams PROD error monitoring and the losing write is +dropped. + +**Fix:** a committed **patch-package** patch, +`patches/cds-caching+2.1.0.patch`, replaces that branch with a single atomic +UPSERT: + +```js +await db.run(cds.ql.UPSERT.into(entity).entries({ ID: key, value, expiresAt })); +``` + +`CacheStore` has a single-column PK (`ID : String(900)`), so UPSERT compiles to +HANA `UPSERT ... WITH PRIMARY KEY` and to SQLite `INSERT OR REPLACE` — +last-write-wins, no read-then-write race, no unique-constraint collision. The +patch re-applies on every `npm ci` / `npm install` via the `postinstall` hook +(`patch-package`). Regression test: +`srv/lib/__tests__/keyv-cds-upsert.test.js` (drives the real `KeyvCDS` against an +in-memory CAP fixture, modelling the TOCTOU window deterministically). + +**Upstream:** the same read-then-insert is still present on `cds-caching` `main`. +Filed upstream as +[mikezaschka/cds-caching#44](https://github.com/mikezaschka/cds-caching/issues/44) +(issue) + +[mikezaschka/cds-caching#45](https://github.com/mikezaschka/cds-caching/pull/45) +(PR). **Drop condition:** once a published `cds-caching` release carries the +upstream fix and we adopt it, delete `patches/cds-caching+2.1.0.patch`. +**Re-verify the patch on any `cds-caching` version bump** — `patch-package` +fails the install loudly if the target file drifted, so the drift surfaces at +`npm ci`, not silently. + diff --git a/docs/superpowers/plans/2026-11-11-cds-caching-upsert-fix.md b/docs/superpowers/plans/2026-11-11-cds-caching-upsert-fix.md new file mode 100644 index 000000000..6e8317b54 --- /dev/null +++ b/docs/superpowers/plans/2026-11-11-cds-caching-upsert-fix.md @@ -0,0 +1,231 @@ +# cds-caching UPSERT Fix (#3102) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Stop `cds-caching` from logging `error`-level unique-constraint-violation noise in PROD when two HA instances concurrently write the same cache key (notably `kg:rl:*` rate-limiter counters), by making the `KeyvCDS.set()` write path an atomic single-key UPSERT. + +**Architecture:** The defect lives in the vendored `cds-caching` plugin (`node_modules/cds-caching/lib/support/KeyvCDS.js`), whose `set()` does read-then-`create`/`update` — a TOCTOU race that collides on the `CacheStore` PK under multi-instance CF. We fix it two ways: (1) **locally and immediately** via a committed `patch-package` patch against our pinned `cds-caching@2.1.0`, replacing the racy branch with `UPSERT.into(entity).entries(...)` (last-write-wins, no collision); (2) **upstream** by filing an issue + PR against `mikezaschka/cds-caching` so the patch can eventually be dropped. The bug is confirmed still present on upstream `main`. + +**Tech Stack:** Node.js 22, `@sap/cds` (CQL `UPSERT`), `cds-caching@2.1.0`, `patch-package`, HANA (prod/hybrid) / SQLite (unit). + +**Spec:** GitHub issue [sap-tutorials/tutorials-ims#3102](https://github.com/sap-tutorials/tutorials-ims/issues/3102) + +## Global Constraints + +- `cds-caching` is pinned at exactly `"2.1.0"` in `package.json` dependencies — the patch filename and `patch-package` match this version verbatim; do **not** bump it as part of this fix. +- Node engine is pinned `22.23.4`; `patch-package` must run under it. +- No raw SQL — use CQL `UPSERT` (`@sap/cds`), consistent with the CAP constraint in CLAUDE.md. +- The `CacheStore` entity is single-PK: `key ID : String(900); value : LargeString; expiresAt : Int64;`. UPSERT semantics are only safe here because the key is a single PK column (composite-key UPSERT on HANA via CDS QL is a known gotcha — not applicable). +- The fix must remain **fail-open**: `set()` must still never throw to its caller (keep the existing `try/catch` returning normally). +- `patch-package` must re-apply automatically on `npm ci` / `npm install` (CI uses `npm ci`), via a `postinstall` hook, without breaking the existing `prepare` script. +- The MCP MTA (`tutorials-mcp`) and QA app reuse the same `srv/lib` tree but **not** `node_modules` patching concerns — no `srv-qa` cp-list change is needed (no new `srv/lib/` file). + +## Review Focus + +- **npm script collision:** the repo has `prepare` (git hooks) but no `postinstall`. Adding `postinstall: patch-package` must not clobber `prepare`; both must run on install. — covered in Task 2 (verify `npm ci` output shows both hooks firing). +- **Patch fails to apply after a future `cds-caching` bump:** `patch-package` errors (non-zero) if the target file drifted. Task 2 pins the exact `2.1.0` and documents the drop-on-upgrade path; the drift surfaces loudly at install, not silently. +- **UPSERT on SQLite (unit profile):** unit tests use in-memory SQLite with `store:"memory"` (not `cds`), so KeyvCDS is never exercised there; the patch's UPSERT path is only hit under `store:"cds"`. Task 1 adds a direct KeyvCDS test against an in-memory SQLite db to prove UPSERT works on *both* dialects and that a double-set of the same key does not throw. +- **`value`/`expiresAt` null handling:** a `set` with no TTL stores `expiresAt: null`; UPSERT must preserve that (not drop the column). Task 1 asserts a re-set with a shorter/absent TTL overwrites cleanly. +- **Concurrent set does not lose the row entirely:** after two racing sets, the key must still be readable (one winner), never left absent. Task 1 asserts the key exists post-double-set. + +--- + +### Task 1: patch-package patch — atomic UPSERT in `KeyvCDS.set()` (+ regression test) + +**Files:** +- Create: `patches/cds-caching+2.1.0.patch` (generated by `patch-package`, not hand-written) +- Modify (as patch source): `node_modules/cds-caching/lib/support/KeyvCDS.js` — the `set(key, value, ttl)` method body +- Test: `srv/lib/__tests__/keyv-cds-upsert.test.js` + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: a committed `patches/cds-caching+2.1.0.patch` that Task 2 wires into install; the behavioral guarantee that `KeyvCDS.set()` is collision-free and still never throws. + +- [ ] **Step 1: Write the failing test** + +Create `srv/lib/__tests__/keyv-cds-upsert.test.js`. It instantiates the real `KeyvCDS` class from `cds-caching`, backed by an in-memory SQLite `cds.db` with the `plugin.cds_caching.CacheStore` entity deployed, and asserts the collision-safe contract: + +```js +import { describe, it, expect, beforeAll } from 'vitest'; +import cds from '@sap/cds'; +import KeyvCDS from 'cds-caching/lib/support/KeyvCDS.js'; + +describe('KeyvCDS.set is collision-safe (issue #3102)', () => { + beforeAll(async () => { + // deploy only the cache-store entity into an in-memory sqlite db + cds.model = await cds.load([require.resolve('cds-caching/db/cache-store.cds')]); + await cds.deploy(cds.model).to('sqlite::memory:'); + }); + + it('a second set of the same key does NOT throw and overwrites (last-write-wins)', async () => { + const store = new KeyvCDS({}); + await store.set('dup', { count: 1 }, 10_000); + await expect(store.set('dup', { count: 2 }, 10_000)).resolves.not.toThrow(); + expect(await store.get('dup')).toEqual({ count: 2 }); + }); + + it('two concurrent sets of a fresh key both resolve and the key is present', async () => { + const store = new KeyvCDS({}); + await Promise.all([store.set('race', { a: 1 }, 10_000), store.set('race', { a: 2 }, 10_000)]); + expect(await store.get('race')).toBeDefined(); + }); + + it('re-set with no TTL overwrites cleanly (expiresAt null)', async () => { + const store = new KeyvCDS({}); + await store.set('nt', 'first', 10_000); + await store.set('nt', 'second'); // no ttl + expect(await store.get('nt')).toBe('second'); + }); +}); +``` + +The decisive assertion is that a simulated collision does not surface. (On SQLite a true PK re-INSERT would throw `UNIQUE constraint failed`; the pre-patch read-then-create code path can hit it under interleaving. Post-patch UPSERT never does.) + +- [ ] **Step 2: Run the test against the UNPATCHED plugin to see it is not yet collision-proof** + +Run: `npm test -- srv/lib/__tests__/keyv-cds-upsert.test.js` +Expected: the concurrent-set / double-set assertions fail or emit the `KeyvCDS set error` log (pre-patch read-then-create path). If the unpatched code happens to pass serially, note it — the patch's value is proven by the HANA PK semantics; keep the test as the post-patch regression guard. + +- [ ] **Step 3: Edit `node_modules/cds-caching/lib/support/KeyvCDS.js`** + +Replace the body of `async set(key, value, ttl)` so the read-then-`create`/`update` branch becomes a single atomic UPSERT, keeping the outer `try/catch` (fail-open, no re-throw) intact: + +```js +async set(key, value, ttl) { + try { + const { db, entity } = await this._getDB(); + if (!db || !entity) return; + const expiresAt = ttl ? Date.now() + ttl : null; + // Atomic last-write-wins. CacheStore has a single-column PK (ID), + // so UPSERT compiles to HANA `UPSERT ... WITH PRIMARY KEY` and to + // SQLite `INSERT OR REPLACE` — no read-then-write race, no unique + // constraint collision under multi-instance concurrency (#3102). + await db.run(UPSERT.into(entity).entries({ ID: key, value, expiresAt })); + } catch (error) { + this.log.error(`KeyvCDS set error for key ${key}:`, error); + } +} +``` + +`UPSERT` is a CQL global provided by `@sap/cds`; `cds` is already required at the top of the file (`const cds = require('@sap/cds')`), and `UPSERT` is exposed as a global by the cds runtime. If `UPSERT` is not in scope in this module, reference it as `cds.ql.UPSERT`. Confirm which form resolves before generating the patch. + +- [ ] **Step 4: Run the test to verify it passes against the patched plugin** + +Run: `npm test -- srv/lib/__tests__/keyv-cds-upsert.test.js` +Expected: PASS, no `KeyvCDS set error` log emitted. + +- [ ] **Step 5: Generate the patch file** + +Run: `npx patch-package cds-caching` +Expected: creates `patches/cds-caching+2.1.0.patch` containing only the `set()` diff. Inspect it to confirm it touches exactly that method and nothing else. + +- [ ] **Step 6: Commit** + +```bash +git add patches/cds-caching+2.1.0.patch srv/lib/__tests__/keyv-cds-upsert.test.js +git commit -m "fix(cds-caching): atomic UPSERT in KeyvCDS.set to stop PK-collision error noise (#3102)" +``` + +--- + +### Task 2: Wire `patch-package` into install + +**Files:** +- Modify: `package.json` — add `patch-package` to `devDependencies`, add/extend a `postinstall` script. + +**Interfaces:** +- Consumes: `patches/cds-caching+2.1.0.patch` from Task 1. +- Produces: automatic patch re-application on `npm ci` / `npm install`. + +- [ ] **Step 1: Add the dev dependency** + +Run: `npm install --save-dev patch-package` +Expected: `patch-package` appears in `devDependencies`. (It also pulls its own deps; confirm the install succeeds under Node 22.) + +- [ ] **Step 2: Add the `postinstall` hook** + +In `package.json` `scripts`, add: +```json +"postinstall": "patch-package" +``` +Leave the existing `prepare` script untouched. (npm runs `postinstall` then `prepare` on a bare `npm install`; both fire.) + +- [ ] **Step 3: Verify the patch applies from a clean install** + +Run: `npm ci` +Expected: install output includes a `patch-package` line applying `cds-caching@2.1.0` cleanly; `node_modules/cds-caching/lib/support/KeyvCDS.js` contains the UPSERT edit afterward. Confirm with: `grep -c "UPSERT.into" node_modules/cds-caching/lib/support/KeyvCDS.js` → `1`. + +- [ ] **Step 4: Re-run the regression test post-`npm ci`** + +Run: `npm test -- srv/lib/__tests__/keyv-cds-upsert.test.js` +Expected: PASS (proves the patch survives a real install cycle). + +- [ ] **Step 5: Commit** + +```bash +git add package.json package-lock.json +git commit -m "build: wire patch-package postinstall for cds-caching UPSERT patch (#3102)" +``` + +--- + +### Task 3: Document the patch + upgrade path + +**Files:** +- Modify: `docs/developers/reference/cds-caching-store.md` — add a short "KeyvCDS.set UPSERT patch (#3102)" section. +- Modify: `CLAUDE.md` — one line in the cds-caching gotcha bullet noting the committed patch and that it must be re-verified on any `cds-caching` bump. + +**Interfaces:** +- Consumes: the patch + wiring from Tasks 1–2. +- Produces: operator-facing record of why the patch exists and how to drop it. + +- [ ] **Step 1: Add the reference-doc section** + +Append to `docs/developers/reference/cds-caching-store.md` a section explaining: the PROD symptom (`error`-level `unique constraint violated` on `PLUGIN_CDS_CACHING_CACHESTORE` from concurrent `kg:rl:*` writes), the root cause (read-then-insert TOCTOU in `KeyvCDS.set`), the fix (committed `patches/cds-caching+2.1.0.patch` → atomic UPSERT), the upstream issue/PR links (Task 4), and the drop condition (patch becomes unnecessary once a `cds-caching` release carrying the upstream fix is adopted — re-verify/remove the patch on any version bump). + +- [ ] **Step 2: Add the CLAUDE.md note** + +Extend the existing `cds-caching CDS-DB store` gotcha bullet with: a committed `patch-package` patch (`patches/cds-caching+2.1.0.patch`) makes `KeyvCDS.set` an atomic UPSERT to kill PROD PK-collision error noise (#3102); re-verify the patch on any `cds-caching` upgrade. + +- [ ] **Step 3: Commit** + +```bash +git add docs/developers/reference/cds-caching-store.md CLAUDE.md +git commit -m "docs(cds-caching): record KeyvCDS.set UPSERT patch + upgrade path (#3102)" +``` + +--- + +### Task 4: Upstream the fix (issue + PR to mikezaschka/cds-caching) + +**Files:** none in this repo (external repo). This task produces links recorded in Task 3's docs — run Task 4 before Task 3 Step 1 if you want the links inline, or update the doc afterward. + +**Interfaces:** +- Consumes: the diagnosis + UPSERT diff from Task 1 (the upstream patch is the same edit applied to `main`, where `set()` is byte-identical). +- Produces: an upstream issue URL and PR URL. + +- [ ] **Step 1: File the upstream issue** + +Against `mikezaschka/cds-caching`: title ≈ "KeyvCDS.set read-then-insert races on PK under multi-instance → unique-constraint errors". Body: the HA race, the `PLUGIN_CDS_CACHING_CACHESTORE` collision, that `set()` on `main` still does read-then-`create`/`update`, and the proposed atomic-UPSERT fix. Confirm no existing open issue covers it first. + +- [ ] **Step 2: Open the upstream PR** + +Fork `mikezaschka/cds-caching`, branch, apply the same `set()` UPSERT edit to `lib/support/KeyvCDS.js` on `main`, and open a PR referencing the issue. Match upstream code style (4-space indent, the existing comment tone). Include a one-line note that single-PK `CacheStore` makes UPSERT safe on HANA + SQLite + Postgres. + +- [ ] **Step 3: Record the links** + +Paste the issue + PR URLs into the Task 3 reference-doc section and the local #3102 PR description. + +--- + +### Task 5: Open the tutorials-ims PR + +**Files:** none (git/PR operation). + +- [ ] **Step 1: Push the branch and open a DEV-targeted PR** + +Per repo convention, PRs target `DEV` (not `main`). Push `worktree-fix-3102-cds-caching-upsert` and `gh pr create --base DEV`. PR body: link #3102, summarize root cause + the two-pronged fix (local patch + upstream issue/PR), link the upstream URLs, and note the patch's drop condition. + +- [ ] **Step 2: Verify the full unit suite is green in CI** + +The full `npm test` suite is hook-blocked locally; confirm the CI unit job passes on the PR before requesting review. diff --git a/docs/superpowers/plans/2026-11-11-concept-page-layout.md b/docs/superpowers/plans/2026-11-11-concept-page-layout.md new file mode 100644 index 000000000..a15424a04 --- /dev/null +++ b/docs/superpowers/plans/2026-11-11-concept-page-layout.md @@ -0,0 +1,236 @@ +# Concept / topic / channel page layout fixes — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix flush-left rendering on four CAP/Hugo page types and cap the hundreds-of-tiles relationship walls on concept hub pages (issue #3095). + +**Architecture:** Two independent changes. (A) One grouped CSS base rule (desktop + mobile media query) in the git-tracked CSS *source*, giving `.concept-page`, `.topic-detail`, `.channel-detail`, `.channels-page` the site-standard 1280px/centered/padded container. (B) An EJS change in `concept-detail.ejs` that moves KG-relationship tile overflow (past N) into a no-JS `
` collapse. + +**Tech Stack:** CSS (`hugo/assets/css/sap-fundamental.src.css`, compiled by postcss `build:css`), EJS (`srv/lib/templates/concept-detail.ejs` via `srv/lib/concept-detail-render.js`), vitest unit tests. + +**Spec:** `docs/superpowers/specs/2026-11-11-concept-page-layout-design.md` + +## Global Constraints + +- **CSS edits go ONLY in `hugo/assets/css/sap-fundamental.src.css`** (git-tracked source). NEVER edit `hugo/assets/css/sap-fundamental.css` (gitignored build output since #2896; editing it causes Hugo-fingerprint drift → blocked publish). +- **Container values copied verbatim from the sibling rule:** desktop `max-width: 1280px; margin: 0 auto; padding: 1.5rem 2rem 3rem;` (`src.css:2402-2407`); mobile `padding: 1rem;` inside the existing `@media` block (`src.css:2774-2777`). +- **No new global container class and no `main { … }` rule** — scope strictly to the four named root classes. +- **Cap mechanism is CSS-only `
`/``** — no island, no JS, overflow moved server-side (not hidden client-side). +- **Cap scope = KG-relationship sections only** (`teaches`, `requires`, `requiredBy`, `relatedTo`); leave phase-4 external sections (learningJourneys, helpDocs, blogPosts, discoveryMissions, videos, apiDocs, samples, communityEvents) uncapped — corpus data shows only these grow unbounded. +- **Cap threshold N = 24.** +- **Test runner:** `npx vitest run --project unit ` (scoped — full suite is hook-blocked locally). +- **Windows worktree:** CSS/EJS files are LF. After edits run `file `; if it flips to CRLF, normalize before commit. +- **Activation is two-step:** the CSS fix (Task 1) activates on deploy; the EJS cap (Task 2) changes body bytes → needs a concepts re-publish after deploy. Note this in the PR body. One PR. + +## Review Focus + +- **Concept with zero `requiredBy` but non-empty other sections** — the cap helper must still render normal sections untouched and emit no empty `
`. Covered in Task 2 tests. +- **`requiredBy.length` exactly === N (24)** — boundary: no `
` should appear (nothing overflows). Covered in Task 2 tests. +- **`requiredBy.length` === N+1 (25)** — boundary: exactly one item inside `
`, summary count correct. Covered in Task 2 tests. +- **Title values needing HTML-escape inside the collapsed overflow** — escaping must hold for items rendered in the `
` half, not just the visible half. Covered in Task 2 tests. +- **Concept index / topic index already-padded pages** — the new grouped rule must not double-apply to `.concepts-index`/`.topics-index` (different classes; verify by inspection that the selector list contains only the four intended classes). Covered in Task 1 review. + +--- + +## Task 1: Container base rule for the four flush-left roots (CSS) + +**Files:** +- Modify: `hugo/assets/css/sap-fundamental.src.css` (add desktop rule near `:3069` `.concept-page__*` block; add the four classes to the mobile `@media` block at `:2774-2777`) +- Test: `test/concept-page-container.test.js` (create — source-string guard, same style as `test/hugo-css-fingerprint.test.js`) + +**Interfaces:** +- Produces: CSS classes `.concept-page`, `.topic-detail`, `.channel-detail`, `.channels-page` each resolving to `max-width:1280px; margin:0 auto; padding:1.5rem 2rem 3rem` on desktop and `padding:1rem` under the mobile media query. No JS/HTML interface. + +- [ ] **Step 1: Write the failing test** + +Create `test/concept-page-container.test.js` (vitest, source-string guard — reads `sap-fundamental.src.css` as text, mirroring `hugo-css-fingerprint.test.js`): + +```js +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +const src = readFileSync(join(import.meta.dirname, '..', 'hugo/assets/css/sap-fundamental.src.css'), 'utf8'); + +describe('#3095 — concept/topic/channel container base rule', () => { + const ROOTS = ['.concept-page', '.topic-detail', '.channel-detail', '.channels-page']; + it('declares a desktop container rule with 1280px/centered/padded for all four roots', () => { + // one grouped rule block containing all four selectors + the sibling values + const rule = src.match(/\.concept-page,\s*\.topic-detail,\s*\.channel-detail,\s*\.channels-page\s*\{[^}]*\}/); + expect(rule, 'grouped base rule present').not.toBeNull(); + expect(rule[0]).toMatch(/max-width:\s*1280px/); + expect(rule[0]).toMatch(/margin:\s*0 auto/); + expect(rule[0]).toMatch(/padding:\s*1\.5rem 2rem 3rem/); + }); + it('adds the four roots to the mobile padding override', () => { + // the four classes appear alongside .group-body inside a rule setting padding:1rem + for (const r of ROOTS) { + expect(src, `${r} in a 1rem padding rule`).toMatch( + new RegExp(`${r.replace('.', '\\.')}[\\s\\S]{0,400}padding:\\s*1rem`) + ); + } + }); + it('does NOT introduce a bare `main {` width rule or touch the index classes', () => { + expect(src).not.toMatch(/\.concept-page,\s*\.topic-detail,\s*\.channel-detail,\s*\.channels-page,\s*\.concepts-index/); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run --project unit test/concept-page-container.test.js` +Expected: FAIL (no grouped rule yet). + +- [ ] **Step 3: Add the CSS rules in `sap-fundamental.src.css`** + +Desktop: insert a grouped rule immediately before the first `.concept-page__*` rule (≈`:3069`): +```css +.concept-page, +.topic-detail, +.channel-detail, +.channels-page { + max-width: 1280px; + margin: 0 auto; + padding: 1.5rem 2rem 3rem; +} +``` +Mobile: in the existing `@media` block containing `.mission-body, .group-body { padding: 1rem; }` (`:2774-2777`), add the four selectors to that same declaration (or a sibling declaration in the same media block) so they also get `padding: 1rem`. + +- [ ] **Step 4: Run the test + verify CSS still compiles** + +Run: `npx vitest run --project unit test/concept-page-container.test.js` +Expected: PASS. +Run: `npm run build:css` +Expected: exits 0, regenerates `sap-fundamental.css` (gitignored — do not stage it). + +- [ ] **Step 5: Verify line endings, then commit** + +Run: `file hugo/assets/css/sap-fundamental.src.css test/concept-page-container.test.js` (expect "ASCII text", not "CRLF"); normalize if flipped. +```bash +git add hugo/assets/css/sap-fundamental.src.css test/concept-page-container.test.js +git commit -m "fix(#3095): add 1280px container base rule for concept/topic/channel/channels page roots" +``` + +--- + +## Task 2: Cap KG-relationship tile walls with a no-JS `
` collapse (EJS) + +**Files:** +- Modify: `srv/lib/concept-detail-render.js` (add a cap helper into the template context) +- Modify: `srv/lib/templates/concept-detail.ejs` (render the four KG sections' `
    ` via the helper) +- Test: `test/concept-detail-cap.test.js` (create) + +**Interfaces:** +- Consumes: `renderConceptDetail(concept, phase4)` from `srv/lib/concept-detail-render.js:52` → returns `{ body, contentHash }`. +- Produces: a template-context helper `renderCappedGrid(items, renderItem, { visible = 24, label })` → string of `
      ` markup where the first `visible` items are direct `
    • ` children and any overflow is wrapped in `
      Show all …
        …rest…
      `. `renderItem(item)` returns the `
    • …
      • ` for one item (reuses the existing per-section markup). When `items.length <= visible`, no `
      ` is emitted. + +- [ ] **Step 1: Write the failing tests** + +Create `test/concept-detail-cap.test.js` (vitest). Build a concept with N-boundary `requiredBy` arrays and assert on the returned `body` string: + +```js +import { describe, it, expect } from 'vitest'; +import { renderConceptDetail } from '../srv/lib/concept-detail-render.js'; +const mk = (n) => Array.from({ length: n }, (_, i) => ({ slug: `c${i}`, title: `Concept ${i}` })); +const base = { slug: 'x', name: 'X', description: '' }; + +describe('#3095 — KG relationship cap', () => { + it('no
      when requiredBy.length === 24 (boundary)', () => { + const { body } = renderConceptDetail({ ...base, requiredBy: mk(24) }, {}); + const sec = body.match(/Concepts that build on this[\s\S]*?<\/section>/)[0]; + expect(sec).not.toMatch(/
      /g) || []).length).toBe(24); + }); + it('one item inside
      when requiredBy.length === 25 (boundary)', () => { + const { body } = renderConceptDetail({ ...base, requiredBy: mk(25) }, {}); + const sec = body.match(/Concepts that build on this[\s\S]*?<\/section>/)[0]; + expect(sec).toMatch(/
      /)[0]; + expect((inside.match(/
    • /g) || []).length).toBe(1); + expect(sec).toMatch(/Show all 25/); + }); + it('235-item hub collapses overflow (24 visible, 211 in details)', () => { + const { body } = renderConceptDetail({ ...base, requiredBy: mk(235) }, {}); + const sec = body.match(/Concepts that build on this[\s\S]*?<\/section>/)[0]; + const inside = sec.match(//)[0]; + expect((inside.match(/
    • /g) || []).length).toBe(211); + }); + it('escapes titles rendered inside the collapsed overflow half', () => { + const items = mk(25); items[24].title = ''; + const { body } = renderConceptDetail({ ...base, requiredBy: items }, {}); + expect(body).not.toMatch(/'; + const { body } = renderConceptDetail({ ...base, requiredBy: items }, {}); + expect(body).not.toMatch(/