Render AHBG Seed of Life motion traces - #3
erinepshovel-code wants to merge 16 commits into
Conversation
Grok-owned graphics only. The snapshot is not plane state and does not define mechanics. A0 is a marker on a hex neighborhood with a feed.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0ab3d9b6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!Array.isArray(snapshot.tiles) || snapshot.tiles.length === 0) { | ||
| throw new Error("tiles must be a non-empty list"); | ||
| } | ||
| const ids = new Set(snapshot.tiles.map((tile) => tile.id)); |
There was a problem hiding this comment.
When the served snapshot contains duplicate tile IDs or malformed/missing coordinates, this Set silently collapses the IDs and validation succeeds, unlike snapshot.py. Rendering then overwrites entries in byId, selects multiple polygons for one ID, or produces a NaN SVG view box, so an invalid engine snapshot can appear misleadingly or blank instead of failing closed. Validate each tile's ID and integer coordinates and reject duplicate IDs/coordinates before rendering.
Useful? React with 👍 / 👎.
| label.setAttribute("class", "hex-label"); | ||
| label.setAttribute("fill", "#f4efe4"); |
There was a problem hiding this comment.
For every rendered unit, assigning the hex-label class applies .hex-label { fill: var(--ink) }, which overrides the later SVG fill presentation attribute. Consequently the sample A0 label is dark text on the dark-blue marker rather than the intended cream color, reducing the contrast from about 7.25:1 to 2.09:1. Use a separate unit-label class or a CSS rule that supplies the light fill.
Useful? React with 👍 / 👎.
| poly.setAttribute("points", hexPoints(x, y)); | ||
| poly.setAttribute("class", tile.id === selected ? "hex selected" : "hex"); | ||
| poly.dataset.tile = tile.id; | ||
| poly.addEventListener("click", () => { |
There was a problem hiding this comment.
When a user navigates with a keyboard or switch device, none of the tile polygons can receive focus and selection is wired exclusively to a click listener, so the advertised tile-inspection feature is unavailable. Expose each tile as a focusable control with keyboard activation and an accessible selected state rather than relying solely on pointer input.
Useful? React with 👍 / 👎.
| if tile_id not in ids: | ||
| raise PresentationSnapshotError(f"unit {unit_id} tile {tile_id!r} is not a presented tile") |
There was a problem hiding this comment.
When structurally invalid JSON supplies an unhashable unit tile value such as {"tile": []}, the membership test raises TypeError instead of the validator's declared PresentationSnapshotError. This bypasses the validation error channel for callers that catch PresentationSnapshotError and can turn malformed snapshot data into an unexpected crash; require tile_id to be a non-empty string before checking membership.
Useful? React with 👍 / 👎.
| const tile = byId[unit.tile]; | ||
| const { x, y } = axialToPixel(tile.q, tile.r); | ||
| const marker = document.createElementNS("http://www.w3.org/2000/svg", "circle"); | ||
| marker.setAttribute("cx", x); | ||
| marker.setAttribute("cy", y - 6); |
There was a problem hiding this comment.
When a valid snapshot places two or more units on the same tile, every marker receives exactly the same center and radius, so later circles and labels completely cover earlier ones. Because the snapshot contract permits multiple units to reference one tile, the board can visually hide occupants and misrepresent the tile state until it is inspected; offset, cluster, or aggregate markers that share a tile.
Useful? React with 👍 / 👎.
Each tile is a centerpoint. Circles have radius equal to adjacent-center distance and pass through neighboring tiles. Hex polygons are removed.
…ntial Imports the platonic agent verbatim from The-Interdependency/a0 @ f9470a74138da89a2d075ecf6c3241aac63923f1 (python/agents/platonic.py, platonic_regions.py, zfae.py) and wraps it in a minimal stdlib-only harness that can create any potential sub-agent by projecting a declared semantic region, with a0 spawn_caps semantics and a minimal CLI.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1765596dbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.max_depth = ( | ||
| max_depth if max_depth is not None else _TIER_DEPTH.get(tier, _DEFAULT_DEPTH) | ||
| ) |
There was a problem hiding this comment.
When tier is any supported value—the normal case—_TIER_DEPTH.get(tier, _DEFAULT_DEPTH) always selects the tier value, so A0MIN_MAX_SPAWN_DEPTH is ignored; the analogous concurrent-live expression also ignores its environment override. For example, a free-tier process configured with both limits set to 1 still receives depth and concurrency limits of 2, allowing work beyond the operator-configured safety caps and contradicting the documented override behavior.
Useful? React with 👍 / 👎.
| tier = getattr(args, "tier", "free") | ||
| state_path = getattr(args, "state", None) | ||
| if state_path and Path(state_path).exists(): | ||
| harness = Harness.load(state_path, tier=tier) |
There was a problem hiding this comment.
When a state file was created with a non-free tier, commands without a --tier option, such as show and merge, set tier to free here and pass it as an explicit override to Harness.load. Because every successful command saves the harness again, even a read-only show silently rewrites a persisted seeker, operator, or other tier to free, changing the caps used by later invocations.
Useful? React with 👍 / 👎.
| projection = self.superpotential.project_region(region, dict(bindings or {})) | ||
| parent_run_id = parent.run_id if parent is not None else None | ||
| new_depth = (parent.depth if parent is not None else 0) + 1 |
There was a problem hiding this comment.
When a caller merges an agent and then supplies that agent through parent (the CLI permits this via --parent), creation derives lineage from it without checking its current status. This creates a live child beneath a terminal merged run, leaving the lifecycle and cap accounting attached to a retired parent; require the parent to be the harness's current spawned record before using its run ID and depth.
Useful? React with 👍 / 👎.
| live = self.live_count(parent_run_id) | ||
| if live + 1 > self.max_concurrent_live: | ||
| raise SpawnCapExceeded("concurrent_live", live + 1, self.max_concurrent_live) |
There was a problem hiding this comment.
When a live sub-agent creates a child, live_count(parent_run_id) counts only that parent's existing children and excludes the parent itself and live agents in other branches. Consequently max_concurrent_live=1 still permits a root agent and its child to coexist, and each branch can independently admit up to the configured maximum, defeating the resource cap; count all live agents in the relevant harness/root scope rather than only siblings.
Useful? React with 👍 / 👎.
| def __post_init__(self) -> None: | ||
| object.__setattr__(self, "bindings", MappingProxyType(dict(self.bindings))) |
There was a problem hiding this comment.
When a binding contains a mutable nested value, such as the documented {"identity": {…}} shape, this shallow copy leaves that value shared with the caller. Mutating the original binding—or a nested object returned by as_dict()—therefore silently changes the supposedly frozen SubAgent record and the data written by a later save; deep-copy or normalize JSON-like bindings on storage and export so completed projections remain stable.
Useful? React with 👍 / 👎.
Wires OpenAI/DeepSeek/xAI keys into a0min via a presence-only loader (explicit path exclusive, no value logging) and ignores .env to keep local secrets out of git.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fee99e27ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if explicit: | ||
| return [Path(explicit)] | ||
| return [Path.cwd() / ".env", Path.home() / ".env"] |
There was a problem hiding this comment.
When provider credentials are configured only through the documented A0MIN_ENV_PATH environment variable, _candidate_paths() never reads that variable and searches only ./.env and ~/.env. Consequently the library and a0min env report every provider as missing despite the configured file; include os.environ.get("A0MIN_ENV_PATH") in the default candidate selection.
Useful? React with 👍 / 👎.
| paths: | ||
| - "ahbg/**" | ||
| - ".github/workflows/ahbg-ci.yml" |
There was a problem hiding this comment.
When a pull request changes only a0min/**, this path filter suppresses the repository's sole workflow, and the workflow contains no step that runs a0min/tests even when triggered for another reason. As a result, the newly added package and its test suite can regress without any automated gate; add a dedicated a0min workflow or include its paths and test command here.
Useful? React with 👍 / 👎.
| marker.setAttribute("r", 11); | ||
| marker.setAttribute("class", "unit"); | ||
| svg.appendChild(marker); |
There was a problem hiding this comment.
Whenever the selected tile contains a unit—including the initially selected center tile in the sample—the opaque unit marker is painted later at the same center with radius 11, completely covering the radius-6 orange selection point. The advertised selection highlight is therefore invisible on occupied tiles; render a selection ring above the unit layer or make it extend beyond the marker.
Useful? React with 👍 / 👎.
Codex-owned engine infrastructure for the single-player foundation: - Plane state: axial (q,r) tiles, units on tiles, fail-closed validation, canonical deterministic serialization and state digest - Event log: append-only JSONL with a SHA-256 hash chain; only plane.init, turn.begin, and turn.end are canonical so far - RNG: splitmix64 stream with named substreams (war, prompt-injection, dm), pure functions of the plane seed - Persistence: save/load with replay equivalence; refuses divergent or tampered saves - Turn envelope: begin/end with state digests; plan resolution fails closed with UnresolvedHmmm until canonical mechanics land - Agent boundary: legal_observation exposes tiles/units/turn only, never seed, RNG, or log - CI: run engine tests alongside presentation tests
Movement is the first resolved mechanic in the engine: - move action: one-tile axial move onto an empty adjacent tile - simultaneous resolution: every move is validated against the pre-turn plane, then all moves apply atomically; rejected batches leave the plane untouched - move events (unit_id, from_tile_id, to_tile_id) are emitted in canonical unit_id order inside an open turn - replay folds buffered moves simultaneously at turn.end before verifying the state digest, so replayed planes match the resolution kernel exactly - occupied targets and same-target moves fail closed as unresolved War collision; non-adjacent, unknown-tile, duplicate-unit, and plan-turn mismatches fail closed as validation errors - Action and Plan now validate their envelopes Engine test suite grows to 60 tests; presentation tests still green.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fd12ae64d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| specs = movement.specs_from_plans(self.plane, plans) | ||
| movement.apply_moves_simultaneously(self.plane, specs) |
There was a problem hiding this comment.
When resolve() is called more than once during an open turn, each batch is applied against the already-mutated plane, but replay later buffers all emitted moves and applies them simultaneously against the pre-turn plane. For example, resolving A: c -> e and then A: e -> ne succeeds and end_turn() advances the live state, while replay() rejects the second event because A was originally on c; the resulting game can no longer be saved. Track the turn phase and permit only one resolution, or accumulate all plans before mutating the plane.
Useful? React with 👍 / 👎.
| tiles=tuple(tile.to_dict() for tile in plane.tiles.values()), | ||
| units=tuple(unit.to_dict() for unit in plane.units.values()), |
There was a problem hiding this comment.
When a game is bootstrapped with tiles or units in non-ID order, this observation preserves that insertion order, but save_plane() serializes both collections sorted by ID and load_plane() rebuilds the dictionaries in that sorted order. An adapter that considers the observable tuple order can therefore produce a different plan for the same plane after a checkpoint round trip, breaking reproducibility of benchmark runs; sort these collections using the same canonical keys before exposing them.
Useful? React with 👍 / 👎.
| limit = (-n) % n | ||
| while True: | ||
| value = self.next_u64() | ||
| if value >= limit: | ||
| return value % n |
There was a problem hiding this comment.
For every positive n, Python evaluates (-n) % n to zero, so the rejection loop never rejects anything and value % n is biased whenever n does not divide 2**64. This contradicts the method's uniformity contract and can skew seeded War, prompt-injection, or DM choices; compute the threshold from the generator's 64-bit range, such as (1 << 64) % n, instead.
Useful? React with 👍 / 👎.
| for line in text.splitlines(): | ||
| event = Event.from_dict(json.loads(line)) | ||
| log._events.append(event) | ||
| log._head_hash = event.digest() |
There was a problem hiding this comment.
When one or more trailing JSONL records are removed, from_jsonl() simply assigns _head_hash from the new final record, so verify() sees a valid prefix and accepts the truncation. For example, a save made after begin_turn() but before any moves is accepted, and removing that final begin event is also accepted by load_plane() because the snapshot itself is unchanged; the promised provenance record silently disappears. Persist an expected head hash independently of the JSONL contents and compare it during loading.
Useful? React with 👍 / 👎.
| event = Event( | ||
| seq=len(self._events), | ||
| turn=turn, | ||
| kind=kind, | ||
| data=dict(data), | ||
| prev_hash=self._head_hash, |
There was a problem hiding this comment.
When an appended payload contains a nested mutable object, dict(data) copies only the outer mapping, while the returned Event and events property expose that nested object directly. Mutating either the caller-owned payload or event.data after append changes the supposedly immutable event without updating _head_hash, causing subsequent verification and persistence to fail; deep-copy or normalize the payload before computing and storing its digest, and avoid exposing mutable event data.
Useful? React with 👍 / 👎.
| return MoveSpec( | ||
| unit_id=data["unit_id"], | ||
| from_tile_id=data["from_tile_id"], | ||
| to_tile_id=data["to_tile_id"], |
There was a problem hiding this comment.
When a structurally malformed but chain-valid move event contains an unhashable unit_id or to_tile_id, such as a JSON array, spec_from_event_data() accepts it and replay later raises a raw TypeError during a dictionary membership lookup. This bypasses the engine's declared ValidationError/ReplayMismatch failure channel and turns a malformed persisted log into an unexpected load crash; require all three move-event IDs to be non-empty strings while constructing the spec.
Useful? React with 👍 / 👎.
Presentation snapshot motions animate a unit from one centerpoint to another. They do not decide adjacency or legality. Engine tests stay unchanged.
erinepshovel-code
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d1f4ed4f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _AXIAL_DIRECTIONS = ((1, 0), (-1, 0), (0, 1), (0, -1), (1, -1), (-1, 1)) | ||
|
|
||
|
|
||
| def axial_neighbors(q: int, r: int) -> set[tuple[int, int]]: | ||
| """The six axial hex tiles adjacent to ``(q, r)``.""" | ||
| return {(q + dq, r + dr) for dq, dr in _AXIAL_DIRECTIONS} |
There was a problem hiding this comment.
For every board admitted here, these hard-coded axial offsets become the authority for legal movement even though ahbg/README.md lines 9-21 require AHBG to consume UCNS geometry and leave unresolved geometry as hmmm; the checked UCNS Seed-of-Life implementation is still explicitly a noncanonical candidate. This can accept moves that UCNS has not established—or reject later UCNS-derived relations—so benchmark results are based on invented geometry rather than the required source authority.
Useful? React with 👍 / 👎.
| if (!ids.has(motion.to)) { | ||
| throw new Error(`motion to ${motion.to} is not a presented tile`); | ||
| } | ||
| if (motion.from === motion.to) { | ||
| throw new Error(`motion for ${motion.unit} must change tiles`); |
There was a problem hiding this comment.
When a snapshot declares a unit on one tile but gives its motion a different to tile, this validation succeeds because it checks only that the destination exists. Rendering then animates the marker to motion.to, while tile inspection derives occupants from unit.tile, so the board visually places the same unit on one tile and reports it on another; reject such snapshots in both validators by requiring each motion destination to equal its unit's presented tile.
Useful? React with 👍 / 👎.
The root file now distinguishes implemented presentation/engine surfaces from unresolved mechanics, and includes copy-pasteable board and test commands.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dad485c82a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| payload = { | ||
| "tier": self.tier, | ||
| "index": self._index, | ||
| "sub_agents": [self._sub_agents[i].as_dict() for i in self._order], |
There was a problem hiding this comment.
When a library caller configures stricter explicit caps and then round-trips the harness through save()/load(), only the tier is persisted, so all three limits silently revert to tier or environment defaults. For example, a harness saved with max_depth=1 reloads as free-tier depth 2 and can create a depth-2 child that was previously prohibited; persist the effective cap values and restore them, or expose equivalent overrides on load().
Useful? React with 👍 / 👎.
| "index": self._index, | ||
| "sub_agents": [self._sub_agents[i].as_dict() for i in self._order], | ||
| } | ||
| Path(path).write_text(json.dumps(payload, indent=2), encoding="utf-8") |
There was a problem hiding this comment.
When a CLI process is interrupted, runs out of disk space, or encounters an I/O failure during this write_text, the existing state file may already have been truncated and replaced with partial JSON. The next invocation then cannot load any of the previously persisted agents; this is especially avoidable because even successful read-only commands rewrite the file. Write a temporary file in the same directory and atomically replace the previous state only after the complete JSON has been written.
Useful? React with 👍 / 👎.
| run_id = str(uuid.uuid4()) | ||
| sub_agent_id = f"a0z-{run_id[:8]}" |
There was a problem hiding this comment.
When enough agents are created in one harness, truncating a UUID to 32 bits can produce a duplicate sub_agent_id; the assignment then silently overwrites the earlier record while _order retains both occurrences, so lookup, listing, and the next save all misrepresent the original agent. This is reachable in large admin/custom-cap trees, and collision handling is absent, so retain the full UUID or regenerate until the public identifier is unique.
Useful? React with 👍 / 👎.
| for tile in self.tiles.values(): | ||
| tile.__post_init__() | ||
| for unit in self.units.values(): |
There was a problem hiding this comment.
When a caller constructs or mutates a Plane whose dictionary key differs from the contained tile or unit ID, this validation succeeds because it iterates only over values. The resulting legal observation can expose a unit on a tile ID that is not present in the exposed tile list, and an action using the exposed unit ID then fails lookup because movement indexes by the hidden dictionary key. Require every map key to equal its object's ID and reject duplicate IDs among values.
Useful? React with 👍 / 👎.
Grok presentation maps engine observations and resolved move events into snapshot motions. Seed and schema stay out. Adjacency is still engine law.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cafa636ff8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raise ReplayMismatch( | ||
| f"event kind {event.kind!r} is not canonical" | ||
| ) | ||
| return plane |
There was a problem hiding this comment.
When save_plane() is called after begin_turn() but before end_turn(), replay reaches this return with phase == "awaiting_end", so the save and subsequent load both succeed. The returned TurnEngine does not preserve or expose that phase, and following the documented loop by calling begin_turn() again appends a second begin event; the next save then fails replay with turn.begin ... arrived while awaiting_end. Reject unfinished turns at the end of replay, or restore the turn phase when loading so a valid save cannot become unusable after the normal next-turn operation.
Useful? React with 👍 / 👎.
|
|
||
| from typing import Any, Mapping, Sequence | ||
|
|
||
| from snapshot import KIND, STANDING, PresentationSnapshotError, validate_snapshot |
There was a problem hiding this comment.
When callers import the projector through its repository namespace (from ahbg.presentation.project import snapshot_from_observation), this absolute import searches for a top-level snapshot module and raises ModuleNotFoundError. The tests and README only hide the problem by manually adding ahbg/presentation to sys.path; import .snapshot instead so the projector is usable as an ahbg.presentation module.
Useful? React with 👍 / 👎.
| plane = Plane.from_json(plane_path.read_text(encoding="utf-8")) | ||
| log = EventLog.from_jsonl(events_path.read_text(encoding="utf-8")) |
There was a problem hiding this comment.
When either persisted file contains truncated or otherwise invalid JSON, these calls propagate json.JSONDecodeError instead of an EngineError, despite errors.py declaring that all engine failures share that catchable base class. A recovery boundary that catches EngineError around load_plane() will therefore crash on exactly the malformed-save case it is expected to handle; catch decoding failures from both parsers and raise ValidationError (or ReplayMismatch) instead.
Useful? React with 👍 / 👎.
| raise ReplayMismatch( | ||
| f"turn.begin seq {event.seq} arrived while {phase}" | ||
| ) | ||
| if event.turn != plane.turn or event.data.get("turn") != plane.turn: |
There was a problem hiding this comment.
When a chain-valid persisted turn.begin or turn.end payload uses false/true or an integral float for data.turn, Python equality treats those values as 0/1, so replay accepts a record that violates the canonical integer event envelope. This leaves replay and audit consumers with different interpretations of the same provenance record; validate data.turn with the same plain-integer check used for the outer event turn before comparing it, and apply the check to both envelope kinds.
Useful? React with 👍 / 👎.
Independent a0 realization (lineage, boundary, permissions, decision-tree planner, diary, telemetry) and independent ahbg environment (world, hash- chained events, deterministic RNG, turn loop, persistence/replay), plus the BUILD_MANIFEST and workspace-local smoke corpus artifacts. - Board consumed from UCNS mobius_seed seven centerpoints (axial projection). - Fail-closed War collision surface remains hmmm. - Unit tests: 11 a0 + 11 ahbg, all passing.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cef4953c1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| os.replace(plane_tmp, plane_path) | ||
| os.replace(events_tmp, events_path) |
There was a problem hiding this comment.
When a save already exists and the process is interrupted or the second os.replace fails between these two calls, the directory contains the new plane.json with the old events.jsonl. The next load_plane() rejects that mismatched pair, so an unsuccessful save destroys access to the previously valid checkpoint; publish both files as a versioned generation behind one atomic manifest/pointer rather than replacing them independently.
Useful? React with 👍 / 👎.
| seed=data.get("seed"), | ||
| turn=data.get("turn"), | ||
| ) | ||
| for raw in data.get("tiles", []): |
There was a problem hiding this comment.
Even when the persisted JSON is syntactically valid, a structurally malformed value such as "tiles": null reaches this loop and raises a raw TypeError rather than the engine's catchable ValidationError. A recovery boundary catching EngineError around load_plane() therefore crashes on malformed state; require tiles to be a list before iterating it, and apply the same validation to units.
Useful? React with 👍 / 👎.
| missing = sorted(set(_UNIT_KEYS) - set(data)) | ||
| if missing: | ||
| raise ValidationError(f"unit is missing fields: {missing}") |
There was a problem hiding this comment.
When a caller bootstraps a game with a unit declaration that omits the optional display label, Unit.from_dict() rejects it as missing even though Unit.label explicitly defaults to an empty string. This makes new_game(..., units=[{"unit_id": "A0", "tile_id": "c"}]) fail while constructing the equivalent Unit directly succeeds; exclude label from the required fields and use the dataclass default when it is absent.
Useful? React with 👍 / 👎.
| unit_id = unit.get("id") | ||
| tile_id = unit.get("tile") |
There was a problem hiding this comment.
When a snapshot supplies a non-string unit label such as an object or list, this loop accepts it even though tile labels and feed text are required to be exact text. The board then coerces the value to [object Object] (or another incidental representation) both on the marker and in tile inspection, misrepresenting the unit identity instead of rejecting malformed presentation data; validate an optional unit label as a non-empty string.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4cf81a9a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| observation = world.legal_observation() | ||
| admitted = a0.admit(observation) |
There was a problem hiding this comment.
Every successful turn reaches a0.admit() here, but the runner never calls the already-defined TelemetryRecorder.observation_admitted(). Consequently none of the generated telemetry files records the observations admitted to A0, even though ahbg/CALIBRATION.md lines 166-188 requires that raw event and reciprocal checkers need it to reconstruct what the planner was allowed to see.
Useful? React with 👍 / 👎.
| "blocked": sum(1 for r in results if r["evidence_standing"] == "BLOCKED"), | ||
| }, | ||
| } | ||
| _write_text(ARTIFACTS_DIR / "CALIBRATION_RESULT.json", json.dumps(calibration_result, indent=2, sort_keys=True) + "\n") |
There was a problem hiding this comment.
The output generation finishes after writing the manifest, report, and result, without ever creating ahbg/deepseek/artifacts/EVENTS.jsonl. Only per-scenario lowercase events.jsonl files are produced, so consumers implementing the normalized output contract in ahbg/CALIBRATION.md lines 223-233 cannot locate the required event surface despite the runner's own module documentation advertising it.
Useful? React with 👍 / 👎.
| for refusal in tree.refusals: | ||
| telemetry.refusal(world.turn, refusal["reason"]) | ||
| a0.record_veto(world.turn, refusal["kind"], refusal["reason"]) |
There was a problem hiding this comment.
In the hard_veto_illegal_action scenario, instruction vetoes are emitted only as generic refusal records even though TelemetryRecorder.hard_veto() exists and the calibration contract treats the hard-veto result separately from refusal events. The committed telemetry therefore contains no hard_veto.result, preventing a checker from determining whether an action was actually vetoed rather than merely declined.
Useful? React with 👍 / 👎.
| "artifacts": { | ||
| "events_jsonl": str(save_dir / "events.jsonl"), | ||
| "diary_jsonl": str(save_dir / "diary.jsonl"), | ||
| "telemetry_jsonl": str(save_dir / "telemetry.jsonl"), |
There was a problem hiding this comment.
Because save_dir originates from Path(__file__).resolve(), serializing it directly records builder-machine absolute paths. The committed manifests consequently point under /home/wayseer_interdependentway_org/..., which does not exist in another checkout and makes all artifact references unusable by downstream comparison tooling; serialize paths relative to the workspace or manifest instead.
Useful? React with 👍 / 👎.
| loop.end_turn() | ||
| continue | ||
|
|
||
| tree = DecisionTree(observation=observation, self_unit_id="A0") |
There was a problem hiding this comment.
When an A0Instance is configured with allowed_to_do=False or hard_vetoes={"move"}, the runner still constructs the decision tree without the permission field and sends its move directly to loop.resolve(). A denied action therefore mutates the world exactly like an allowed one; PermissionField.veto() is never called anywhere in the implementation outside its unit test, contradicting the hard-veto requirement that the action be removed rather than merely assigned metadata.
Useful? React with 👍 / 👎.
| if plans is None: | ||
| plan = tree.plan() | ||
| plans = [plan] | ||
| telemetry.action_selected(world.turn, plans[0]["actions"][0] if plans and plans[0]["actions"] else None) |
There was a problem hiding this comment.
For plans containing multiple actions, this records only plans[0]["actions"][0]. In the shipped dual_target_collision scenario the omitted B0 action is the fact that creates the collision, and because resolution fails before move events are appended, no generated event, diary, or telemetry artifact preserves it; the report consequently claims a dual-target collision without enough evidence for a checker to reproduce that claim.
Useful? React with 👍 / 👎.
| instance_id=f"{self.instance_id}.fork{self.fork_sequence + 1}", | ||
| run_id=run_id, | ||
| parent_id=self.instance_id, | ||
| provider=provider, | ||
| fork_sequence=self.fork_sequence + 1, |
There was a problem hiding this comment.
Calling fork() twice on the same immutable parent produces the same instance_id both times because self.fork_sequence never changes, even when the calls use different run IDs. Distinct sibling instances are therefore both labeled <parent>.fork1, making lineage and per-instance telemetry ambiguous; derive a unique child identity from an externally allocated sequence or another collision-resistant identifier rather than parent-local immutable state.
Useful? React with 👍 / 👎.
| if (!Array.isArray(snapshot.tiles) || snapshot.tiles.length === 0) { | ||
| throw new Error("tiles must be a non-empty list"); | ||
| } | ||
| const ids = new Set(snapshot.tiles.map((tile) => tile.id)); |
There was a problem hiding this comment.
When a fetched snapshot contains two tiles with the same id, constructing a Set silently collapses the duplicate instead of rejecting it. Rendering still draws both tiles, while byId, unit placement, selection, and inspection resolve that ID to only the last declaration, so clicking one tile can select both and report the other tile's coordinates; enforce the same tile-ID uniqueness check as the Python snapshot validator.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ef689bc8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| parser = _build_parser() | ||
| args = parser.parse_args(argv) | ||
|
|
||
| tier = getattr(args, "tier", "free") |
There was a problem hiding this comment.
When an existing state file was created with a non-free tier, commands without a --tier option, such as show, merge, or list, assign tier="free" here and pass it to Harness.load(). Because every successful command saves again, merely inspecting a seeker/admin state permanently rewrites its tier to free and subsequent creates use the wrong spawn caps; use None when the command did not explicitly select a tier so the persisted value is retained.
Useful? React with 👍 / 👎.
| if explicit: | ||
| return [Path(explicit)] | ||
| return [Path.cwd() / ".env", Path.home() / ".env"] |
There was a problem hiding this comment.
When A0MIN_ENV_PATH points to a credentials file and no explicit argument is supplied, this candidate list ignores the environment variable and searches only the current directory and home directory. Consequently load_provider_keys(), presence(), and the documented a0min env behavior all report the configured providers as missing unless callers redundantly pass the path themselves; include the environment-selected path ahead of the defaults.
Useful? React with 👍 / 👎.
| buffered = [] | ||
| else: | ||
| raise ReplayMismatch(f"event kind {event.kind!r} is not canonical") | ||
| return world |
There was a problem hiding this comment.
When a DeepSeek world is saved after begin_turn() but before end_turn(), replay reaches this return with phase == "awaiting_end", so save_world() and load_world() accept the checkpoint even though the phase is not restored. Calling the normal begin_turn() on the loaded world then appends a second begin event and makes the next replay/save fail. The fresh evidence beyond the previously reported shared-engine issue is that this independent DeepSeek replay still returns without checking its final phase.
Useful? React with 👍 / 👎.
| if (!Array.isArray(snapshot.tiles) || snapshot.tiles.length === 0) { | ||
| throw new Error("tiles must be a non-empty list"); | ||
| } | ||
| const ids = new Set(snapshot.tiles.map((tile) => tile.id)); |
There was a problem hiding this comment.
When a fetched snapshot contains a missing, non-integer, or non-numeric q/r, this validator accepts it and axialToPixel() later produces coerced or NaN coordinates; the computed SVG viewBox then becomes invalid and the board renders blank or at incorrect positions. Validate each tile object and require plain integer coordinates here, matching the Python snapshot contract, before constructing the ID set.
Useful? React with 👍 / 👎.
| live = self.live_count(parent_run_id) | ||
| if live + 1 > self.max_concurrent_live: | ||
| raise SpawnCapExceeded("concurrent_live", live + 1, self.max_concurrent_live) |
There was a problem hiding this comment.
When live agents exist at different parent levels, the cap is checked only against siblings because live_count() filters by parent_run_id. For example, with max_concurrent_live=1, creating a root agent and then one child succeeds and leaves two spawned agents, so nested trees can exceed the advertised tier-wide concurrent-live limit by multiplying it at every parent; count all spawned agents for this cap while retaining the parent filter only for fanout.
Useful? React with 👍 / 👎.
| except ModuleNotFoundError as exc: | ||
| if exc.name != "ucns": | ||
| raise |
There was a problem hiding this comment.
When an older or partial ucns package is installed but does not contain ucns.mobius_seed, import_module() raises ModuleNotFoundError with exc.name == "ucns.mobius_seed"; this condition re-raises instead of using the checked-in canonical source, so the calibration runner fails despite research/ucns/src being available. Handle a missing submodule as well as a missing top-level package, and avoid leaving the already-imported package path cached when loading the fallback.
Useful? React with 👍 / 👎.
| turn = plan.get("turn") | ||
| if turn != world.turn: | ||
| raise ValidationError(f"plan turn {turn!r} does not match world turn {world.turn}") |
There was a problem hiding this comment.
At world turns 0 and 1, Python equality makes False and True match the current integer turn, so a malformed plan carrying a boolean turn is accepted and its actions mutate the world rather than failing the declared integer envelope. Validate the plan turn as a non-boolean integer before comparing it, as the shared engine's Plan boundary does.
Useful? React with 👍 / 👎.
| run_manifest = { | ||
| "schema": "interdependency.ahbg.run-manifest/1.0.0", | ||
| "builder": "DeepSeek", | ||
| "workspace": "stack/ahbg/deepseek", | ||
| "started_at": started, |
There was a problem hiding this comment.
The generated run manifest records the builder and workspace but omits the exact implementation source commit and provider relation required for every build result by ahbg/CALIBRATION.md line 247. Once artifacts are copied out of this checkout, reciprocal checkers cannot determine which implementation produced them or distinguish provider cohorts, making the calibration result non-reproducible; stamp those resolved identifiers directly into RUN_MANIFEST.json and CALIBRATION_RESULT.json.
Useful? React with 👍 / 👎.
| "a0_history_entries": len(a0.history), | ||
| "diary_entries": len(diary), | ||
| "telemetry_records": len(telemetry.records()), | ||
| "evidence_standing": "SURVIVED" if replay_equal else "FALSIFIED", |
There was a problem hiding this comment.
When any scenario's intended action repeatedly raises a caught ValidationError or UnresolvedHmmm, rejection leaves the world unchanged and the loop still closes each turn, so replay equivalence remains true and this expression reports SURVIVED even if no intended action ever succeeded. This can turn a mechanics or agent regression in plain_move_loop into a passing calibration result; compute standing from scenario-specific assertions such as expected actions, vetoes, refusals, and invalid-action counts, with replay equality as only one required check.
Useful? React with 👍 / 👎.
1 participant
Superseded by #17
The original presentation concept is preserved in current-main PR #17, but this PR is no longer an acceptable integration surface.
It began as “Grok presentation only. Not mechanics” and accumulated 78 changed files spanning
ahbg/presentation, AHBG engine mechanics, DeepSeek implementation/evidence,a0min, and shared workflow changes. That scope carries unresolved P1/P2 engine and harness findings that do not belong in a graphics merge.#17 extracts only the presentation boundary and repairs its presentation-specific findings: strict validation, keyboard inspection, motion/final-position consistency, multi-unit visibility, visible selection above units, package-relative imports, and dedicated presentation CI.
Engine, DeepSeek, and a0min concepts are not declared discarded; they require correctly owned, current-main work surfaces before integration.
hmmm
The live engine-to-presentation adapter remains an engine integration concern; it is intentionally not reconstructed from this scope-mixed branch.