Skip to content

Commit 4da2c24

Browse files
committed
Overhaul PeasyAI validation pipeline and improve code generation quality
- Replace ad-hoc code review with unified 4-stage validation pipeline: Stage 1: PCodePostProcessor (deterministic regex auto-fixes) Stage 2: Structured validator chain (13 validators with auto-fix) Stage 3: LLM wiring review for test files (circular deps, init order) Stage 4: LLM spec correctness review (observes completeness, assertions) - Add NamedTupleConstructionValidator for cross-file type checking - Add extraneous-semicolon auto-fix in SyntaxValidator - Fix ValidationPipeline context merging for preview-time cross-file validation - Update Timer template with bounded delays for liveness property support - Update FailureDetector design doc: liveness property, hot states, Timer as component - Add review_test_wiring and review_spec_correctness LLM review prompts - Improve generation prompts: named tuples, circular dependency patterns, helper fns - Fix streamlit lazy-import issue for non-UI contexts - Remove stale tools (simulator, trace_explorer) and dead code - Increase PChecker per-test timeout from 20s to 300s - Update CLAUDE.md with pipeline architecture docs and fix stale .env references - Add regression test support for wiring_fixes and spec_fixes Made-with: Cursor
1 parent 1817838 commit 4da2c24

31 files changed

+2725
-2020
lines changed

CLAUDE.md

Lines changed: 218 additions & 27 deletions
Large diffs are not rendered by default.

Src/PeasyAI/resources/instructions/generate_enums_types_events.txt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,23 @@ Write the enums, types, and events required for each of the state machines. If m
2323

2424
4. **Do NOT declare config init events** (like eProposerInit, eAcceptorInit) unless the design explicitly uses the init-event pattern. Most machines use the constructor payload pattern where config is passed via `new Machine(config)`.
2525

26+
5. **Named tuple construction must match the type definition exactly.** Every ``type`` you define here will be used in ``new Machine(...)`` and ``send ..., eEvent, ...`` call sites throughout the project. The field names you choose become the API contract:
27+
```
28+
// Given this type definition:
29+
type tNodeConfig = (failureDetector: machine);
30+
// ALL construction sites MUST use the exact field name:
31+
new Node((failureDetector = fd,)); // CORRECT
32+
new Node((fd,)); // WRONG — anonymous tuple
33+
new Node(fd); // WRONG — bare value
34+
```
35+
Choose field names carefully — they cannot be changed later without updating every call site.
36+
37+
6. **Event payload types define the send-site contract.** When you declare ``event eFoo: tBarPayload;``, every ``send target, eFoo, ...`` in the project must construct a ``tBarPayload`` with named fields:
38+
```
39+
// Given: event eNodeSuspected: tNodeSuspectedPayload;
40+
// type tNodeSuspectedPayload = (suspectedNode: machine);
41+
send client, eNodeSuspected, (suspectedNode = node,); // CORRECT
42+
send client, eNodeSuspected, (node); // WRONG
43+
```
44+
2645
Return only the generated P code without any explanation attached. Return the P code enclosed in <Enums_Types_Events.p></Enums_Types_Events.p>.

Src/PeasyAI/resources/instructions/generate_machine.txt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,18 @@ When the Enums_Types_Events.p file is provided as context, you MUST use the EXAC
3636
WRONG: fun HandlePromise(msg: tPromise) {{ ... msg.propNum ... msg.value ... }}
3737
The function parameter type annotation MUST match the event's payload type exactly as declared.
3838

