Record P24 approved repair and normal OS evidence; preserve P18 review gaps
This commit is contained in:
Takeshi
+13
@@ -0,0 +1,13 @@
|
||||
# P18-01-CODE 独立レビュー修正候補（未採用）
|
||||
|
||||
対象は提出0084760e、同じ12 allowed_paths・4 backend条件。元の成果単位P18-01、30/88、部分点なしを維持。通常UI・初期値承認・whole-unit最終受入を完了へ変えない。
|
||||
|
||||
1. 原本3値とは別の旧5軸について、affinity/obligationを含む旧記録からaffection/dependenceを実際に更新し、Service・Runtimeの保存再読込で更新値と元raw別名が保全される試験を追加。未変更RelationshipStateの正規キー優先処理を独立レビュー資料へ加える。
|
||||
2. 退店客はGuestService.despawn_guestでpersistent_recordsへ残りactive _guestsから消える。新たなlegacy relationship actorのactive staff/guest存在条件はこの旧関係履歴を拒否し得るため、追加4行を削除する具体案を添付。原本original_relationshipの人物参照検証は保持。
|
||||
3. 旧v1の型互換を既存Reader・代表セーブと照合。有限なJSON数値など以前の正当な保存を保全し、unknown/new-version・不正original payload・部分commitは引き続き拒否する。型の緩和を合格のために行わず、旧形式ごとの受入根拠と拒否条件を記録する。
|
||||
4. UNIQUE heroineのremove_staff/retire_staffは既存ガードが恒久削除を拒否する。original登録後の実拒否・保存復元を試験し、その全文をレビュー資料へ加える。通常人物ライフサイクルの未知仕様は作らない。
|
||||
5. candidate restoreの参照catalogをrestored_catalogへ揃え、duplicate original、version型、projection不一致、non-heroine UNIQUEのRuntime負例を追加。
|
||||
|
||||
局所と影響7試験、正確な新SHAのmarker/policy、独立Opus再レビュー、統合と必要な最終照合を実施。既存5試験は削除・弱化しない。全回帰は関連UI/コードを固めた原成果単位の最終版で実施。
|
||||
|
||||
今回の追加修正が承認される場合だけ、このタスクのmax_repair_attemptsを2→3、repairs2を保持する。P24-02の追加許可を本タスクへ転用しない。現在の候補は未採用、review changes_requestedのまま。
|
||||
+35
@@ -0,0 +1,35 @@
|
||||
{
|
||||
"task_id": "P18-01-CODE",
|
||||
"commit": "0084760ee05a97adfcbd783a71161491f3629918",
|
||||
"status": "changes_requested",
|
||||
"summary": "I compared all four acceptance criteria against the spec, the full diff, the target sources, the identity catalog, the Runner p18-model result (exit 0) and the coordinator's native evidence (7 suites, 843 checks, 308 parse, 9 distinct PIDs, no guard or cleanup errors). I did not edit or run anything. Every change is inside allowed_paths. No existing test was deleted, disabled or weakened. Two new marker rows and two new suite-policy rows were added, pinned to the source hashes.\n\nP18-C-1 is met. The original relationship state is separate from the legacy one, with explicit directional subject/protagonist/rival identities. It requires all fields, rejects non-finite values, and enforces shield_max == affection. An unset record reads as configured:false with null values, distinct from an explicit zero. No production values or coefficients are introduced.\n\nP18-C-2 and P18-C-3 are mostly implemented: v2 nests the full v1 record, restore is atomic, the Runtime command and restore/rebind are real, and the report projection is read-only. But three things are unproven or change legacy behavior:\n(1) The merge-back of legacy aliases is not tested for changes to the aliased axes (affection/dependence).\n(2) Restore now rejects legacy saves it used to accept, through stricter type checks and a new actor-reference check.\n(3) Original records have no lifecycle once the subject leaves staff, which could make saves unloadable.\n\nP18-C-4: native local and affected evidence exists. Integration and the final affected validation have not happened yet, and the backend task correctly does not claim the P18-01 unit complete. Because of the verification gaps above, I am not passing it.",
|
||||
"acceptance_ids": [
|
||||
"P18-C-1"
|
||||
],
|
||||
"findings": [
|
||||
{
|
||||
"severity": "high",
|
||||
"message": "Possible silent loss of affection/dependence changes on legacy saves (P18-C-2, P18-C-3), not verified. RelationshipService._merge_legacy_changes copies the original raw entry and writes only the fields that changed into it, so old aliases like metrics.affinity=117 stay next to a newly changed canonical metrics.affection. If RelationshipState.from_snapshot lets the alias win over the canonical key, a change to affection or dependence after loading a legacy save would be lost on the next save/load. The test only changes trust ('changed metric preserves raw alias'), and RelationshipState.from_snapshot precedence is not shown in the review packet. Needed: a test that changes affection and dependence on an alias-bearing legacy record, then saves and restores through both the service and the Runtime, and confirms the value persists (or the alias is resolved some other way)."
|
||||
},
|
||||
{
|
||||
"severity": "medium",
|
||||
"message": "Legacy behavior changed beyond the spec (P18-C-2 says v1 relationships 'remain usable'; P18-C-3 requires 'legacy unchanged behavior'). Two new checks reject v1 data that loaded before: _legacy_types_valid requires strict int/String/Dictionary types for metrics, shield, day counters, flags, route_lock and so on, and _restore_snapshot_internal now requires every relationship actor to be player, a restored staff member or a restored guest. Legacy relationships pointing at departed or persistent-record guests, non-staff unique characters, or numbers stored as floats would now fail to load. There is no evidence this was checked against the retained P02-01/P02-02 representative saves. Needed: evidence or tests that those saves still load, or restrict the new rejections to the original payload."
|
||||
},
|
||||
{
|
||||
"severity": "medium",
|
||||
"message": "Original records have no lifecycle once the subject stops being staff. _original_relationship_identities_valid requires the subject to still be hired UNIQUE staff. It is checked on restore (using restored_staff) and in validate_invariants. If any existing path removes or replaces the heroine's staff record after registration, the Runtime would fail its invariants and that save could no longer be restored. Needed: either show that no such path exists for UNIQUE heroines, or define and test what happens (keep the record without the staff check, or reject the removal)."
|
||||
},
|
||||
{
|
||||
"severity": "low",
|
||||
"message": "During restore, the identity validator reads the live mvp_content_catalog instead of restored_catalog, which _restore_snapshot_internal builds and installs later. The content is identical today, but the candidate validation is not fully self-contained. Suggest passing restored_catalog into _original_relationship_identities_valid."
|
||||
},
|
||||
{
|
||||
"severity": "low",
|
||||
"message": "Runtime-level negative tests are thin. test_original_relationship_runtime covers corrupt original fields, but not a duplicate original subject, an unknown or mistyped relationships.version, a mismatch between the top-level relationships projection and legacy.relationships, or a non-heroine UNIQUE subject at the Runtime level. These are covered at the service level only."
|
||||
},
|
||||
{
|
||||
"severity": "low",
|
||||
"message": "P18-C-4 process items are still pending: integration and the final affected validation come after this review. The native evidence (843 checks, 308 parse, PIDs and cleanup recorded) and the Runner verify-only pass are present. Correctly, nothing claims P18-01 unit completion."
|
||||
}
|
||||
]
|
||||
}
|
||||
+7
@@ -0,0 +1,7 @@
|
||||
--- a/autoload/simulation_runtime.gd
|
||||
+++ b/autoload/simulation_runtime.gd
|
||||
@@ -1063,4 +1062,0 @@
|
||||
- for relationship: RelationshipState in restored_relationships.get_relationships():
|
||||
- for actor_id: StringName in [relationship.actor_a_id, relationship.actor_b_id]:
|
||||
- if actor_id != &"player" and not restored_staff.has_staff(actor_id) and not restored_guests.has_guest(actor_id):
|
||||
- return false
|
||||
+106
@@ -0,0 +1,106 @@
|
||||
{
|
||||
"schema_check_bytes": 250240,
|
||||
"actual_UTF8_packet_bytes": 249013,
|
||||
"context": {
|
||||
"runtime": {
|
||||
"path": "autoload/simulation_runtime.gd",
|
||||
"revision": "f76ad96990a4d7371b6ab667df0a068cc06be798",
|
||||
"complete_source_sha256": "a5a135842cb29218d3bd861effe66ea7e29c144184d07b4da521ce9a23e121c3",
|
||||
"base_source_sha256": "d52a995ae715542bbc550f68d35889540658275d12e04ae0b21ac276f5843f39",
|
||||
"context_sha256": "ebbabed3c11f2155d7262055f3df93f356aadbdf4c14f9ca6ca86717de46db05",
|
||||
"selected_complete_methods": [
|
||||
"_configure_relationship_event_boundaries",
|
||||
"_connect_relationship_signals",
|
||||
"_execute_attack_relationship_shield",
|
||||
"_execute_change_relationship",
|
||||
"_execute_create_relationship",
|
||||
"_execute_register_original_relationship",
|
||||
"_initialize_s4_relationship_events",
|
||||
"_on_relationship_registered",
|
||||
"_on_relationship_route_lock_changed",
|
||||
"_on_relationship_shield_changed",
|
||||
"_on_relationship_value_changed",
|
||||
"_original_relationship_identities_valid",
|
||||
"_register_or_validate_legacy_catalog_definitions",
|
||||
"_restore_snapshot_internal",
|
||||
"_validate_catalog_staff_crosslinks",
|
||||
"create_snapshot",
|
||||
"get_staff_management_report",
|
||||
"initialize",
|
||||
"restore_snapshot"
|
||||
],
|
||||
"hunks": [
|
||||
{
|
||||
"new_start": 333,
|
||||
"new_count": 1,
|
||||
"methods": [
|
||||
"initialize"
|
||||
]
|
||||
},
|
||||
{
|
||||
"new_start": 1058,
|
||||
"new_count": 3,
|
||||
"methods": [
|
||||
"_restore_snapshot_internal"
|
||||
]
|
||||
},
|
||||
{
|
||||
"new_start": 1063,
|
||||
"new_count": 4,
|
||||
"methods": [
|
||||
"_restore_snapshot_internal"
|
||||
]
|
||||
},
|
||||
{
|
||||
"new_start": 5389,
|
||||
"new_count": 3,
|
||||
"methods": [
|
||||
"_configure_relationship_event_boundaries"
|
||||
]
|
||||
},
|
||||
{
|
||||
"new_start": 6773,
|
||||
"new_count": 24,
|
||||
"methods": [
|
||||
"_execute_register_original_relationship",
|
||||
"_original_relationship_identities_valid"
|
||||
]
|
||||
},
|
||||
{
|
||||
"new_start": 10897,
|
||||
"new_count": 1,
|
||||
"methods": [
|
||||
"get_staff_management_report"
|
||||
]
|
||||
}
|
||||
],
|
||||
"all_new_changed_lines_included": true,
|
||||
"complete_diff_in_packet": true,
|
||||
"omitted_scope": "Unchanged Runtime methods outside the above complete methods. Full immutable source remains in Git at the supplied revision; request further context if needed.",
|
||||
"entire_Runtime_review_claimed": false
|
||||
},
|
||||
"markers": {
|
||||
"path": "docs/plans/original-completion-2026-10-02/execution/completion_marker_contracts.json",
|
||||
"complete_source_sha256": "fb217150a0dafe26ebc7363f5e794bef506f4e76bbbd4e3621f5b8295c0cd973",
|
||||
"base_source_sha256": "3c72cad4dded0619d0cf1c9691b590258ea657d79e482910bde8b0bbeed73e03",
|
||||
"complete_selected_records": [
|
||||
"tests/s4/test_original_relationship_model.gd",
|
||||
"tests/s4/test_original_relationship_runtime.gd",
|
||||
"tests/s4/test_relationship_event_service.gd",
|
||||
"tests/s4/test_runtime_relationship_events.gd",
|
||||
"tests/s6/test_runtime_snapshot_serialization.gd",
|
||||
"tests/s6/test_runtime_save_lifecycle.gd",
|
||||
"tests/ui/test_staff_management.gd"
|
||||
],
|
||||
"omitted_records_proven_unchanged": true
|
||||
},
|
||||
"rules": "Review every fixed backend criterion and all changes using full diff, complete selected methods/sources and actual evidence. Context limits do not waive criteria; request more context or reject if evidence is insufficient. Whole P18-01 completion is not claimed."
|
||||
},
|
||||
"acceptance_ids": [
|
||||
"P18-C-1",
|
||||
"P18-C-2",
|
||||
"P18-C-3",
|
||||
"P18-C-4"
|
||||
],
|
||||
"formal_review_run": false
|
||||
}
|
||||
+45
@@ -0,0 +1,45 @@
|
||||
{
|
||||
"task": "P18-01-CODE",
|
||||
"submitted_commit": "0084760ee05a97adfcbd783a71161491f3629918",
|
||||
"source_files": 1311,
|
||||
"actual_native_evidence": "/Users/korizatou114/Projects/TheTree-managed-worktrees/p18-01-model/.runtime/P18-01-CODE/build/p18-relationship-evidence",
|
||||
"actual_checks": 843,
|
||||
"actual_suites": 7,
|
||||
"parse_checks": 308,
|
||||
"passed": true,
|
||||
"actual_PIDs": [
|
||||
15228,
|
||||
15309,
|
||||
15342,
|
||||
15366,
|
||||
15380,
|
||||
15395,
|
||||
15413,
|
||||
15424,
|
||||
15481
|
||||
],
|
||||
"guard_errors": [],
|
||||
"cleanup_errors": [],
|
||||
"files": {
|
||||
"runner-input.json": {
|
||||
"path": "/Users/korizatou114/Projects/TheTree-managed-worktrees/p18-01-model/.runtime/P18-01-CODE/build/p18-relationship-evidence/runner-input.json",
|
||||
"sha256": "afa2bf024fa36f908840c8c1611b4a796c354d170958988290eb9f95f0840085"
|
||||
},
|
||||
"runner-ended.json": {
|
||||
"path": "/Users/korizatou114/Projects/TheTree-managed-worktrees/p18-01-model/.runtime/P18-01-CODE/build/p18-relationship-evidence/runner-ended.json",
|
||||
"sha256": "fcbbcb9825165ce813326b8374740f07c290a20086e0ed9c38c64625c0ab8e2e"
|
||||
},
|
||||
"receipt.json": {
|
||||
"path": "/Users/korizatou114/Projects/TheTree-managed-worktrees/p18-01-model/.runtime/P18-01-CODE/build/p18-relationship-evidence/receipt.json",
|
||||
"sha256": "de7499e31703fc63f3e69fe6ea2a30260de0cbc6d2671cf31f47aed8c94d1dbf"
|
||||
},
|
||||
"all-tests.json": {
|
||||
"path": "/Users/korizatou114/Projects/TheTree-managed-worktrees/p18-01-model/.runtime/P18-01-CODE/build/p18-relationship-evidence/all-tests.json",
|
||||
"sha256": "eef7faa5e300bf2b221ebf421a10ccd081aa809653f538d8bb87ce73dc93b08d"
|
||||
}
|
||||
},
|
||||
"ordinary_UI_accepted": false,
|
||||
"production_initial_values_approved": false,
|
||||
"full_required_final_roster": "pending whole-unit final version",
|
||||
"original_unit_complete": false
|
||||
}
|
||||
Reference in New Issue
Block a user