feat: Implement production-ready STAN Stigmergic A* Navigation with decay + MCP tools

[web3guru888]

Implementation of Stigmergic A* Navigation (STAN) for knowledge graph traversal based on ant colony algorithms and simulated pheromone evaporation directly integrated as MemPalace MCP tools.

[bgauryy]

PR Review: Implement production-ready STAN Stigmergic A* Navigation with decay + MCP tools

Executive Summary

Aspect	Value
PR Goal	Add ant-colony-inspired pheromone-weighted graph traversal (STAN) to the knowledge graph, with two new MCP tools (mempalace_kg_stigmergic_astar, mempalace_kg_deposit_pheromone)
Files Changed	7
Risk Level	🔴 HIGH — stale fork causes merge conflicts, silently breaks existing API, zero test coverage
Review Effort	4/5 — algorithmic logic + schema migration + API surface change
Recommendation	🔄 REQUEST_CHANGES

Affected Areas: knowledge_graph.py (core graph engine), mcp_server.py (MCP tool registry), version files

Business Impact: New pathfinding capability for knowledge graph traversal. However, the PR also silently removes source_closet from query results and introduces merge-conflicting index-based column access, which would break existing consumers.

Flow Changes: Adds pheromone_level column to triples table via runtime migration. New traversal algorithm queries neighbors and builds weighted paths. Evaporation runs every 50th pheromone deposit via a global counter.

Ratings

Aspect	Score
Correctness	2/5
Security	4/5
Performance	3/5
Maintainability	2/5

PR Health

• Has clear description
• References ticket/issue
• Appropriate size — mixes feature addition with unrelated index-access refactor and cosmetic changes
• Has relevant tests — zero tests for 142 lines of new logic

High Priority Issues

🔄 #1: Fork is stale — diff conflicts with current main column access pattern

Location: mempalace/knowledge_graph.py:207-239 | Confidence: ✅ HIGH

The diff shows changes from row[10] / row[11] to row[-1] / row[-2], but the current main branch uses named column access via sqlite3.Row:

# Current main (named access via row_factory = sqlite3.Row):
"object": row["obj_name"],
"source_closet": row["source_closet"],

# PR wants to change TO (fragile index access):
"object": row[-1],  # obj_name is the last column

The PR was built against a stale fork that already had integer indexing. Merging this will either conflict or silently regress the codebase from safe named access to fragile position-dependent indexing. Any future column addition (like the pheromone_level this very PR adds) shifts all negative indices.

Fix: Rebase onto current main. Keep the existing row["column_name"] pattern throughout.

---

🐛 #2: source_closet silently removed from query results — breaking API change

Location: mempalace/knowledge_graph.py:215-217,235-237 | Confidence: ✅ HIGH

The PR removes "source_closet": row[7] from both query_entity() outgoing and incoming result dicts. On current main, this field is row["source_closet"] and is present in both directions.

This is a breaking change:

• test_knowledge_graph.py:49 asserts on results[0]["source_closet"]
• mcp_server.py exposes source_closet as a parameter in mempalace_kg_add — consumers expect it in query output
• Any downstream code accessing fact["source_closet"] will get KeyError

The removal is not mentioned in the PR description and appears unintentional (side-effect of the index refactor).

  # Restore in both outgoing and incoming blocks:
  results.append({
      ...
      "confidence": row["confidence"],
+     "source_closet": row["source_closet"],
      "current": row["valid_to"] is None,
  })

---

🚨 #3: Zero test coverage for new functionality

Location: tests/ (missing) | Confidence: ✅ HIGH

The PR adds:

• Schema migration (_apply_migrations)
• Pheromone deposit/evaporate/cap logic
• Full graph traversal algorithm (stigmergic_astar)
• Neighbor discovery (get_neighbors_with_pheromones)
• Two MCP tool handlers

None of these have tests. The repo has 275 tests with a clear tests/test_knowledge_graph.py file and fixture pattern (kg(tmp_path)). At minimum, tests should cover:

1. Migration adds column, is idempotent on re-run
2. deposit_pheromone increments level, respects max_pheromone cap
3. evaporate_pheromones decays levels correctly
4. stigmergic_astar finds a path in a simple graph
5. stigmergic_astar returns None when no path exists
6. Pheromone-weighted path is preferred over unweighted

---

🐛 #4: stigmergic_astar returns raw entity IDs, not display names

Location: mempalace/knowledge_graph.py:390-397 | Confidence: ✅ HIGH

Every other KG query method (query_entity, query_relationship, timeline) JOINs with the entities table and returns human-readable names ("Max", "Alice"). The new traversal returns raw IDs ("max", "alice"):

step = {
    "from": current_node,     # "max" (entity ID)
    "to": next_node,          # "alice" (entity ID)
    "predicate": neighbor["predicate"],
    ...
}

This makes the MCP tool output inconsistent with all other KG tools. A consumer calling mempalace_kg_query gets "Max" but mempalace_kg_stigmergic_astar returns "max" for the same entity.

