LearnNewsExamplesServices
Frontmatter
titlefeat(ai): config-lift guideGapWeightThreshold to aiConfig.data (#10086)
authortobiu
stateMerged
createdAtApr 19, 2026, 5:20 PM
updatedAtApr 19, 2026, 5:33 PM
closedAtApr 19, 2026, 5:33 PM
mergedAtApr 19, 2026, 5:33 PM
branchesdev ← agent/10086-config-lift-guide-gap-threshold
urlhttps://github.com/neomjs/neo/pull/10102
Merged
tobiu
tobiu commented on Apr 19, 2026, 5:20 PM

Summary

Promotes the previously file-local GUIDE_GAP_WEIGHT_THRESHOLD = 0.8 constant in GapInferenceEngine to aiConfig.data.guideGapWeightThreshold, making the concept-graph gap weight gate curator-tunable without a code change. Default preserved (0.8); the derivation comment moves from the const JSDoc to the config template next to the value itself — where tuners will see it.

The Problem

#10084 introduced the threshold as a file-local const with a full derivation comment. #10087 widened its scope from gating [GUIDE_GAP] alone to gating all three concept-graph signals ([GUIDE_GAP] + [EXAMPLE_GAP] + [ORPHAN_CONCEPT]). With three signals now bottlenecked through one number, the value of human-tuning has compounded: a single config bump can silence tier-2/3 noise across all three channels at once as the ontology grows (#10036 / #10037 / #10050).

Architectural Reality (verified against precedent)

  • ai/mcp/server/memory-core/config.template.mjs — checked-in source of truth for the config schema. The matching config.mjs is gitignored (see .gitignore), generated/maintained per-checkout via bootstrapWorktree.mjs or manual bootstrap. Only the template goes into git; local config.mjs parity is the individual operator's responsibility.
  • ai/services.mjs:51 — Memory_Config exported for SDK consumption. Used identically in DreamService.mjs:82 (aiConfig.data.autoDream) — established pattern.
  • ai/daemons/services/GapInferenceEngine.mjs — the threshold is used once, at the gate in inferConceptGraphGaps. Replacing the const with aiConfig.data.guideGapWeightThreshold is a 1-line substitution; caching as a local const threshold at the top of the method is an optimization (avoids repeated property access in the for-loop) but also reads the live config value at gate time, which lets tests mutate it at runtime.
  • Environment-variable escape hatch: NEO_GUIDE_GAP_WEIGHT_THRESHOLD follows the existing pattern (NEO_VECTOR_DIMENSION, GRAPH_DECAY_FACTOR, etc.) so operators can tune without editing the config file.

Implementation

File Change
config.template.mjs New guideGapWeightThreshold key after decayFactor (both graph-pipeline floats). JSDoc carries the full ConceptService.calculateWeight derivation + post-#10087 context. Env override via NEO_GUIDE_GAP_WEIGHT_THRESHOLD.
GapInferenceEngine.mjs Deleted the file-local const. Added Memory_Config as aiConfig to the existing services.mjs import. Cache const threshold = aiConfig.data.guideGapWeightThreshold once per cycle at the top of inferConceptGraphGaps. Class + method JSDoc updated to reference the config path and explain the historical guideGap* prefix (introduced in #10035 for GUIDE_GAP alone; widened in #10087 to gate all three concept-graph signals).
DreamService.spec.mjs New test: threshold override via aiConfig.data.guideGapWeightThreshold changes emission behavior. Plants a weight-0.5 concept (below default 0.8), overrides threshold to 0.3, asserts both [GUIDE_GAP] and [ORPHAN_CONCEPT] now surface. try/finally restores the original. Proves the config is read at gate time, not captured at module load.

Parity note on the gitignored local config.mjs: the shipped template is the source of truth. Each checkout's local config.mjs must mirror the new key for its tests + runtime to pick it up. Updated manually in this worktree; bootstrapWorktree.mjs will carry it forward to future worktrees once the main checkout's local config is updated.

Gold Standards Leveraged / Traps Avoided

  • Gold Standard (aiConfig.data pattern): follows the established autoDream, autoGoldenPath, decayFactor convention. No new architectural surface.
  • Gold Standard (env override pattern): NEO_GUIDE_GAP_WEIGHT_THRESHOLD mirrors GRAPH_DECAY_FACTOR, NEO_VECTOR_DIMENSION. Consistent operator-control story.
  • Gold Standard (runtime-read not module-load-captured): caching inside the method body, not at module top-level, means tests + live mutations are honored without module reimport.
  • Trap avoided (rename scope-creep): the config name guideGapWeightThreshold is arguably imprecise post-#10087 (gates three signals, not one). Rename considered but scoped out to keep this PR tight — the ticket AC literally specifies guideGapWeightThreshold, and renaming would break the env-var operator contract. Documented the historical prefix in JSDoc so future readers aren't confused.
  • Trap avoided (committing gitignored file): caught config.mjs in first staging attempt — .gitignore enforcement stopped it. Reshaped to commit only the template; parity note in commit + PR body documents the operator responsibility.

Testing

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

41 passed (ConceptService 23, DreamService 10 — including new threshold-override test, DreamServiceGoldenPath 1, ConceptIngestor 7). Stable across 5 consecutive runs.

The new test covers AC #4 ("threshold override via config produces different gap-emission behavior") explicitly: same concept, same graph state, different config value → different gap output. The existing "skip low-weight concepts" test remains as the default-path counterpart.

Out of Scope / Follow-Up

  • Rename guideGapWeightThreshold → conceptGapWeightThreshold: the historical prefix is misleading post-#10087. A rename PR would also update the env var name (breaking operator contract) and the JSDoc everywhere. Low value today (config likely not yet set by any operator); could ship later if the name becomes a stumbling block.
  • Bootstrap-worktree config regeneration (#10088 adjacent): the template/local split requires manual sync or worktree re-bootstrap. Currently no tooling regenerates local config.mjs from template on template changes; operators edit both files in parallel. A npm run sync-config or similar could close this gap if it becomes painful.

Related

  • Parent epic: #10030 (Concept Ontology & Semantic Gap Inference)
  • Origin PR: #10084 (introduced the const)
  • Preceded by: #10100 (scope hoist — same epic), #10101 (ORPHAN_CONCEPT — widened threshold scope)
  • Weight formula precedent: ConceptService.calculateWeight (tier + uniqueness + coverage_deficit)

Resolves #10086

Origin Session ID: 1db25bbe-f39d-4dcd-bb0e-bc125ce91326

tobiu
tobiu commented on Apr 19, 2026, 5:27 PM

Input from Claude Opus 4.7 (Claude Code):

❋ # PR Review Summary

Status: Approved (pending human merge) — with an explicit architectural-debt deduction caught by tobi that I missed during ticket-intake.

Self-Review Opening: Self-review of #10086. This PR chose (a) ai/mcp/server/memory-core/config.template.mjs as the config destination because aiConfig was already imported wherever it needed to be read — path of least resistance, not architectural justification. (b) Cached the threshold once per cycle at the top of inferConceptGraphGaps rather than per-concept, so tests can mutate the config live. (c) Kept the historical guideGapWeightThreshold name despite it now gating three signals (documented the historical prefix in JSDoc). Rationale documented in the PR body; (a) is the deduction tobi flagged post-push that I should have caught in ticket-intake.

📊 Evaluation Metrics

  • [ARCH_ALIGNMENT]: 72 — Implementation correctness is sound (config-lift + env override + runtime-read pattern all match precedent). Major deduction: placed in memory-core config because that's where aiConfig already lived — import-convenience, not service-boundary correctness. The threshold is consumed by ai/daemons/services/GapInferenceEngine, which is a daemon service, not a memory-core-specific one. Under the mid-term architecture tobi described — "services outside MCP servers into the SDK, MCP servers as thin wrappers" — this config belongs in an SDK-level config file that doesn't yet exist. KB config would have been "a tick cleaner" (concept graph is KB-adjacent) but still not the right destination. Ticket-intake should have run the service-boundary check (does this config semantically belong to the service that consumes it?) before I picked a file. I didn't. The code ships correct for today; the placement is deferred debt. Filing the migration ticket as a Required Action below.

  • [CONTENT_COMPLETENESS]: 80 — Anchor & Echo JSDoc is thorough; derivation comment moved to the template next to the value; try/finally in the new test restores config correctly. Deductions: (a) the PR body rationalizes the placement via the established aiConfig.data pattern without interrogating whether that pattern's current home is the right one for this specific config. Should have said something like "placed in memory-core temporarily because the SDK-level config destination doesn't yet exist; migration ticket #XXXX." (b) The JSDoc comment near the config key notes the historical prefix but doesn't note the deferred SDK migration — future readers looking at config.template.mjs won't know this config is expected to move.

  • [EXECUTION_QUALITY]: 90 — 41/41 stable × 5 runs. New override test proves live config reads at gate time (not module-load capture). The .gitignore catch on config.mjs was caught loudly by git staging rather than going silent, which is the right failure mode. Deduction: light — the finally block in the override test restores to original but doesn't verify the restore took effect (a subsequent test relying on default threshold would catch divergence; I didn't add that explicit check).

  • [PRODUCTIVITY]: 95 — Ticket's 4 ACs all satisfied. Scope stayed tight. Env-override escape hatch + derivation comment + runtime-read pattern are all bonus value over the minimum ticket scope.

  • [IMPACT]: 40 — Operator-tuning knob with immediate effect. Today's default of 0.8 is the "right" value for thin concept ingestion; the impact grows once richer ingestion (#10036 / #10037 / #10050) creates a real tuning need.

  • [COMPLEXITY]: 28 — 3 files in git (template + source + test) + 1 gitignored local sync. Shallow scope. The most complex consideration was architectural placement — which I got wrong.

  • [EFFORT_PROFILE]: Quick Win — small surface area, high operator-control value, proven pattern reuse. Would have been cleaner still with the SDK-config destination in hand; instead it's a Quick Win with ~1 PR of deferred debt.

🕸️ Context & Graph Linking

  • Target Epic / Issue ID: Resolves #10086
  • Related Graph Nodes:
    • Parent: #10030 (Concept Ontology & Semantic Gap Inference)
    • Preceded by: #10100, #10101 (earlier #10030 sub-issues)
    • Origin PR: #10084 (introduced the constant)
    • Deferred debt: SDK-config migration ticket (to be filed as follow-up; I'll propose content below)

🧠 Graph Ingestion Notes

  • [KB_GAP]: The mid-term architecture "services out of MCP servers into the SDK" isn't queryable via ask_knowledge_base("where should cross-cutting config live?"). The boundary between MCP-server-owned config (transport/auth/storage paths) and service-layer config (behavior knobs like this threshold) is implicit in code layout but not documented in learn/agentos/. Worth a doc ticket alongside the migration PR.

  • [TOOLING_GAP]: .gitignore correctly caught my attempt to stage config.mjs. Good failure mode. No tooling gap here — the system worked.

  • [RETROSPECTIVE]: Third-order ticket-intake failure mode this session. The skill chain is now clearly five stages: challenge premise → challenge prescription → identify substrate → identify consumer → identify service-boundary ownership. Earlier sessions (#10085 Items 3 + 4) caught substrate mistakes; #10101 caught consumer identity; this PR caught placement-boundary. Each failure mode is architecturally independent — passing one check doesn't buy you the others. Filing feedback_config_service_boundary.md as the fifth entry in the discipline chain. Observation: the "default to the path of least resistance" bias (memory-core because aiConfig was already there) is the common thread across all three misses. Each time the fix is: slow down before picking the destination; ask what the consumer semantically owns.

📋 Required Actions

  • Follow-up ticket (will propose visibly to tobi before creating): SDK-level config migration — move daemon/service-layer configs out of memory-core/config.template.mjs into an SDK-layer config destination. Scope: identify which existing config keys (guideGapWeightThreshold, decayFactor, autoDream, autoGoldenPath, etc.) are daemon-facing vs MCP-server-facing; create the new SDK config file; migrate; update all imports. Non-blocking for #10086 but THE architectural destination for this config key. Would keep the guideGapWeightThreshold name + default during migration — only the file path changes.
  • (Optional, minor) Strengthen override test to verify finally actually restored the original threshold (subsequent-behavior check).
  • (Optional, follow-up) Doc ticket: learn/agentos/ explainer for the config-boundary split between MCP-server configs and SDK-layer configs.
  • (Post-merge, recurring) KB sync so ask_knowledge_base("how is the concept-graph gap threshold configured?") returns current answers.

Approved for human merge. The code ships correct; the placement debt is documented and filed. I'd rather ship with the architectural miss explicitly deducted + tracked than pretend the placement choice was intentional.

tobiu
tobiu commented on Apr 19, 2026, 5:32 PM

Input from Claude Opus 4.7 (Claude Code):

❋ ## Follow-up migration ticket filed: #10103

Tracks the architectural destination for guideGapWeightThreshold (and sibling daemon/service-layer config keys) — SDK-layer config file, migrating out of memory-core/config.template.mjs. Keys identified for migration: guideGapWeightThreshold, decayFactor, autoDream, autoGoldenPath, autoIngestFileSystem, realTimeMemoryParsing, modelProvider, neoEmbeddingProvider, chromaEmbeddingProvider, handoffFilePath (definitive list TBD during intake).

#10103 preserves the env-var contracts (NEO_GUIDE_GAP_WEIGHT_THRESHOLD etc.) so the migration is file-path + import-path only; operator-visible surface unchanged.

This PR ships as-is with the placement debt explicitly tracked. Handed off.