---
number: 10078
title: 'test: close AC-checklist coverage gaps in ConceptService.spec (#10033)'
author: tobiu
state: MERGED
createdAt: '2026-04-19T08:28:01Z'
updatedAt: '2026-04-19T09:00:52Z'
closedAt: '2026-04-19T09:00:46Z'
mergedAt: '2026-04-19T09:00:46Z'
head: agent/10033-concept-graph-unit-test-completeness
base: dev
url: 'https://github.com/neomjs/neo/pull/10078'
---
## Summary

Closes the four named-AC gaps in #10033 by strengthening two existing tests and adding two new targeted tests to `test/playwright/unit/ai/services/ConceptService.spec.mjs`. All assertions are cited with explicit `AC #10033:` markers in-code so future readers can trace the contract mapping without cross-referencing this PR.

## Context

PR #10048 (ticket #10032) shipped `ConceptService.mjs` + 16 Playwright tests at `test/playwright/unit/ai/services/ConceptService.spec.mjs`. PR #10052 extended the suite via #10049 (aliases + ANALOGOUS_TO edges + production-graph validation) bringing the spec to 21 tests. A memory-mining sweep against the Memory Core surfaced session `f76d5307` confirming #10032's test coverage, which reframed #10033 from "write the whole suite" to "close the named-AC coverage gaps."

Concretely, four of #10033's acceptance-criteria items were covered implicitly but not asserted by name:

| AC item | Prior state | New assertion |
|---|---|---|
| "Root nodes are Tier 1 concepts" | Only `tree.tier === 0` on anchor | `tree.children.every(c => c.tier === 1)` |
| "Coverage annotations are correct" | Asserted `✅` and `❌` appear somewhere | Asserts they appear on the **correct per-concept line** (threading → ✅, vdom → ❌) |
| "Output under 100 lines for Tier 1-2" | Not asserted | New test: `serializeForLLM(2)` on production graph ≤ 100 lines |
| "Tier 1 > Tier 3 in weight" | Tier 1 vs Tier 1 (gap-bumped) comparison only | New test: tier-1 node vs tier-3 node at identical coverage; asserts `tier1Weight > tier3Weight` |

## Gold Standards Leveraged / Traps Avoided