39+
## File-scope helper functions:
40+
Some machines provide convenience helper functions declared OUTSIDE the machine body (at file scope). For example, a Timer machine may provide:
41+
```
42+
fun CreateTimer(client: machine) : Timer {{
43+
return new Timer(client);
44+
}}
45+
fun StartTimer(timer: Timer) {{
46+
send timer, eStartTimer;
47+
}}
48+
```
49+
If the design document specifies helper functions for this machine, include them after the machine's closing brace.
50+
3951
## Checklist before returning code:
4052
- Every state has handlers, defer, or ignore for EVERY event this machine could receive.
4153
- The start state entry correctly unpacks the constructor payload (if parameterized).
@@ -44,5 +56,6 @@ The function parameter type annotation MUST match the event's payload type exact
4456
- Single-field named tuple VALUES always have a trailing comma: ``(field = value,)``. But type annotations do NOT: ``fun Foo(x: tMyType)`` or ``fun Foo(x: (field: int))``.
4557
- All type names in function signatures match EXACTLY what is declared in Enums_Types_Events.p.
4658
- All field names in payload access (e.g., payload.fieldName) and construction (e.g., (fieldName = value,)) match EXACTLY what is declared in the type definition.
59+
- If the design doc specifies file-scope helper functions, they are included after the machine body.
4760

4861
Verify that the generated code strictly follows all the P syntax rules before considering it final. Return only the generated P code without any explanation attached. Return the P code enclosed in XML tags where the tag name is the filename.

Src/PeasyAI/resources/instructions/generate_machine_structure.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ When the Enums_Types_Events.p file is provided as context, you MUST use the EXAC
9494
CORRECT: on ePromise do HandlePromise; ... fun HandlePromise(msg: tPromisePayload) {{ ... }}
9595
WRONG: on ePromise do HandlePromise; ... fun HandlePromise(msg: tPromise) {{ ... }}
9696

97+
## File-scope helper functions:
98+
If the design document specifies helper functions for this machine (e.g., CreateTimer, StartTimer), include their declarations (with empty bodies) AFTER the machine's closing brace. These are convenience wrappers declared at file scope.
99+
97100
The machine file should NOT contain:
98101

99102
1. Specification monitors (spec keyword)

