feat(gossip): priority channels, noise tolerance, amplification, random walk - #2250
feat(gossip): priority channels, noise tolerance, amplification, random walk#2250jjcav84 wants to merge 12 commits into
Conversation
|
Hi @milla-jovovich @igorls — this PR is rebased onto the latest "develop", the full test suite passes, and the design/context is documented in the body. Ready for review. |
|
@milla-jovovich @igorls this branch is up to date with |
fatkobra
left a comment
fatkobra
left a comment
There was a problem hiding this comment.
I found two blocking channel-semantics issues.
1. An explicit max_hops argument is overwritten by the channel default
The preliminary message correctly incorporates the caller's explicit
max_hops value.
When the final message is constructed, however, the implementation reads
max_hops from the channel parameters again. Since every built-in channel
defines a value, the caller's explicit value is replaced by the channel
default.
The precedence should be:
- explicit caller argument;
- channel default;
- global fallback.
Please preserve the resolved value through the complete message-building
path.
Regression coverage should verify that:
- an explicit
max_hops=10remains 10 even on a channel whose default is 2; - omitting the argument uses that channel's value of 2;
- channels without a value use the global fallback.
2. The random walk does not ensure that the selected node is off-radius
_random_walk_swap() selects from any chatter node that was not already
selected by the normal ranking.
“Not selected” is not equivalent to “outside the source's gossip radius.”
A lower-ranked node can still be inside the same permitted radius.
Please either:
- calculate an actual off-radius candidate set and choose only from it; or
- change the feature contract and documentation so it no longer claims
off-radius exploration.
The test should make the distinction observable by providing:
- one unselected node that is still inside the radius;
- one node that is genuinely outside it;
- deterministic random selection;
and verifying that only the genuine off-radius node is eligible.
This branch is stacked on #2239 and should be rebased after the parent
changes are complete.
6ca0b83 to
cff6d2c
Compare
|
Hi @fatkobra — this branch is stacked on its parent gossip PR. I will rebase it once the parent lands and then re-request review. No action needed until then. |
|
Merged latest |
2c20788 to
805b245
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.
- 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
Add a rooms field to ChatterNode and match hallway co-occurrence rooms against node rooms rather than hall names. Regression test uses a room (audit_report) that differs from the node hall (security) to prove the namespaces are no longer conflated. Refs MemPalace#2247
…mPalace#2221) The MemPalace#2221 fix reached develop through MemPalace#2228 at an earlier revision of the branch, so three guards and their tests did not come with it. One of the three is a regression the gate that did land introduced. sweep_directory: the new gate probes the file type with f.stat() inside a try, and its except OSError printed SKIP and continued. A dangling symlink, a symlink loop and a file unlinked between rglob and the gate all raise there. Before the gate existed each of them reached sweep(), raised, and was appended to failures — so the gate turned "could not read this transcript" into a silent skip and a successful exit. A probe that FAILS is an error, not a benign file type: log it, print WARNING, book it in failures. A probe that succeeds and reports a non-regular file still skips silently. _parse_gradle: the is_file() gate sat in front of the try whose except OSError the parser already had, so a manifest under a directory with r but no x raised PermissionError out of a call that used to answer "no manifest name". The gate moves inside that try. _collect_manifest_names stats with os.path.isfile, which reports instead of raising, matching the parsers it guards. split_file: the type gate in main() covers the files the glob listed, but split_file builds its output names itself, so a pre-existing FIFO at one of them wedged write_text in the kernel waiting for a reader. Output names that are anything but a regular file are skipped. That gate asks os.path.lexists, not os.path.exists. exists() follows the link, so a DANGLING symlink at an output name reads as "nothing there" and the write goes through it, creating the target — a chunk landing wherever the link points rather than in the output directory. Measured: the two calls differ on that one case and agree on every other (regular file, symlink to a file, missing name, FIFO, symlink to FIFO, directory). test_gather_origin_samples_survives_an_unreadable_directory broke under root rather than passing vacuously: CAP_DAC_OVERRIDE walks into the 0o444 directory, the walled-off file stays readable, and the count assertion sees two samples instead of one. It now carries the same needs_unprivileged_posix gate as the three new permission tests. miner._read_text_no_follow: comment fix only. F_SETLEASE on a FIFO fails EINVAL, not ENXIO — measured on Linux 6.18 / glibc 2.39. The code branches on EAGAIN and is unaffected.
Per-message try/except so one failing message does not discard unprocessed siblings or prior children. Requeue children while holding the lock and reserve capacity against concurrent callers. Refs MemPalace#2248
- Encode the original predicate in the gossip source_file so viral_facts can group by the source fact identity instead of the destination predicate. - Use one reference timestamp for all as_of metrics (active and expired). - Format the default reference as canonical UTC to satisfy KG validation. Refs MemPalace#2249
805b245 to
7e2382e
Compare
|
@fatkobra thanks for the detailed review — the blockers are noted. As you pointed out, these are stacked on #2239 and should wait for the parent to be resolved first. Opened #2335 to make the design case for derived routing vs KG mutation per your review on #2239. Once there's sign-off on the derived-view approach (no KG writes, sidecar for routing state), I'll rewrite the stack with the fixes for your specific blockers on this PR. |
|
Closing — stacked on #2239 which was just closed. See comment there for rationale. |
3 participants
Stacked on #2239 (feat/gossip-mvp).
Summary
Implements the priority-channel model and several remaining chatter-behavior parameters from the gossip design spec:
critical,high,medium,low.ttl_seconds,fanout,gossip_probability,max_hops, pluslatency_ms/reliability/capacitymetadata.select_chatter_nodesenforcesnoise_tolerance(drops low-relevance nodes)._should_forwardapplies the channel'sgossip_probability,chatter_levelmultiplier, andamplification_factorfor critical/high messages.chatter_statusnow returnschannels,noise_tolerance,amplification_factor,randomness_factor, andgossip_probability.Test plan
tests/test_gossip.pypasses (18 tests).tests/test_mcp_server.pypasses.