- **Gold Standard (memory-mining):** invoked the new `memory-mining` skill (#10075, landed as commit `5cf7ddcf5`) before starting — surfaced session `f76d5307` and saved ~15min of re-deriving the #10032/#10049 coverage baseline.
- **Gold Standard (existing fixture):** reused the `createMinimalFixture` helper where possible; added a small inline fixture only for the tier-1 vs tier-3 weight test where the minimal fixture had no tier-3 node.
- **Trap avoided:** rewriting the spec or moving it to a different location (#10033's ticket body references `test/playwright/unit/ai/graph/...` but the actual canonical path is `test/playwright/unit/ai/services/...` per #10032's `ai/services/ConceptService.mjs` placement — memory `aae16459` confirms tobi's architectural correction during #10032). The ticket path is stale; the file location is correct as-is.
- **Trap avoided:** bundling #10033 and #10035 into one PR. #10035 (GapInferenceEngine refactor) is load-bearing architecture work — keeping #10033 as a clean checklist-closure unblocks #10035's own clean branch and PR.

## Implementation Footprint

`test/playwright/unit/ai/services/ConceptService.spec.mjs` — +56 lines / −3 lines.

- Test `getConceptTree should build a nested tree from the anchor` → added Tier-1 root-children assertion
- Test `serializeForLLM should produce tree with connector characters` → strengthened marker-placement assertions
- **New test:** `serializeForLLM Tier 1-2 from production graph should stay under 100 lines`
- **New test:** `calculateWeight should order tier 1 above tier 3 at equal coverage`

## Acceptance Criteria (from #10033)

- [x] All tests pass via `npm run test-unit -- test/playwright/unit/ai/services/ConceptService.spec.mjs`
- [x] Tests use fixture JSONL data, no live graph dependency (production-graph tests are additive bonus coverage and explicitly marked)

All seven named-checklist items from #10033 now have explicit named-test coverage.

## Test Plan

```bash
npm run test-unit -- test/playwright/unit/ai/services/ConceptService.spec.mjs
```

Expected: **23 passed** (21 prior + 2 new). Verified locally — 656ms runtime.

## Origin Session ID

`62d6f155-e57f-4279-9b59-36c9e4ecbc5e` — session 3 continuing from the #10075 merge

## Related

- #10030 — parent epic (Concept Ontology & Semantic Gap Inference)
- #10032 / PR #10048 — shipped the 16 base tests
- #10049 / PR #10052 — added aliases + ANALOGOUS_TO tests
- #10035 — next sub-issue in the chain (Graph Integration & GapInferenceEngine Refactor), unblocked by this PR

Resolves #10033

## Comments

### `@tobiu` commented on 2026-04-19T08:33:48Z

**Input from Claude Opus 4.7 (Claude Code):**

> ❋ # PR Review Summary
> 
> **Status:** Approved (pending human merge)
> 
> **Self-Review Opening:** Self-review of #10033. This implementation chose to strengthen/extend the existing spec over rewriting it, and chose a narrow AC-closure scope over bundling #10033+#10035 into one PR. Rationale: memory-mining surfaced that PR #10048 and PR #10052 had already delivered 21 of the 23 eventual tests; closing the 4 named-AC gaps in place preserves the block-chain provenance and keeps #10035's load-bearing refactor isolated on its own branch. Gaps and honest soft-deductions noted below.
> 
> ---
> 
> ### 📊 Evaluation Metrics
> *   **`[ARCH_ALIGNMENT]`**: 95 — Follows project test-file conventions (`test/playwright/unit/ai/services/` canonical path per #10032's `ai/services/` architectural correction); reuses existing `createMinimalFixture` helper; `test:` (no scope) commit subject per git log precedent (`f6967de43`, `5c2626d38`, etc.); no scope drift into #10035 territory.
> *   **`[CONTENT_COMPLETENESS]`**: 92 — Every new assertion carries an explicit `AC #10033:` in-code marker so future readers trace the contract without cross-referencing this PR. **Soft deductions:** (a) PR body phrasing *"all seven named-checklist items"* reads ambiguously — it refers to the 7 section headers in the ticket's AC block, not the ~17 individual checkboxes; could be tighter. (b) The tier-1 vs tier-3 weight test uses `toBeGreaterThan` without expected-value assertions; minimally rigorous rather than rigorous.
> *   **`[EXECUTION_QUALITY]`**: 88 — Tests pass in 656ms. Diff is clean (+56 / −3 single file, no production code). **Primary deduction:** the production-graph 100-line test (`serializeForLLM Tier 1-2 should stay under 100 lines`) codes the AC wording directly but without measuring the current production-graph output first. With the concept ontology under active enrichment (#10050 description expansion, #10036 memory-core discovery, #10037 chromadb embedding), that ceiling could be approached silently and then flake under unrelated concept-additions. Should probably either measure-and-annotate the current line count as a headroom baseline, or lift the threshold to a named constant with a comment explaining the intent.
> *   **`[PRODUCTIVITY]`**: 95 — All four named-AC gaps closed. #10035 unblocked cleanly per the block graph.
> *   **`[IMPACT]`**: 55 — Test-coverage completeness; narrow contract closure. Unblocks exactly one downstream sub-issue; no capability or debt-reduction delta on its own.
> *   **`[COMPLEXITY]`**: 15 — Trivial. 56 added lines in one spec file. No production code touched.
> *   **`[EFFORT_PROFILE]`**: **Maintenance** — routine AC-checklist closure. Not a Quick Win (no new capability), not a Heavy Lift (trivial scope). Pure block-chain hygiene.
> 
> ---
> 
> ### 🕸️ Context & Graph Linking
> *   **Target Epic / Issue ID:** Resolves #10033
> *   **Related Graph Nodes:**
>     - #10030 — parent epic (Concept Ontology & Semantic Gap Inference)
>     - #10032 / PR #10048 — shipped the 16 base tests this PR extends
>     - #10049 / PR #10052 — added the aliases + ANALOGOUS_TO tests (brought total to 21)
>     - #10035 — **next** in the block chain; unblocked by this PR
>     - Session memory `aae16459` (session `f76d5307`, 2026-04-17) — memory-mining hit that informed the AC-gap audit
> 
> ---
> 
> ### 🧠 Graph Ingestion Notes
> 
> *   **`[TOOLING_GAP]`**: The 100-line threshold in the new production-graph serialize test is a latent flake risk under active concept-ontology enrichment. It's not a bug — it codifies #10033's AC wording — but without measuring the current output first, a future concept-description expansion via #10050 could push the production tree past 100 lines unexpectedly, breaking this test as a side effect. Consider either (a) extracting the threshold to a named constant with a comment on expected headroom, or (b) adding a one-line measurement that logs `lines.length` at run time so the next reader sees the current margin. Low-priority polish, can be a follow-up commit or a post-merge touch-up.
> 
> *   **`[RETROSPECTIVE]`**: First in-session validation of the `memory-mining` skill I shipped under #10075 — same session, same PR sequence. Before writing any test code, queried the Memory Core for `"Concept Ontology sub-issue ordering GapInferenceEngine unit tests"` + `"#10033 Concept Graph unit tests vs #10035 GapInferenceEngine refactor sequencing"`. Session `f76d5307` / memory `aae16459` surfaced within ~15s, revealing PR #10048 had already shipped 16 tests and tobi had architecturally corrected the service placement from `ai/graph/` to `ai/services/` during that session. This reframed #10033 from "write the whole suite" to "close the 4 named-AC gaps" and steered the spec-file path decision away from the stale path still in the ticket body. The mechanism the skill exists to enforce (reflex-as-skill invocation replacing reflex-as-rule drift) is self-validating in miniature here — the loop closes cleanly.
> 
> ---
> 
> ### 📋 Required Actions
> 
> All acceptance criteria are met; the items below are polish rather than blockers.
> 
> - [ ] (Optional, follow-up) Make the 100-line production-graph threshold resilient — either extract to a named constant with a headroom comment, or measure-and-log the current line count so future readers know how close to the ceiling the ontology currently runs. Flake guard, not a correctness issue.
> - [ ] (Optional, deferred) Tighten the tier-1 vs tier-3 weight assertion with expected-value ranges when the weight formula base values are documented more formally. Current `toBeGreaterThan` is sufficient for the AC contract but minimally rigorous.
> 
> Approved for merge when you're ready. Zero production code touched; rollback is a single-file revert. The memory-mining discipline that drove the scope decision is now documented as a concrete Gold Standard in the PR body — future agents picking up sibling tickets inherit the pattern automatically.
> 

---

### `@tobiu` commented on 2026-04-19T08:43:56Z

**Input from Claude Opus 4.7 (Claude Code):**

> ❋ ## Polish commit `29fb2ac0b` — 100-line hard cap → observability
> 
> Applied tobi's challenge from the review thread. The original test codified a stale heuristic without firing the `Hypothesis vs Root Cause` gate from `ticket-intake §1.4`. The 100-line cap measured against a production graph currently producing 53 lines — 2× headroom on a 50-line estimate from a smaller-context era. Capping a growth-target ontology at frontier-model scale is self-defeating.
> 
> **Replaced with:**
> 
> ```js
> test('serializeForLLM Tier 1-2 from production graph should produce non-empty output and report current size', () => {
>     // ...
>     console.log(`[ConceptService] serializeForLLM(2) produced ${lines.length} lines from the production graph`);
>     expect(lines.length).toBeGreaterThan(0);
> });
> ```
> 
> - Logs the current line count on every run (visible in CI output) — growth trends and sudden jumps are now observable
> - Asserts non-empty to catch serializer regressions without codifying an arbitrary ceiling
> - In-code comment documents the reasoning in full, pointing future readers back to this review thread
> - Verified: 23/23 tests pass in 617ms; CI log line reads `[ConceptService] serializeForLLM(2) produced 53 lines from the production graph`
> 
> ## `[RETROSPECTIVE]` follow-up filed
> 
> The miss pattern is a real blind spot in `ticket-intake`'s §1.4 Hypothesis gate — hardcoded numerical thresholds in ACs were not specifically enumerated as a high-risk prescription class. Filed as **a new ticket** to strengthen the skill itself rather than leaving it as tacit knowledge. The fix will make this failure mode durable across harnesses and sessions, not just captured in my private memory.
> 
> Also saved as durable feedback memory (`feedback_challenge_numerical_thresholds_in_acs.md`) so future sessions on any harness inherit the rule.
> 
> ## Updated review tally
> 
> - `[EXECUTION_QUALITY]`: 88 → **92** (premise-risk addressed, not just flake-risk)
> - `[CONTENT_COMPLETENESS]`: unchanged (92) — the AC mapping is still accurate; the AC itself was stale, which is a different class of issue now formalized in the follow-up ticket
> 
> No further blockers. Still approved for merge.
> 

---

