experiment: freeze critical PTCNA evaluation - #15
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e084e29b53
ℹ️ 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".
| usefulness_status=UNRESOLVED, | ||
| superiority_status=UNRESOLVED, |
There was a problem hiding this comment.
Distinguish target construction failures from comparator failures
When target_factory() raises, this shared try returns UNRESOLVED for usefulness, even though the frozen artifact specifies that a target-backend error must produce FALSIFIED usefulness. Because comparator construction is in the same block, the receipt cannot identify which backend failed and can contradict the preregistered failure propagation; construct each backend under a separate handler and apply its frozen status.
AGENTS.md reference: AGENTS.md:L35-L38
Useful? React with 👍 / 👎.
| advantage threshold over the hashed-linear fallback. It is an in-sample role | ||
| acquisition test, not a generalization test. Its digest was frozen before either |
There was a problem hiding this comment.
Do not report label-blind training as role acquisition
This is described as a role-acquisition test, but the target's PCNAEngine.reward() at ptcna/neural/pcna.py:323-329 never uses its winner argument and instead nudges every ring using the same reward_outcome; permuting all expected role labels therefore leaves target training unchanged. Meanwhile HashedLinearFallback.reward() updates the labeled winner's weights, so the frozen superiority comparison gives only the comparator class-specific supervision and cannot support the stated acquisition verdict.
AGENTS.md reference: AGENTS.md:L35-L38
Useful? React with 👍 / 👎.
| if plan.digest != artifact["plan_digest"]: | ||
| raise ValueError("frozen critical evaluation plan digest mismatch") |
There was a problem hiding this comment.
Anchor the frozen plan digest outside the mutable artifact
The only expected digest is read from the same JSON object as the plan, so changing the plan and recomputing its adjacent plan_digest passes this check. In that scenario load_frozen_plan() does not reject semantic drift despite its contract and docstring; compare against an immutable digest anchored separately from the caller-selected artifact, such as a code constant fixed by the preregistration commit.
AGENTS.md reference: AGENTS.md:L35-L38
Useful? React with 👍 / 👎.
| result = { | ||
| "schema": "ptcna.critical-evaluation-result", | ||
| "schema_version": "1.0.0", | ||
| "source_commit": artifact["source_commit"], |
There was a problem hiding this comment.
Bind each result receipt to the implementation that ran
When this shipped wrapper is executed after any target or comparator implementation change, it still copies the preregistration artifact's fixed parent commit into the result; backend validation checks only the unchanged identity strings, so modified code can run while the receipt claims 3d67f359... as its source. Record and verify the actual implementation revision or backend code digest before execution so evidence cannot be attributed to code that did not run.
AGENTS.md reference: AGENTS.md:L35-L38
Useful? React with 👍 / 👎.
| declared cognitive, self-model, and autonomy roles, three training epochs, five | ||
| fresh repetitions, a 0.75 post-training target threshold, and a strict 0.05 |
There was a problem hiding this comment.
Isolate the global Sigma state between fresh repetitions
The target instances created for these advertised fresh repetitions are not independent: PCNAEngine.reward() mutates the process-global singleton returned by get_sigma() (ptcna/neural/sigma.py:121-128), and subsequent instances inject that accumulated Sigma state into Psi during inference (ptcna/neural/pcna.py:234-242). Later repetitions—and even runs preceded by unrelated PTCNA use in the same process—therefore start from contaminated target state, so reset or inject Sigma per repetition before aggregating the critical verdict.
AGENTS.md reference: AGENTS.md:L35-L38
Useful? React with 👍 / 👎.
| "training_epochs": 3, | ||
| "reward_outcome": 1.0, | ||
| "repetitions": 5, |
There was a problem hiding this comment.
Freeze the target's time-derived random seed
With the frozen positive reward, every target training step calls MemoryCore.flush_to(), whose _reset() seeds new state from int(time.time()) at ptcna/neural/memory_core.py:82-92. The critical plan neither fixes nor records this seed, so identical executions can train on different target states and can cross the frozen verdict thresholds solely because they ran at different wall-clock times; expose and freeze the seed or record a complete RNG schedule in the plan.
AGENTS.md reference: AGENTS.md:L35-L38
Useful? React with 👍 / 👎.
| "max_seconds": 120.0, | ||
| "stopping_rule": "complete_or_first_backend_error_or_resource_limit", |
There was a problem hiding this comment.
Enforce the wall-clock bound around backend calls
If a backend constructor, infer(), or reward() blocks longer than 120 seconds, the evaluator cannot apply this frozen resource limit because it only checks elapsed time before entering synchronous backend calls. A hung call therefore produces neither the required UNRESOLVED receipt nor the preregistered stopping behavior and may run indefinitely; execute backend operations under an enforceable deadline or describe this as a cooperative rather than exact bound.
AGENTS.md reference: AGENTS.md:L35-L38
Useful? React with 👍 / 👎.
Frozen before outcome inspection
ptcna.experimental.v1; comparatorfallback.hashed-linear.v167cdad3aefb3e33f6fbf3994de54e1b73a01105527bf241fce08947ed7046bbeNeither backend has been executed against this workload in this PR.
Validation