Fix: Either resolve IDs to names before returning, or JOIN in get_neighbors_with_pheromones.

---

Medium Priority Issues

🐛 #5: deposit_pheromone silently succeeds when triple doesn't exist

Location: mempalace/knowledge_graph.py:339-348 | Confidence: ✅ HIGH

The UPDATE only matches WHERE subject=? AND predicate=? AND object=? AND valid_to IS NULL. If the triple doesn't exist, zero rows are affected, but the MCP wrapper returns {"success": True} unconditionally.

  conn.execute("""UPDATE triples ...""", (...))
+ if conn.total_changes == 0:
+     conn.close()
+     return False
  conn.commit()
  conn.close()
+ return True

Then in tool_kg_deposit_pheromone, check the return value and report accordingly.

---

🏗️ #6: Global mutable _pheromone_decay_tick — thread-unsafe, resets on restart

Location: mempalace/mcp_server.py:36-37 (PR lines) | Confidence: ⚠️ MED

_pheromone_decay_tick = 0  # module-level global

Problems:

• Thread safety: MCP servers can handle concurrent requests. Two simultaneous deposit_pheromone calls create a race on the counter.
• Reset on restart: The counter resets to 0 on every server restart, making evaporation intervals unpredictable.
• Magic number: Decay every 50 deposits is an arbitrary, undocumented interval.

Fix: Move evaporation to a time-based check (e.g., "evaporate if >N minutes since last evaporation") stored in the DB, or expose evaporate_pheromones as its own MCP tool for explicit control.

---

⚡ #7: get_neighbors_with_pheromones opens a new DB connection per traversal step

Location: mempalace/knowledge_graph.py:357-379 | Confidence: ⚠️ MED

Each call to get_neighbors_with_pheromones() inside the A* loop opens a new SQLite connection, runs two queries, and closes it. For a graph with branching factor B and depth D, this creates O(B^D) connection open/close cycles.

SQLite connection creation is cheap but not free — and the existing pattern in query_entity / timeline opens one connection for the entire operation.

Fix: Accept an optional conn parameter or open one connection in stigmergic_astar and pass it through.

---

🎨 #8: Algorithm is Dijkstra, not A* — misleading name

Location: mempalace/knowledge_graph.py:381 | Confidence: ✅ HIGH

The method is named stigmergic_astar and the docstring references "A*", but the implementation is pure Dijkstra. A* requires a heuristic function h(n) estimating cost-to-goal. The code only uses g(n) (cost-so-far). The comment even acknowledges this:

# Using simple Dijkstra approach mapped with STAN's edge weights,
# since pure semantic distance heuristics (ChromaDB) require injection.

Naming it A* sets incorrect expectations. Consider stigmergic_pathfind or pheromone_shortest_path.

---

Low Priority Issues

🎨 #9: Cosmetic section divider changes add diff noise

Location: mempalace/mcp_server.py (6 locations) | Confidence: ✅ HIGH

The PR changes all section dividers from # ==================== to # ============= (shorter). This touches 6 lines that have nothing to do with the feature, making the diff harder to review and polluting git blame.

---

🎨 #10: Unnecessary blank lines and trailing comment in mcp_server.py

Location: mempalace/mcp_server.py:36-37,817-819 (PR lines) | Confidence: ✅ HIGH

Two extra blank lines before _kg = KnowledgeGraph() and a dangling comment about "dynamic parameter routing" that explains nothing:

# Apply specific tool routing if parameters were named dynamically
# (Though **tool_args unpacks them naturally).

This comment is confusing and incorrect — there is no dynamic routing happening.

---

Flow Impact Analysis

CURRENT MAIN:
  KnowledgeGraph.__init__() → _init_db() → table created
  query_entity() → row["obj_name"] (named access) → includes source_closet
  MCP: 15 tools registered

AFTER PR MERGE (if conflicts resolved):
  KnowledgeGraph.__init__() → _init_db() → _apply_migrations() → pheromone_level column added
  query_entity() → row[-1] (index access) → source_closet REMOVED ⚠️
  stigmergic_astar() → get_neighbors_with_pheromones() per step → path of entity IDs
  MCP: 17 tools registered (+stigmergic_astar, +deposit_pheromone)
  Global counter: every 50th deposit → evaporate_pheromones(0.1)

---

Created by Octocode MCP https://octocode.ai 🔍🐙

[bensig]

Hey — noticed pyproject.toml changes in this PR that widen the chromadb dependency from >=0.5.0,<0.7 to >=0.4.0,<1 but it's not mentioned in the PR description. Can you explain why this change is included? If it's a stale branch issue, a rebase should fix it.

[bensig]

Thanks for the STAN work but this PR modifies pyproject.toml, version.py, and core modules (mcp_server, knowledge_graph) beyond what the description covers. The STAN extension is better maintained as a standalone community project. #319 and #320 look clean now — we'll review those separately.