feat(gossip): add gossip protocol MVP and MCP tools - #2239
Conversation
d5f0846 to
defaf2a
Compare
7dedcd0 to
4501b8e
Compare
Implements mempalace/gossip.py with ChatterNode, GossipMessage, GossipProtocol, default chatter-node config, topic/priority detection, propagation through palace graph tunnels, and TTL-bounded derived KG triples. Adds tests/test_gossip.py with 16 unit tests. Full suite: 4261 passed, 31 skipped.
4501b8e to
c5665e2
Compare
|
Hi maintainers — this PR is rebased onto the latest develop and the full test suite passes. Ready for review. |
|
@milla-jovovich @igorls this branch is up to date with |
fatkobra
left a comment
There was a problem hiding this comment.
Thanks for building this first gossip slice. The overall separation between
configuration, selection, propagation, and reporting is promising.
I found three blocking issues.
1. The automatic default configuration is specific to one project
DEFAULT_GOSSIP_CONFIG contains project-specific wings, halls, specialties,
and radii, including names such as orkid, brutal-marketing,
past-performance, and negentropy.
When no gossip configuration exists, that topology is automatically written
to the user's palace. An unrelated palace therefore silently acquires another
project's organizational model, and subsequent propagation can write derived
facts according to that model.
Please make the built-in default neutral. Reasonable options would be:
- an empty configuration that requires explicit setup before propagation;
- a minimal topology derived from the current palace;
- or a clearly labelled example configuration that is never installed
automatically.
Please add a regression test using a palace whose wings and rooms share none
of the example project's names, and verify that enabling or inspecting gossip
does not persist unrelated topology or create destination facts for those
names.
2. A node can be reported as propagated even when every KG write failed
The per-target kg.add_triple() exceptions are caught, but the node is still
appended to chatter_nodes and the message is still included in the
propagation report.
As a result, the caller can receive a successful-looking propagation result
even when zero destination facts were written.
Please make write outcomes observable. At minimum, the result should
distinguish:
- attempted targets;
- successful writes;
- failed writes and their errors;
- nodes with at least one successful propagation;
- and nodes for which every write failed.
A node should not be reported simply as propagated when all of its writes
failed.
Please add regression coverage for:
- every
add_triple()call raising; - a mixture of successful and failed targets;
- all writes succeeding.
3. mempalace_gossip is missing from the daemon write classifier
The MCP server includes mempalace_gossip in _MUTATING_TOOLS, but the
daemon's service.WRITE_TOOLS does not include it.
The generic daemon mcp_tool execution path accepts only tools classified as
writes, so gossip is currently treated as an unknown or non-write operation
rather than as the mutation that the MCP server says it is.
Please add mempalace_gossip to the daemon write classification and prevent
the two sets from drifting again.
A table-driven invariant should assert that every member of
_MUTATING_TOOLS is classified as a write unless it is explicitly listed
and documented as an exclusive maintenance operation.
Once these three issues are addressed, I will be happy to re-review the
parent gossip implementation.
- Make DEFAULT_GOSSIP_CONFIG neutral/empty; add EXAMPLE_GOSSIP_CONFIG for tests. - Add config parameter to GossipProtocol and gossip() for explicit setup. - Report attempted/successful/failed targets and only mark a node propagated when at least one write succeeds. - Add mempalace_gossip to service.WRITE_TOOLS and invariant test. Refs MemPalace#2239
|
Hi @fatkobra — thanks for the review. I believe all three blockers are now addressed:
Verification:
Could you re-review when you have a moment? |
fatkobra
left a comment
There was a problem hiding this comment.
Thanks — I re-reviewed c611528, and my three blocking concerns are
resolved.
- The installed default configuration is now neutral, with empty chatter
nodes and topics. The project-specific topology has been moved to an
example configuration that is not installed automatically. - Propagation now exposes attempted, successful, and failed target outcomes,
including individual errors, and a chatter node is reported as propagated
only when at least one KG write succeeds. mempalace_gossipis now classified as a write by the daemon and has
regression coverage for that classification.
The classification test in this branch is specific to the gossip tool rather
than the complete table-driven invariant I originally described. However,
#1992 already implements that repository-wide parity check and the associated
fail-closed handling for classifier drift, so I do not think it should be
duplicated here.
My blockers on this PR are resolved. Approved.
|
Merged latest |
|
Thanks for the persistence on this one, and for turning fatkobra's three I read the whole stack rather than just this PR (#2239 → #2247 → #2248 → The name has to changeIn MemPalace, "gossip" is already taken, and it means the P2P thing. Two
That's epidemic dissemination in the standard sense: a replica learns about a So a top-level This is fixable and worth fixing: what you've built is relevance routing / One related thing, and I flag it purely as a courtesy to you: #2255 checks The harder question: what this writes into the knowledge graphThis is the part I'd want settled before more code gets built on top. Every propagation writes derived triples into the user's KG — one per The KG is entity-first and keyed by real names — people, projects. After And the loop closes: Set against the mission — verbatim always, never summarize or paraphrase, I don't think the underlying idea is bad. "Which parts of the palace should ScopeSix stacked PRs adding one feature, where each depends on the one before, is Two small concrete things while I'm here:
Where I'd go from hereThe rename is unconditional. On the rest — before you write more code, could Thanks again for the work and for staying responsive through review — I know |
- Make DEFAULT_GOSSIP_CONFIG neutral/empty; add EXAMPLE_GOSSIP_CONFIG for tests. - Add config parameter to GossipProtocol and gossip() for explicit setup. - Report attempted/successful/failed targets and only mark a node propagated when at least one write succeeds. - Add mempalace_gossip to service.WRITE_TOOLS and invariant test. Refs MemPalace#2239
- Make DEFAULT_GOSSIP_CONFIG neutral/empty; add EXAMPLE_GOSSIP_CONFIG for tests. - Add config parameter to GossipProtocol and gossip() for explicit setup. - Report attempted/successful/failed targets and only mark a node propagated when at least one write succeeds. - Add mempalace_gossip to service.WRITE_TOOLS and invariant test. Refs MemPalace#2239
- Make DEFAULT_GOSSIP_CONFIG neutral/empty; add EXAMPLE_GOSSIP_CONFIG for tests. - Add config parameter to GossipProtocol and gossip() for explicit setup. - Report attempted/successful/failed targets and only mark a node propagated when at least one write succeeds. - Add mempalace_gossip to service.WRITE_TOOLS and invariant test. Refs MemPalace#2239
- Make DEFAULT_GOSSIP_CONFIG neutral/empty; add EXAMPLE_GOSSIP_CONFIG for tests. - Add config parameter to GossipProtocol and gossip() for explicit setup. - Report attempted/successful/failed targets and only mark a node propagated when at least one write succeeds. - Add mempalace_gossip to service.WRITE_TOOLS and invariant test. Refs MemPalace#2239
- Make DEFAULT_GOSSIP_CONFIG neutral/empty; add EXAMPLE_GOSSIP_CONFIG for tests. - Add config parameter to GossipProtocol and gossip() for explicit setup. - Report attempted/successful/failed targets and only mark a node propagated when at least one write succeeds. - Add mempalace_gossip to service.WRITE_TOOLS and invariant test. Refs MemPalace#2239
- Make DEFAULT_GOSSIP_CONFIG neutral/empty; add EXAMPLE_GOSSIP_CONFIG for tests. - Add config parameter to GossipProtocol and gossip() for explicit setup. - Report attempted/successful/failed targets and only mark a node propagated when at least one write succeeds. - Add mempalace_gossip to service.WRITE_TOOLS and invariant test. Refs MemPalace#2239
|
Opened #2335 to make the design case per @fatkobra's request. TL;DR: gossip becomes a routing layer, not a fact layer. No KG writes. Propagation state goes to a sidecar table that recall never reads. The The child PRs (#2247–#2251) keep their core logic; only their storage target changes. Will rewrite once there's sign-off on the derived-view approach. |
Replace project-specific wing/room names (orkid, brutal-marketing, past-performance, negentropy) with neutral placeholders (alpha, beta, gamma, delta) in EXAMPLE_GOSSIP_CONFIG, test fixtures, and the implementation report doc. Per igorls review on MemPalace#2239: private palace topology should not land in the public tree. Tests pass (38 gossip, 3 MCP gossip, 44 KG). Ruff clean. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Closing this PR and the stacked gossip children (#2247–#2251, #2255) in favor of a simpler approach. After further reflection and @igorls's feedback, the write-time propagation model is the wrong abstraction for the problem we're trying to solve. The actual problem is that search is wing-siloed by default — the LLM searches one wing and misses context in related wings. Propagating synthetic triples into the KG at write time doesn't fix that; it just moves the silo problem earlier and pollutes the verbatim fact store. The wing-affinity logic in this PR (chatter node topology, hallway-aware selection, affinity scoring) is worth keeping and will be extracted into a query-time search expansion PR instead — making Thanks to @fatkobra and @igorls for the thorough reviews — the design feedback directly shaped this decision. |
Summary
mempalace/gossip.py) with chatter nodes, message propagation, TTL-bounded KG triples, and topic/priority detection.mempalace_gossipandmempalace_gossip_statusMCP tools.tests/test_gossip.pyandTestGossipToolsintests/test_mcp_server.py.website/reference/mcp-tools.mdandREADME.mdto 46 tools.Test plan
uv run pytest tests/ -vpasses (4313 passed, 31 skipped ondevelop).uv run ruff check mempalace/gossip.py mempalace/mcp_server.py tests/test_gossip.py tests/test_mcp_server.pypasses.