Src/PeasyAI/resources/instructions/generate_test_files.txt

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,63 @@ test tcBasicPaxos [main=Scenario1_BasicPaxos]:
105105
{{ Proposer, Acceptor, Learner, Client, Scenario1_BasicPaxos }};
106106
```
107107

108+
## CRITICAL — Resolving Circular Dependencies in Machine Wiring
109+
110+
Sometimes there is a circular dependency: Machine A's constructor needs a reference to Machine B, and Machine B's constructor needs a collection of Machine A instances. You MUST resolve this correctly.
111+
112+
Strategy: break the cycle by using an initialization event for one side.
113+
114+
Example — FailureDetector needs a `seq[Node]`, but each Node needs the FD reference:
115+
```
116+
// Step 1: Create nodes WITHOUT the FD reference (they'll get it via an init event)
117+
node1 = new Node();
118+
node2 = new Node();
119+
nodes += (0, node1);
120+
nodes += (1, node2);
121+
122+
// Step 2: Create the FD with the node list
123+
fd = new FailureDetector((nodes = nodes,));
124+
125+
// Step 3: Send the FD reference to each node via an init event
126+
send node1, eNodeInit, (failureDetector = fd,);
127+
send node2, eNodeInit, (failureDetector = fd,);
128+
```
129+
130+
Alternative: if the Node machine accepts its config via constructor payload and has no circular dependency, simply create machines in the right order:
131+
```
132+
// If FD doesn't need node references at construction time:
133+
fd = new FailureDetector();
134+
node1 = new Node((failureDetector = fd,));
135+
node2 = new Node((failureDetector = fd,));
136+
```
137+
138+
The key rule: every machine reference passed in a constructor or event payload must point to an already-created machine that will correctly handle the events sent to it.
139+
140+
## CRITICAL — Named Tuple Construction
141+
142+
When a machine's entry function expects a named-tuple type like ``tConfig = (field1: T1, field2: T2)``, you MUST construct it with named fields:
143+
```
144+
CORRECT: new Machine((field1 = val1, field2 = val2));
145+
WRONG: new Machine((val1, val2)); // anonymous tuple — type mismatch!
146+
WRONG: new Machine(val1); // bare value — type mismatch!
147+
```
148+
For single-field types, include a trailing comma: ``new Machine((field = value,));``
149+
150+
Similarly for ``send``: if ``event eFoo: tPayload;`` and ``type tPayload = (bar: int);``, then:
151+
```
152+
CORRECT: send target, eFoo, (bar = 42,);
153+
WRONG: send target, eFoo, (42); // anonymous — type mismatch!
154+
WRONG: send target, eFoo, (this); // bare value — type mismatch!
155+
```
156+
108157
## Checklist before returning code:
109158
- Every scenario machine is matched by a `test` declaration.
110159
- Every `test` declaration includes `assert SpecName in` referencing the spec monitors from PSpec. Without `assert`, PChecker won't verify any properties.
111160
- All machines used in the scenario are listed inside the braces `{{ ... }}`.
112161
- No events, types, or machines from PSrc are redeclared.
113162
- Machine creation order respects dependencies.
114163
- Single-field named tuples have trailing commas: `(field = value,)`.
164+
- Every machine reference in a constructor payload points to a real, already-created machine that handles the expected events.
165+
- Every `new Machine(...)` and `send ..., eEvent, ...` uses named tuple syntax matching the type definition.
115166

116167
Verify that the generated code strictly follows all the P syntax rules before considering it final. Return only the generated P code without any explanation attached. Return the P code enclosed in XML tags where the tag name is the filename.
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
You are reviewing a P specification monitor file for correctness against the design document's safety properties.
2+
3+
You are given:
4+
1. The generated spec monitor code.
5+
2. All machine source files (showing events each machine sends/handles).
6+
3. The Enums_Types_Events.p file (showing all events and their payload types).
7+
4. The design document (showing the intended safety properties).
8+
9+
Your job is to check the following and return a FIXED version of the spec file:
10+
11+
## CHECK 1: Observes Clause Completeness
12+
The spec must observe ALL events it needs to verify the safety property.
13+
14+
Think step-by-step:
15+
- Read the safety property from the design document.
16+
- Identify which events carry information relevant to that property.
17+
- Verify the `observes` clause includes all of them.
18+
19+
Example — "If a node crashes, it must eventually be suspected":
20+
- Needs `eCrash` (to know which node crashed)
21+
- Needs `eNodeSuspected` (to know which node was suspected)
22+
- Needs `eHeartbeat` (to know a crashed node stopped sending heartbeats)
23+
24+
BAD: `spec ReliableDetection observes eNodeSuspected { ... }`
25+
GOOD: `spec ReliableDetection observes eCrash, eHeartbeat, eNodeSuspected { ... }`
26+
27+
## CHECK 2: Correct Event-to-Machine Tracking
28+
The spec must correctly track which machine an event refers to. Events carry payload with machine references — the spec must use the payload to identify the specific machine.
29+
30+
BAD (tracks nothing specific):
31+
```
32+
on eCrash do {
33+
crashCount = crashCount + 1;
34+
}
35+
```
36+
37+
GOOD (tracks which machine crashed):
38+
```
39+
on eCrash goto CrashHandling with HandleCrash;
40+
```
41+
But wait — `eCrash` has no payload in many designs. If `eCrash` is sent to a specific node, the spec sees the event but doesn't know WHICH node received it. In that case, the spec needs a different strategy.
42+
43+
KEY INSIGHT: Spec monitors observe events globally. `on eCrash do ...` fires for EVERY `eCrash` event in the system. If `eCrash` has no payload, the spec cannot know which machine was the target. The spec must use other observable events (like the absence of heartbeats from a specific node) to infer crashes.
44+
45+
If an event has a payload (e.g., `event eNodeSuspected: tNodeSuspectedPayload;` with `type tNodeSuspectedPayload = (suspectedNode: machine);`), the handler MUST accept the payload parameter and use it:
46+
47+
BAD:
48+
```
49+
on eNodeSuspected do {
50+
suspectedCount = suspectedCount + 1;
51+
}
52+
```
53+
54+
GOOD:
55+
```
56+
on eNodeSuspected do (payload: tNodeSuspectedPayload) {
57+
suspectedNodes += (payload.suspectedNode);
58+
}
59+
```
60+
61+
## CHECK 3: Assertion Logic Matches the Safety Property
62+
The assertions must actually verify what the design document says. Read the property carefully and check that the assert conditions are correct.
63+
64+
Example property: "If a node crashes, it must eventually be suspected"
65+
- This means: after eCrash for node X, eventually eNodeSuspected for node X should fire.
66+
- The spec should track crashed nodes and suspected nodes, and assert consistency.
67+
68+
BAD assertion (checks something unrelated):
69+
```
70+
assert !(node in heartbeatAfterSuspicion),
71+
"Node sent heartbeat after suspicion";
72+
```
73+
74+
GOOD assertion (checks the actual property):
75+
```
76+
// Track that a crashed node was indeed suspected
77+
on eNodeSuspected do (payload: tNodeSuspectedPayload) {
78+
if (payload.suspectedNode in crashedNodes) {
79+
detectedCrashes += (payload.suspectedNode);
80+
}
81+
}
82+
```
83+
84+
## CHECK 4: No Forbidden Keywords
85+
Spec monitors CANNOT use: `this`, `new`, `send`, `announce`, `receive`, `$`, `$$`, `pop`.
86+
These are illegal in monitors and will cause compilation errors.
87+
88+
## CHECK 5: Payload Type Names Match Exactly
89+
All event handler parameter types must match the types declared in Enums_Types_Events.p exactly.
90+
91+
## RESPONSE FORMAT
92+
93+
Return your response in this exact format:
94+
95+
<analysis>
96+
Brief description of what was wrong and how you fixed it.
97+
List each issue found as a bullet point.
98+
</analysis>
99+
100+
<fixed_files>
101+
<Specification.p>
102+
... fixed spec code ...
103+
</Specification.p>
104+
</fixed_files>
105+
106+
Use the ACTUAL filename of the spec file (e.g., Safety.p, Spec.p, Specification.p).
107+
Only include files that changed. If the spec is already correct, return it unchanged.
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
You are reviewing a P test driver file for correctness of machine wiring and initialization.
2+
3+
You are given:
4+
1. The generated test driver code.
5+
2. All machine source files (showing each machine's constructor signature).
6+
3. The Enums_Types_Events.p file (showing all types and events).
7+
8+
Your job is to check ONLY the following and return a FIXED version of the test driver:
9+
10+
## CHECK 1: Dependency Order
11+
Every `new Machine(config)` call must happen AFTER all machines referenced in `config` have been created.
12+
13+
BAD:
14+
```
15+
fd = new FailureDetector((nodes = nodeSet,)); // nodeSet is empty here!
16+
node1 = new Node((failureDetector = fd,));
17+
nodeSet += (node1);
18+
```
19+
20+
GOOD:
21+
```
22+
node1 = new Node((failureDetector = fd,));
23+
nodeSet += (node1);
24+
fd = new FailureDetector((nodes = nodeSet,)); // nodeSet has node1
25+
```
26+
27+
## CHECK 2: Circular Dependencies Must Be Resolved
28+
If Machine A's constructor needs Machine B, and Machine B's constructor needs Machine A, one side MUST use the init-event pattern to break the cycle.
29+
30+
BAD (circular — impossible to satisfy both constructors):
31+
```
32+
fd = new FailureDetector((nodes = nodeSet,)); // needs nodes
33+
node1 = new Node((failureDetector = fd,)); // needs fd
34+
// But fd was created with empty nodeSet!
35+
```
36+
37+
GOOD (break cycle with init-event):
38+
```
39+
// Create FD first with empty nodes (it will get them via registration)
40+
fd = new FailureDetector();
41+
// Create nodes with FD reference
42+
node1 = new Node((failureDetector = fd,));
43+
nodeSet += (node1);
44+
// Send nodes to FD via init event
45+
send fd, eFDInit, (nodes = nodeSet,);
46+
```
47+
48+
ALTERNATIVE GOOD (if the machine supports it — create nodes first, then FD):
49+
```
50+
// If Node can be created without FD and configured later:
51+
node1 = new Node();
52+
nodeSet += (node1);
53+
fd = new FailureDetector((nodes = nodeSet,));
54+
send node1, eNodeInit, (failureDetector = fd,);
55+
```
56+
57+
When resolving a circular dependency:
58+
- Look at which machine's constructor is SIMPLER (fewer fields, or has fields that are easier to provide later).
59+
- Break the cycle on that side by removing the problematic field from its constructor config type and adding an init event instead.
60+
- You MUST also update the machine's code to handle the init event if it doesn't already.
61+
- If the machine already handles an init event pattern (start state waits for a config event), use that.
62+
63+
## CHECK 3: No Empty Collections Passed as Config
64+
If a machine's constructor expects `set[machine]` or `seq[machine]`, the collection MUST contain the actual machines, not be empty or default.
65+
66+
BAD:
67+
```
68+
fdConfig = (nodes = default(set[machine]),);
69+
fd = new FailureDetector(fdConfig);
70+
```
71+
72+
GOOD:
73+
```
74+
nodeSet += (node1);
75+
nodeSet += (node2);
76+
fd = new FailureDetector((nodes = nodeSet,));
77+
```
78+
79+
## CHECK 4: Named Tuple Construction
80+
Every `new Machine(...)` and `send ..., eEvent, ...` must use named tuple syntax matching the type definition exactly.
81+
82+
## RESPONSE FORMAT
83+
84+
If the test driver is correct, return it unchanged.
85+
86+
If there are issues, return the FIXED test driver code. If fixing requires changes to machine files (e.g., adding an init event handler), also return those changed files.
87+
88+
Return your response in this exact format:
89+
90+
<analysis>
91+
Brief description of what was wrong and how you fixed it.
92+
</analysis>
93+
94+
<fixed_files>
95+
<TestDriver.p>
96+
... fixed test driver code ...
97+
</TestDriver.p>
98+
<Enums_Types_Events.p>
99+
... only if you needed to add new init events ...
100+
</Enums_Types_Events.p>
101+
<MachineName.p>
102+
... only if you needed to add init event handlers ...
103+
</MachineName.p>
104+
</fixed_files>
105+
106+
Only include files that changed. If nothing changed, include only the original TestDriver.p.

Src/PeasyAI/resources/rag_examples/Common_Timer/PSrc/Timer.p

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ event eTimeOut;
1111
event eDelayedTimeOut;
1212

1313
machine Timer {
14-
// user/client of the timer
1514
var client: machine;
15+
var numDelays: int;
1616

1717
start state Init {
1818
entry (_client : machine) {
@@ -22,18 +22,22 @@ machine Timer {
2222
}
2323

2424
state WaitForTimerRequests {
25-
on eStartTimer goto TimerStarted;
25+
on eStartTimer goto TimerStarted with {
26+
numDelays = 0;
27+
};
2628

2729
ignore eCancelTimer, eDelayedTimeOut;
2830
}
2931

3032
state TimerStarted {
3133
entry {
32-
// Only fire the timer with a chance of 1/10 to avoid livelocks
33-
if(choose(10) == 0) {
34+
// Bound the number of delays so the timer is guaranteed to
35+
// eventually fire (required for liveness properties).
36+
if (numDelays >= 3 || choose(10) == 0) {
3437
send client, eTimeOut;
3538
goto WaitForTimerRequests;
3639
} else {
40+
numDelays = numDelays + 1;
3741
send this, eDelayedTimeOut;
3842
}
3943
}

0 commit comments

Comments
 (0)