Deep review 2026-06-11: correctness fixes, IC generator overhaul, codegen & tooling#413
Closed
darktorres wants to merge 114 commits into
Closed
Deep review 2026-06-11: correctness fixes, IC generator overhaul, codegen & tooling#413darktorres wants to merge 114 commits into
darktorres wants to merge 114 commits into
Conversation
master raised the policy max to 3.27...3.31 (commit 9975437, for CMP0156 whole-archive linking). That also flips CMP0155 to NEW, which auto-enables C++20 module dependency scanning: ninja generates CXX.dd dyndep files and unity builds break, since scanned sources compile individually instead of being batched (18 unity TUs + 88 lone files, vs 120 unity TUs / 0 lone with scanning off). wiRedPanda uses no C++20 modules, so the scan is pure overhead. Set CMAKE_CXX_SCAN_FOR_MODULES OFF: keeps the CMP0156 policy bump, drops the unwanted scanning, restores full unity batching.
calculatePriorities treated any successor pending on the explicit DFS
stack as a feedback edge, but the stack also holds pending *siblings* —
on the cycle-free graph A->B, A->C, C->B iterated [A,B,C] it produced
{C:1, B:1, A:2} instead of {B:1, C:2, A:3}, and a different iteration
order gave a different result. A consumer could then evaluate at or
before its producer in the same tick; pure DAGs run the non-settling
fast path, so nothing masked the mis-order and an edge-triggered
flip-flop could latch a stale value permanently.
Cycle-free graphs now use a two-phase iterative DFS with proper
post-order assignment: deterministic, iteration-order independent, and
linear in nodes + edges.
Cyclic graphs keep the legacy ordering verbatim
(PrioritiesInternal::legacyCalculatePriorities): their single-pass order
is not load-bearing for correctness because the simulation switches to
iterative settling over all elements whenever feedback is present, and
the gate-built latch ICs (SR/JK master-slave) settle correctly under the
legacy order — zero-delay latch races have no graph-derivable correct
order, and reordering flips which stable state a latch reaches from an
undetermined start (verified against the level1 JK flip-flop IC).
Adds the diamond regression test (three iteration orders) and a
cycle-ordering test to TestPriorities.
QBitArray::toggleBit on a position >= 2048 is an out-of-bounds heap write on the truth table's 2048-bit key in release builds (Q_ASSERT in debug — reproduced via the new regression test before the fix). The position was validated only as non-negative, at every layer. Guard all four layers: - ToggleTruthTableOutputCommand::redo() throws for positions outside the key — the model boundary shared by UI and MCP, with undo()==redo() - handleToggleTruthTableOutput rejects position >= 256 * outputSize() with a descriptive ValidationError - schema-mcp.json gains maximum: 2047 on command and response - the Pydantic client model gains le=2047
A crafted or corrupt .panda file could install a key shorter than the 2048 bits updateLogic() indexes (256*output + row), turning every simulation tick into an out-of-bounds QBitArray read. readBoundedBitArray only bounds the array against the stream size, not against the class invariant. setkey() is the single mutation point (load is its only caller; the constructor already establishes 2048 bits), so it now restores the invariant — padding with zeros or truncating. testSetKey's expectation of a 16-bit key asserted the invariant violation itself and is updated to the enforced contract.
With a non-power-of-two data width the select lines can encode values that address no data input. Mux::updateLogic then either read a select line back as data (totals 4-6 and 8-10: silently wrong output in an educational tool) or indexed past the input vector entirely (total 7: out-of-bounds read, reproduced as a QList::at assert by the new regression test before the fix). Out-of-range select is indeterminate routing — the honest four-state answer is Unknown, not a clamp. Demux had the same shape (memory-safe, but out-of-range select silently dropped the data as all-Inactive) and is aligned to all-outputs-Unknown for consistency. Closes the F3 test gap: mux/demux now have non-power-of-two width coverage (totals 4, 7, 8).
…s (F7, F20) Two faces of the same element-vs-port mismatch around the BeWavedDolphin model, which holds one row per port: F7 — loadSignals() snapshots input values per *port*, but restoreInputs() consumed the array per *element*: with a multi-port input (rotary) before other inputs, every subsequent element got the wrong saved value, and the rotary itself was "restored" by a setOn(value, port) loop that simply left the last port selected. Closing the waveform sweep silently corrupted circuit input state (reproduced by the new regression test). restoreInputs() now mirrors the snapshot walk exactly and restores a rotary by re-selecting its saved Active port. F20 — the MCP create_waveform handler targeted input_patterns rows and read results back using *element* indices. Rows are now resolved through new canonical accessors (inputRowLabels/outputRowLabels) built by the same enumeration loadSignals uses, so MCP and GUI can never disagree on row layout; returned labels carry the per-port "label[k]" form. testSignalRowEnumerationMatchesModel pins the enumeration to the model layout (multi-port input and output); end-to-end MCP coverage runs with the Python suite.
hasModifiedFiles() compared the workspace's basename against Settings::autosaveFiles(), which stores absolute paths (built with absoluteFilePath in WorkSpace::autosave) — the 'recovered autosave counts as modified' check could never match. After autosave recovery the undo stack is clean, so closing the app asked only the generic confirmation and ~WorkSpace then deleted the autosave file: the crash-recovered work was silently lost. Compare absolute paths. Regression test loads a file registered in the autosave list and asserts hasModifiedFiles() despite a clean undo stack (MainWindow befriends the GUI test class, the established pattern).
mcp_infrastructure moved into the mcp_client package, but ic_builder_base.py (the base for all 69 IC generators) and generate_inline_ic_fixtures.py still imported the old flat module — ic_builder_base's fallback even re-tried the same flat import after inserting MCP/Client into sys.path. Every generator failed at import time (ModuleNotFoundError, reproduced before the fix); the committed .panda fixtures were unaffected but could not be regenerated. Import mcp_client.mcp_infrastructure in both, point the fallback's existence check and error text at the package layout, and verify a level-1 generator imports end-to-end.
QNEConnection::updatePath skipped all geometry when Application::interactiveMode was false — but both MCP modes run non-interactive, so the --mcp-gui window AND headless --mcp export_image painted wire-less circuits (paint() draws path()). Re-enabling interactiveMode was not an option: it gates exception/info dialogs that an automated MCP session cannot dismiss. Split the overloaded flag: new Application::renderingEnabled (default true) gates the geometry work, and only the contexts that truly never paint opt out — TestUtils::setupTestEnvironment and the 12 fuzz harnesses. MCP modes and CLI export paths keep rendering. The former QVERIFY(true) placeholder testConnectionPathUpdate now asserts both behaviors (was red under the old gate).
The empty-canvas context menu enabled Paste only for the legacy mime type, but copy()/cut() write only the current MimeType::Clipboard — right-click Paste was permanently grayed out for data copied by this very app (Edit>Paste was unaffected). Add ClipboardManager::canPaste(mimeData) mirroring exactly the formats paste() reads, and gate the menu on it — gate and capability share one source of truth and cannot drift again. Unit test pins the predicate to copy() output, the legacy format, and non-circuit data.
Display7's pre-v1.6/v1.7 pin remap permutes the input vector via setInputs(), which left each port's index() at its old position. QNEConnection::save() derives connection serial IDs from port->index() while GraphicElement::save() numbers ports by vector position — after auto-migration resaved a pre-1.6 file, connections to the display referenced the wrong pins on the next load. setInputs() (whose only caller is that remap) now re-assigns port->setIndex(i) after the vector swap. Load-time connection resolution is pointer-keyed and unaffected. Regression test permutes a Display7's ports and asserts index() == position (red before the fix).
wpanda.rc hardcoded FILEVERSION/PRODUCTVERSION 4,3,0,0 and was added verbatim via target_sources — the target's VERSION property does not emit VERSIONINFO, so every Windows build since 4.3 shipped Explorer file properties, installer upgrade data, and crash-report metadata saying 4.3.0.0 while the project is at 5.1.2. Template the file as wpanda.rc.in (the Doxyfile.in pattern) with @PROJECT_VERSION@ substitution and an absolute icon path (the generated file lives in the build tree). configure_file runs on every platform so template breakage surfaces in any CI run; only the target_sources hookup stays WIN32-only. Verified: the generated rc carries 5,1,2,0.
…ure throw (F19, F23, F22) F19 — both generators emitted only TruthTable output 0: a multi-output table exported code where outputs 1..N silently held their initial value. Each generator now loops the outputs, reading key bits 256*k + row; the single-output text stays byte-identical (golden files unchanged, verified by the export suites). Disconnected extra outputs emit a comment instead of throwing. F23 — SystemVerilog declared module inputs only for Button/Switch/Clock; a rotary's ports got aux wires with no driver, leaving floating (z) signals wherever a rotary fed logic. Rotary positions are now one-hot module inputs (label_k), mirroring Arduino's per-pin mapping; the aux declaration skip covers the newly-mapped ports. F22 — the SV constructor returned silently when the output file could not be opened; generate() streamed into a device-less QTextStream and the UI reported success with no file written. It now throws exactly like ArduinoCodeGen. All four regression tests verified red against the pre-fix code.
F8 - setVerbosity: data-driven loop replaces the fall-through cascade;
category five is reachable at verbosity 5 and values >5 mean max
verbosity instead of silencing everything (regression-tested)
F9 - ElementFactory::textToType maps QMetaEnum's -1 to Unknown, matching
its documented contract and callers' == Unknown checks (the test
row asserting raw -1 encoded the bug and is updated)
F10 - CLI export modes (-a/-w/-c) print a stderr error and exit 1 when
the input circuit file is missing instead of silently succeeding
F11 - FlipCommand undo text: chained .arg — the (value, fieldWidth)
overload never substituted %2
F12 - D/JK/SR/T flip-flops reset m_simLastClk to Unknown when inputs go
invalid: recovery with the clock already high can no longer
fabricate a rising edge from stale state (regression-tested red:
pre-fix latched a definite, wrong Inactive)
F13 - InputRotary::setOn clamps negative positions (raw file values
reach it via load); previously no port was selected at all
F14 - Dolphin AutoCrop sizes by input *ports* like combinational mode,
not elements (wrong table length with multi-port inputs); also
gains the kMaxSimLength cap
F15 - UpdateChecker records the daily-check date on any successful,
parsed reply (the single writer) — previously the date was only
written when a dialog showed, so the GitHub API was hit on every
launch while no update existed; network failures still retry
F16 - removed the four stray f-prefixes (ruff F541) in Scripts and the
fixtures generator; ruff clean
F21 - MCP connect_elements enforces ConnectionManager::isConnectionAllowed
— duplicates, second drivers, and wireless-port wires are rejected
with a ValidationError instead of degrading to Error status
F36 - RecentFiles watches entries restored from settings, matching its
own constructor comment (deletions now update the menu live)
F37 - the [ / ] property shortcuts route Clock/Buzzer frequency and
display color changes through UpdateCommand: undoable and
autosave-dirty, like the adjacent port-size path
F38 - ElementEditor guards empty selections before m_elements[0] in
truthTable(), setTruthTableProposition() (size()>1 passed for
empty) and audioBox()
F39 - Scene::unsortedElements() serves the hot paths (key triggers,
mute) that don't care about evaluation order — elements() ran a
full topological sort on every keypress
F40 - TruthTable::updateLogic computes the row index once with integer
bit ops instead of building and re-parsing a binary QString per
output per tick
F41 - dropped IC::resetInternalState's setInputSize(0)/setOutputSize(0):
silent no-ops after the first load (min==max locked), documented
F42 - outputChanged() doc no longer claims it resets the flag
F43 - removeICFile matches ICs by their backing file name instead of
label() + ".panda" — renamed instances were silently skipped
F44 - MCPValidator caches compiled json_validators per schema identity
(command:x / response:x / response:base) — schemas are immutable
after load and a fresh full compile per validation call was the
dominant per-request cost
F45 - removed protocol/type_models.py's duplicate ElementCreationResult
and OutputValueResult: protocol/results.py holds the canonical,
package-exported definitions (the copies had already drifted on
frozen=)
F46 - rotate_element schema documents the existing modulo-360
normalization, resolving the request/response bound asymmetry
without rejecting the convenience input
F47 - change_input_size/change_output_size gain a global maximum of 256
in the schema and le=256 in the Pydantic models; per-element
bounds remain enforced by the server
Full lint stack (ruff + pyright + mypy) clean over MCP/Client.
- level1 DFF: or_s/or_r gates renamed to match their actual function (label swap only — wiring identical); comments corrected (F31) - level6_alu_8bit: CarryIn/SubCarryIn exposed as boundary inputs (SubCarryIn saved ON so the unconnected default keeps standalone SUB correct), SubCarryOut output added, Zero/Negative retapped to the final mux3 results instead of the 5-way selector (they reflected A XOR B for NOT/SHL/SHR), dead Mux8way layer deleted, shift tap lists renamed to their true function (F26, F31) - level7_alu_16bit: ADD and SUB carries chained between the byte halves (16-bit arithmetic was wrong whenever the low byte carried); Zero is AND of the half-Zeros, not NOR (F26) - level6_stack_pointer_8bit: PUSH/POP implemented — adder B becomes 0x01 (Pop) / 0xFF (Push) via B[0]=Push|Pop, B[1..7]=Push, with a per-bit hold mux ahead of the priority mux; dangling Enable removed (F26) - level6_ram_256x8 renamed to level6_ram_8x8 matching its actual capacity; the three consumers wire the low 3 address bits and document the partial decode; stack_memory_interface drops its dangling MemRead/Enable inputs (F26) Fixture regeneration and Level 6-9 test updates follow in part 2.
…F26, F32-F35) F26 — remaining circuit defects: - multi_cycle_cpu_8bit: wire the full datapath (regfile operands into the execute stage, RawInstr-addressed memory stage, DataOut write-back into R0 gated by AND(RegWrite, Phase3)); InstrLoad Vcc instead of GND; reuse the existing NotQ0 for the counter feedback instead of a duplicate gate; drop clock connects to the now-combinational decode/execute stages. - cpu_16bit_risc: wire the fetch instruction fields into the 16-bit ALU (OperandA = zero-extended SrcBits, OperandB = zero-extended DestReg, ALUOp = OpCode[0..2]) so the CPU actually computes. - cpu_program_counter_8bit: remove the dangling WriteEnable input and the Vcc feed in fetch_stage / clock connects in single_cycle_cpu that fed it. F32 — dedupe identical generator pairs into parameterized builders: level4_comparator_4bit and level6_register_file_8x8 are now thin wrappers over the level3/level5 builders (same circuits, different output names). The register-file builder keeps the explicit Vcc on ~Preset/~Clear from the level-6 copy (behavior-neutral; unconnected pins default inactive). F33 — decode/execute stages: remove the unused embedded Clock elements and dead Reset switches (both stages are purely combinational). F34 — make implicit unconnected-port constants explicit GND: PISO MSB shift-in In1[3], program-counter adder B[1..3] (4-bit) and B[1..7] (8-bit). F35 — barrel rotator: left/right rotation formulas in the docstring were swapped relative to the wiring.
… (F26, F31-F35) Regenerate the 24 .panda fixtures affected by the generator fixes (the modified generators plus the two interface-affected dependents, level7_execution_datapath and level9_fetch_stage_16bit, whose child ICs gained/lost boundary ports). level6_ram_256x8 is renamed to the honest level6_ram_8x8 (3-bit address, 8 words). Test updates, asserting the now-correct behavior: - TestLevel6Ram256x8 -> TestLevel6Ram8x8 (class, registry, CMake, golden) - TestLevel6ALU8Bit: structure 21 in / 12 out (CarryIn, SubCarryIn, SubCarryOut ports added by F26) - TestLevel7ALU16Bit: new cross-byte carry/borrow cases for ADD and SUB (the byte halves' carry chains are now connected) - TestLevel6StackPointer8Bit: Enable input removed; new push/pop (SP -1/+1) and reset-to-0xFF behavioral tests - TestLevel6StackMemoryInterface: MemRead/Enable inputs removed; SP is reset explicitly before asserting the SP-addressed path - TestLevel7CPUProgramCounter8Bit: dangling WriteEnable input removed - TestLevel8DecodeStage/ExecuteStage: Clock/Reset inputs removed (F33, the stages are purely combinational) SystemVerilog goldens regenerated for the 16 changed netlists. The behavior-neutral regens (comparators, PISO, PCs, register files, barrel rotator, D flip-flop) kept their tests and goldens green with zero expectation changes, as required. Full suite: 176/176 green.
…F30) F24 — disposition all 46 remaining QVERIFY(true) placeholders: - Made real (behavioral assertions added): LengthDialog accept/reject via queued close; TrashButton drag-enter filtering and drop->removeICFile (direct protected-handler calls — sendEvent drop needs platform drag state the offscreen QPA lacks); ElementTabNavigator tab/backtab/wrap; ElementEditor scene connect/disconnect, label propagation, combos; Display7/14/16 paint (new TestUtils::pixmapHasInk asserts ink on the canvas); port-name tooltips; ICRegistry cache+invalidate; Led custom appearance persisted-state roundtrip; QNEConnection destructor port detach; GraphicsView fast-mode render hints; CircuitExporter produces a loadable image; LanguageManager persists the language; DolphinSerializer empty-model roundtrip + deterministic corrupt-stream rejection; FileUtils no-op/dep-chain copy outcomes; Simulation clock add/remove, SR-latch feedback detection via isInFeedbackLoop, mid-run removal; MainWindow about-dialog actually appears. - QSKIP with honest reasons: ElementPalette (needs MainWindowUi; covered by TestMainWindowGui), UpdateChecker (needs network reply injection). - Kept as documented survival markers: TestDanglingPointer's five crash-regression tests, where reaching the line IS the assertion. The ElementEditor work exposed a real crash: apply() dereferenced a null m_scene when editing with a stale selection after setScene(nullptr) (tab change). The guard now includes !m_scene. F28 — tautological tests rewritten to assert actual behavior: - TestWorkspaceFileops extension tests now save through the workspace and assert the produced file name (append + no-duplication). - testLoadCorruptedFileHandling now requires the Pandaception instead of accepting any outcome. - TestMux constructor trio collapsed into one structural test plus real selection-routing and paint tests. F29 — TestRamCell1bit: removed the copy-pasted AND(WE, WE) no-op gate; WE drives the hold-mux select directly. F30 — MCP soft-asserts surfaced: record_limitation() on the test base feeds TestResults.known_issues, and the orchestrator summary reports a distinct LIMITATION count/section instead of folding those cases into PASS (multi-clock dual domain, clock gating, pattern recognition). Assertions unchanged; lint stack (ruff+pyright+mypy) clean. Full suite: 176/176 green.
- F48: .vscode/tasks.json ran the nonexistent ./build/test — point it at ./build/test_wiredpanda. - F49: CLAUDE.md test-class count 132 -> 176, devcontainer Ubuntu 22.04 -> 24.04 (matches .devcontainer ubuntu:24.04), and the pre-refactor "LogicAnd" code-evidence block rewritten against the current And::updateLogic / Clock::updateOutputs sources; DEVELOPER_GUIDE.md 137 -> 176. - F50: ignore aqtinstall.log (already dropped in working directories). - F51: delete dead assets — Misc/text.svg (not even in the .qrc; the Text element uses text.png) and the four JK-latch/T-latch SVGs (no JKLatch/ TLatch element exists; repo-wide grep confirms only D-latch/SR-latch are referenced) — plus their Components.qrc entries. Also fold the implementation-time corrections into the review report: F1's fix is recorded as the landed hybrid (legacy DFS for cyclic graphs, correct two-phase DFS for DAGs) and F46 as reduced to schema documentation (the handler already normalizes the angle). Full suite: 176/176 green.
…bris (F57-F60, F62) - F57: controller_4bit docstring mapping table and test row labels now match the implemented (and test-asserted) ctrl[2] = opcode[3]|opcode[2] - the 10xx range also asserts register write. - F58: priority encoder no longer builds the unused 8-input or_7_to_0 (41 -> 40 elements). - F59: binary counter no longer builds the dead bit-0 XOR gates - bit 0 is a plain NOT(Q0) (31 -> 28 elements). - F60: d_latch comment typo (NOR2 takes S, not R). - F62: stale comments removed (stack_pointer pre-F26 'B inputs to 0' block, ram_8x8 '256x8 total', decode_stage empty clock section, cpu PC removed- WriteEnable note). - Regenerated the two affected fixtures + SystemVerilog goldens (diffs are pure dead-signal removal/renumbering); level tests green unchanged. - Adds .claude/GENERATOR_AUDIT_2026-06-12.md (full 69-generator audit).
…MS-JK (F55, F56) F56 (level1_d_flip_flop): Preset/Clear were injected into the slave latch only. Asserting them while Clock=1 against an opposing master drove both slave NORs low (the invalid Q=Q_bar=0 state) and the forced level was lost on release. Now OR-injected into BOTH latches (7474 semantics). Red-first: testAsyncPresetClearUnderClockHigh failed against the old fixture. Boundary interface unchanged - only the DFF fixture regenerated; all ten DFF-consuming level tests green with zero expectation changes. F55 (level1_jk_flip_flop): rebuilt as the classic pulse-triggered master-slave JK (7476-style). The master now takes its J/K feedback from the SLAVE outputs and is transparent while Clock=1; the slave transfers the master state on the falling edge. This removes the old master self-feedback SCC that had no stable state for J=K=1 (the toggle depended on the simulator's legacy intra-SCC settle order - the order-dependence the F1 hybrid fix preserves). Preset/Clear are forced into both latches, matching F56. Red-first: testHoldDuringClockHigh failed against the old fixture (the old slave leaked J/K changes to Q during the clock-high phase). Arduino goldens for both refreshed (flattened netlists). The generate-mode simavr divergence for feedback ICs is pre-existing and now documented as F63 in the audit report (the untouched sr_latch fails it too); normal-mode TestArduino passes 90/90. SV goldens unaffected (behavioral emission by port signature). Full suite: 176/176.
…rity (F52) The level-5 PC documented 'reset (Reset to 0)' but the input was dead - created and never wired, because level4_register_4bit had no reset port to receive it. And its mux cascade computed D = inc ? Sum : (load ? loadValue : Q), so increment beat load, contradicting the documented 'load > increment > hold'. The 8-bit PC does both correctly; this brings the 4-bit one in line. - level4_register_4bit: new Reset input (active HIGH) -> NOT -> all four DFFs' ~Clear, byte-for-byte the level3_register_1bit pattern (async, family-consistent). Sole consumer is the level-5 PC. - level5_program_counter_4bit: reset wired through to the register; muxes restructured to mirror the 8-bit PC (Mux1 sel = inc AND NOT(load) picks Sum vs hold, Mux2 sel = load picks loadValue) so reset > load > increment > hold. - Red-first: testProgramCounter4BitReset and testProgramCounter4BitLoadBeatsInc both failed against the old fixtures; test4BitRegisterReset covers the new register port (async clear, reset dominates load, clean release). - Regenerated both fixtures + SV/Arduino goldens (register's generate-mode simavr testbench passes). Full suite: 176/176.
… (F54)
The memory stage's Reset input was dead - created and documented ('reset
signal') but wired to nothing, the same class of defect F33 fixed for the
decode/execute stages. Per the implement-don't-remove directive it now
clears the backing memory, built down the RAM stack:
- level4_ram_8x1: new Reset input (active HIGH) -> NOT -> all 8 storage
DFFs' ~Clear (the register_1bit pattern, async).
- level6_ram_8x8: Reset input fanned out to all 8 bit-slice RAMs.
- level8_memory_stage: its Reset now drives the RAM's Reset. Red-first:
testMemoryStageReset failed against the old fixture.
- Consumers whose memory must survive reset tie the new port to an
explicit GND (F34 convention): instruction_memory_interface (program
memory) and stack_memory_interface (stack contents - only the pointer
resets).
- Regenerated ram_8x1 -> ram_8x8 -> imem/stack_memory_interface/
memory_stage (boundary of memory_stage unchanged - the level-9 CPUs get
their reset-to-memory wiring in the final CPU integration commit).
- New tests: testRAMReset (8x8), testMemoryStageReset; RAM 8x8 inputSize
13 -> 14. SV goldens refreshed including the recursive netlists of the
ICs that embed the RAM (fetch stage, CPUs). Arduino normal mode 90/90.
Full suite: 176/176.
The 16-bit ALU's docstring promised a true 16-bit A<<1 / A>>1, but the two 8-bit halves shifted independently with zero fill: SHL lost A[7] -> Result[8] and SHR lost A[8] -> Result[7]. ADD/SUB had been fixed in F26 by chaining carry ports; the shifts had no fill ports to chain. - level6_alu_8bit: the internal GND_SHL/GND_SHR fill constants are now boundary input ports ShlIn (bit-0 fill on SHL) and ShrIn (bit-7 fill on SHR), saved-off so standalone 8-bit behavior is unchanged - the exact F26 carry-port mechanism. inputSize 21 -> 23. - level7_alu_16bit: OperandA[7] -> high.ShlIn and OperandA[8] -> low.ShrIn; the fills are plain operand bits, no extra logic. The outer fills keep their zero defaults. - Red-first: shl_cross_byte_0x0080 (-> 0x0100) and shr_cross_byte_0x0100 (-> 0x0080) failed against the old fixtures; full-width shl_0x4321/ shr_0x8642 rows added too. - Regenerated alu_8bit + its index-sensitive dependents alu_16bit and execution_datapath; SV goldens refreshed (incl. the recursive netlists of execute_stage and the CPUs). Arduino normal mode 90/90. Full suite: 176/176.
…-first fields (F53) + PISO serial input F53 (level9_fetch_stage_16bit, rebuilt to the level8_fetch_stage pattern): - The decoded fields were emitted MSB-first (OpCode[0]=Instruction[15]) against the project-wide index-0-is-LSB convention; they are now OpCode[i]=Instr[11+i], DestReg[i]=Instr[6+i], SrcBits[i]=Instr[i]. - InstrLoad was a dead input: there was no instruction register at all. The stage now has a real 16-bit IR (2x level6_register_8bit; WriteEnable=InstrLoad, Reset wired) plus RawInstr[0..15] unregistered outputs for zero-delay consumers. - The stage also had NO programming interface, so its instruction memory was permanently blank and every field-decode assertion compared 0 == 0. It now mirrors level8: ProgAddr[0..7]/ProgData[0..15]/ProgWrite with a PC-vs-ProgAddr address mux. level9_cpu_16bit_risc: - Passes the programming interface through; InstrLoad held high; PCLoad/PCData tied to explicit GND (F34 convention). - The ALU now decodes from RawInstr LSB-first (single-cycle, the level9_single_cycle pattern): OperandA=RawInstr[0..5], OperandB=RawInstr[6..10], ALUOp=RawInstr[11..13]. - New testCPUComputesOnDecodedFields: programs real ADD/SUB/AND/OR/XOR instructions and asserts the computed Result - the first non-vacuous end-to-end test this CPU has had. inputSize 2 -> 27. PISO (level4_shift_register_piso): the MSB shift-in documented as 'external source (not connected)' is now a real SIN input (saved off = the old zero fill); testSerialInput walks a 1 through the register. Regenerated fetch16, cpu16, piso + SV goldens. Full suite: 176/176.
…memory reset (F54, decode completion) - level8_decode_stage: InstrDecodedLines[0..31] was documented from the start but never built. Implemented as level2_decoder_4to16 on OpCode[0..3] with each line gated by OpCode[4] / NOT(OpCode[4]) (the RegWrite inverter is reused) - the same hierarchical pattern as the 8-bit instruction decoder. outputSize 6 -> 38. testInstrDecodedLinesOneHot sweeps all 32 opcodes and asserts exactly line==opcode fires. - level9_single_cycle_cpu_8bit / level9_multi_cycle_cpu_8bit: connect their Reset to the memory stage's now-functional Reset (F54) and pick up the regenerated decode/memory fixtures in one regen pass. - SV goldens refreshed. Full suite: 176/176. - Audit report updated: all findings F52-F62 implemented (seven commits), per-item status recorded.
… Q_bar coverage Deep review of the four level-1 (storage/latching) generators confirmed all circuits are textbook-correct, including the F55/F56 reworks. Two secondary follow-ups: - create_level1_jk_flip_flop.py was the only level-1 generator inlining raw self.mcp.send_command() calls with hand-rolled error prints and manual counter bookkeeping. Rewrite it to use begin_build/create_element/connect like its three siblings. The MCP command order is preserved, so the regenerated level1_jk_flip_flop.panda is byte-identical (no fixture churn). - TestLevel1SrLatch/TestLevel1DLatch asserted only Q; wire Q_bar and check Q/Q_bar complementarity at every step (the D-FF and JK tests already do). ctest: 176/176.
The slave reset path built a dedicated slave_not_qm = NOT(Qm) gate, but the master latch already exposes Qm_bar (master_nor_qbar) and the D flip-flop has no slave->master feedback, so the master settles independently before the slave evaluates. Source the slave R gate from master_nor2 directly and remove the NOT gate (23/31 -> 22/30 elements/connections). Behavior-neutral: Qm_bar and NOT(Qm) agree for every legal input. Dependents load the D-FF by reference, so only level1_d_flip_flop.panda regenerates. Refreshed the two netlist-derived Arduino goldens (D-FF and the D-FF-flattening binary counter); the binary counter still validates ALL PASS on simavr. ctest: 176/176.
The 5-way selector built its 2:1 mux tree from 4 raw Mux primitives; swapped each for a level2_mux_2to1 IC with a shared InputVcc holding every Enable active (StaticInput, so no new boundary port). Port remap In0/In1/S0/Out -> Data[0]/Data[1]/Sel[0]/Output. No behavior change. Regenerated the fixture, 8 SV goldens (alu_8bit/alu_16bit -> execution_datapath -> CPUs) and the alu_selector Arduino golden. Full ctest 176/176.
Added GreaterIn/EqualIn/LessIn so two 4-bit comparators chain into an 8-bit magnitude comparison: Greater = local OR (allEqual AND GreaterIn), Less = local OR (allEqual AND LessIn), Equal = allEqual AND EqualIn. Shared builder, so both the level-3 and level-4 fixtures gain the ports (empty embedder cascade). Standalone use ties EqualIn high; updated both comparator tests accordingly and added testComparator4BitCascade. Regenerated both fixtures, 2 SV + 1 Arduino golden. Full ctest 176/176.
…F99) Added an active-high enable input EI (forces addr/valid to 0 when low) and a cascade output EO = EI AND no-input-active, so two 8-to-3 encoders chain into a 16-to-4 priority encoder (high stage's EO -> low stage's EI). Empty embedder cascade. Standalone ties EI high; updated the encoder test and added testEnableInputOutput. Regenerated the fixture, SV + Arduino golden. Full ctest 176/176.
…100) Added a CascadeIn input XORed into the final parity on both blocks. Tied low it is a no-op (standalone behaviour unchanged), so chaining a less-significant block's Parity into this CascadeIn extends the parity tree across more bits. Empty embedder cascade. Added testParityCascadeIn to each; regenerated both fixtures, 2 SV + 2 Arduino goldens. Full ctest 176/176.
Added active-high CountEnable (hold), synchronous Load + Data[0-3], and async Clear. Per-bit D path: next -> hold_mux(CE) -> load_mux(LD) -> FF.D (two level2_mux_2to1 each); Clear inverts to the FFs' active-low Clear. The CountEnable hold creates a D=Q path on the structural level1_d_flip_flop, but both the SV and Arduino sequential differentials pass (verified codegen-safe, unlike the register mux-hold of F97). Clear replaced the warm-up-edge init hack; added testClear/testCountEnable/testParallelLoad. Regenerated fixture + SV + Arduino golden. Full ctest 176/176.
…rs (F102) Both shift-type counters kept their async PRESET and gained active-high CountEnable (hold) and synchronous Load + Data[0-3], using the same per-bit shift_in -> hold_mux(CE) -> load_mux(LD) -> FF.D chain as the binary counter. The SV sequential differential passes for both (count-enable hold is codegen-safe here). Normal operation ties CountEnable high; added testCountEnableHold and testParallelLoad to each. Regenerated both fixtures + SV goldens. Full ctest 176/176.
…lear (F103) Establishes a uniform active-HIGH control convention: removed the internal Preset/Clear inverters from level1_d_flip_flop and level1_jk_flip_flop so Preset=1/Clear=1 assert directly (no longer 7474/7476-faithful — consistency over datasheet fidelity in a zero-delay functional simulator). Cascaded to all 10 embedders: every inactive Preset/Clear tie moved from Vcc to Gnd, and the reset-inverter NOTs were deleted where an active-high control now drives the FF clear directly. Renamed the binary counter's active-high 'Clear' -> 'Reset' (no longer clashing with the FFs' active-low... now active-high 'Clear') and the johnson/ring 'PRESET'(active-low) -> 'Init'(active-high). Updated the FF + counter tests. Regenerated 12 fixtures, 13 SV goldens (+stack_memory_interface) and 3 Arduino goldens; SV differential 73/0, full ctest 176/176.
…F104) New .claude/IC_GENERATOR_CONVENTIONS.md records the uniform active-HIGH, PascalCase, one-name-one-meaning control-port convention and its rationale (zero-delay functional simulator -> active-low TTL polarity is an electrical artifact with no functional benefit). Pointer added in CLAUDE.md.
…F105) Renamed control ports to the canonical vocabulary (no behaviour change): enable/EN -> Enable, reset -> Reset, load/LOAD -> Load, Write_Enable/writeEnable -> WriteEnable, loadValue -> LoadValue. Cascaded across ~20 generators (decoders, RAMs, register files, program counters, shift register, clock-gated decoder, instruction decoders, stack pointer, decode stage, both CPUs), their tests, and the CpuHelpers.h / MemoryHelpers.cpp connect-by-label helpers. Regenerated 21 fixtures (dependency order) + 7 SV goldens; SV differential 73/0, full ctest 176/176. Completes the active-high + PascalCase convention (see .claude/IC_GENERATOR_CONVENTIONS.md).
pylint inferred TestResults.known_issues as FieldInfo (from the '= Field(...)' value) rather than its list[str] annotation, raising E1133 where it's iterated. Moved the three list fields into Annotated[list[str], Field(...)] — the idiomatic Pydantic v2 form, matching the int fields in the same model — so the annotation drives inference. pylint 10.00/10; ruff/mypy/pyright clean.
The Lint CI job only ran inside MCP/Client, leaving 76 Python files unlinted: the 72 IC generators (Tests/Integration/IC/Generators/), Scripts/, and Tests/Fixtures/. Add a repo-root ruff.toml mirroring MCP/Client/ruff.toml's rule set so the whole repo shares one standard. Ruff applies the nearest ancestor config, so MCP/Client/ruff.toml keeps governing that tree unchanged. The only carve-out is RUF001-003 for the generators, whose docstrings intentionally use typographic glyphs (x for dimensions like 8x8, bullet points, status emoji) that are correct text, not the ASCII confusables those rules target. Source cleanup to satisfy this config follows in subsequent commits.
Deterministic ruff formatting across the IC generators, Scripts/, and Tests/Fixtures/ (75 files). Pure layout/whitespace — no semantic change. Verified the level2_half_adder generator still emits a byte-identical .panda fixture after reformatting.
ruff check --fix across the IC generators, Scripts/, and Tests/Fixtures/: sorted imports (I001), dropped dead f-prefixes (F541), and PEP 585/604 annotation modernization (UP006/UP015/UP045) — 153 fixes, all semantically inert. Reformat re-settled afterward (no further changes). Verified the half_adder generator still emits a byte-identical fixture. Remaining ruff findings (E501 long lines, SIM/B007/B904/ASYNC240 logic lints) are addressed in follow-up commits.
The 56 remaining over-120-column lines were all the per-generator
'Successfully created … IC (N elements, M connections)' success log
(55 of them) plus one explanatory comment in the PISO shift register.
Split each success f-string at the ' IC (' boundary into two implicitly
concatenated f-strings (identical runtime text) and wrapped the comment.
Verified decoder_2to4 and binary_counter still emit byte-identical
fixtures. No remaining E501 in the non-MCP trees.
Hand-applied the remaining ruff lints (all behavior-preserving): - SIM103: return the boolean condition directly (fix_style, update_pch) - SIM102: collapse nested guards into a single 'and' (parity gen/checker, stack_memory_interface, fix_style) - SIM108: ternary for the decoder source-id selection (decoder 2to4/3to8/4to16) - B007: drop/underscore unused loop vars (alu_selector, multi_cycle_cpu, run_all_generators) - B904: re-raise the mcp_client import diagnostic with 'from None' ASYNC230/240 (blocking pathlib in the one-shot fixture scripts) are ignored in the root ruff.toml, mirroring MCP/Client's documented decision — those guards run before any coroutine is scheduled, so nothing is starved. Verified all eight circuit-affecting generators still emit byte-identical fixtures. ruff check + ruff format --check are now clean across the IC generators, Scripts/, and Tests/Fixtures/.
…ruff The Lint job only fired on MCP/Client changes and only linted that tree, leaving 76 Python files ungated. Widen the path filter to the generators, Scripts/, Tests/Fixtures/, and ruff.toml, and run the two ruff steps from the repo root so they cover every Python tree (each file resolved against its nearest ruff config). The MCP-specific tools (mypy/pyright/pylint/etc.) stay scoped to MCP/Client. ruff from requirements.txt is reused, so the whole repo is checked against a single ruff version. Now enforces what is already locally green.
vulture reports no dead code across the IC generators, Scripts/, and Tests/Fixtures/ at the same --min-confidence 80 used for MCP/Client, so gate them too. Runs from the repo root over the explicit trees. deptry and import-linter stay MCP/Client-only (documented inline): one needs a project manifest, the other enforces package layering — neither fits flat one-shot scripts.
pyrefly reports no type errors across the IC generators, Scripts/, and Tests/Fixtures/. Resolve both import roots (mcp_client and the generators' sibling modules) via --search-path flags rather than a config file, so the MCP/Client pyrefly run (its own pyrefly.toml) is untouched.
Extend mypy to the IC generators, Scripts/, and Tests/Fixtures/, and
enable --check-untyped-defs repo-wide (MCP/Client passes cleanly with it
too) for one consistent strictness. MYPYPATH supplies both import roots.
Fixes (all behavior-preserving — the two touched IC generators and the
inline-IC fixture generator still emit byte-identical output):
- generate_inline_ic_fixtures.py: a typed _element_id() helper narrows the
Optional result payload at one place instead of 36 unchecked subscripts;
(resp.result or {}).get(...) for the 3 optional .get() reads.
- run_all_generators.py: annotate start_time/end_time as datetime | None.
- create_level6_stack_pointer_8bit.py / create_level5_controller_4bit.py:
annotate control_signals; guard the mux source id before connect().
- Scripts/fix_style.py, migrate_panda_files.py: dict/list annotations and a
None-guard on the subprocess stdin close.
deptry and import-linter remain MCP/Client-only.
Add a scoped pyrightconfig.nonmcp.json (extraPaths for both import roots,
include = the three trees) invoked via 'pyright -p'; its non-standard name
is deliberately not auto-discovered so the MCP/Client 'pyright .' run keeps
its defaults. The mypy .result narrowing from the previous commit already
cleared pyright's 40 reportOptionalSubscript errors.
Remaining 2 (pyright-only): classify_include in fix_style.py returned
(None, None), making the unpacked header Optional at the IncludeEntry call
sites. Annotate it to return a str header always ('' when no match — callers
branch on kind, never header) and assert the regex's quoted-group invariant.
fix_style.py is a C++ style tool, not a circuit generator: no fixture change.
Reuse MCP/Client/.pylintrc with one carve-out, --disable=duplicate-code: the IC generators are intentionally parallel one-script-per-IC files sharing a build skeleton, and reuse already proved fragile (it broke the SystemVerilog codegen differential), so the repetition is by design. PYTHONPATH supplies both import roots. Real findings fixed (all behavior-preserving — the two touched generators regenerate byte-identical fixtures): - invalid-name: operandA_inputs/opA_id/... → snake_case in execute_stage and alu_16bit; - no-else-return ×2 (run_all_generators), chained-comparison (fix_style: 0 < end_idx < len); - 12 missing docstrings across update_pch, migrate_panda_files, fix_style, and the inline-IC fixture main(); - wrong-import-position: the post-sys.path mcp_client imports in the fixture file get a scoped pylint disable (they must follow the path bootstrap). Score 9.82 → 10.00/10. deptry and import-linter remain MCP/Client-only.
…rator parent_inline_ic.panda and nested.panda were generated by an older version of the inline-embedding logic and never refreshed: the current generate_inline_ic_fixtures.py emits a more compact blob (10083→9581 B and 15235→14231 B respectively). Regenerate both so the committed fixtures match their generator. TestICInline passes 155/0 against the refreshed fixtures; the other four inline fixtures regenerate byte-identical.
… absent
testArduinoSequentialMultiCycleCpu8Bit compiles and runs a real Arduino
sketch under simavr (unlike the export tests, which only compare goldens),
so it needs arduino-cli + simavr. requireLinuxForArduinoTools() gates only
on the OS, so on stock CI runners (Linux without the AVR toolchain) the test
hard-failed ('arduino-cli not found' → runTestbench returns false → QVERIFY).
Guard it with the same tool-availability QSKIP that testSimavrFunctionalSimulation
already uses, so it skips gracefully when the tools are missing. Verified: it
still PASSes when the tools are present and SKIPs (not fails) when arduino-cli
is off PATH.
testTruthTableMultiOutput (and the other codegen unit tests) read the generated .ino/.sv back and match multi-line patterns containing '\n'. The files were opened with QIODevice::ReadOnly only, so on Windows the codegen's native CRLF line endings (it writes with QIODevice::Text) stayed as '\r\n' and the '\n' matches failed — 'Compared values are not the same'. Open these reads with QIODevice::ReadOnly | QIODevice::Text, matching the convention the passing export tests already use (TestArduino.cpp, TestSystemVerilogExport.cpp). On Linux this is a no-op; on Windows it normalizes CRLF→LF on read. The tests assert logical structure, not line-ending bytes, so this is the correct layer to fix.
TestLevel9SingleCycleCpu8bit.cpp and TestLevel9MultiCycleCpu8bit.cpp each defined enum ALUOp and static encodeInstruction/encodeStore/encodeLoad with identical bodies. Under the mega-unity build (UNITY_BUILD_BATCH_SIZE=1000) both files land in one translation unit, so the duplicates collided: 'multiple definition of enum ALUOp', 'redefinition of int encodeInstruction'. Hoist the shared definitions into Tests/Integration/IC/Tests/Cpu/Cpu8bitIsa.h (pragma-once, inline functions) and include it from both files. One definition per TU, no collision. Verified: a fresh UNITY_BUILD_BATCH_SIZE=1000 build links cleanly, and both Level9 tests (88 + 33) plus the codegen unit tests pass in the unity binary.
The IC preview popup appeared whenever the cursor was over an IC's ports, not just its body. Ports are child QNEPort items that don't accept hover events, so the IC's shape (which unites the ports' bounding rect) received the hover and showForIC() ran unconditionally. Guard the hover handlers with isCursorOverPort(): hide the popup over a port and re-arm it (via ICPreviewPopup::isShowActiveFor) when the cursor returns to the body, so body->port->body within one hover behaves.
Hovering an IC used to trigger two overlapping affordances: Qt's tooltip bubble with the filename and the custom preview popup, which fought for the same screen space. Move the filename into the popup as a framed caption above the rendered circuit and drop the IC's Qt tooltip so only one thing shows. The caption sits inside the inset frame with the preview image, so it reads against the framed background. ICs with no preview image (empty or oversized) now show a name-only popup in place of the old tooltip.
Regenerate the .ts files via the update_translations target, which extracted three previously-untranslated strings, and complete them across all 39 locales: - "Could not open file for writing: %1" (SystemVerilogCodeGen) - "TruthTable toggle position out of range: %1" (Commands) - "Error: no input circuit file given." (Main CLI) Validated with lrelease: 551 finished, 0 unfinished per locale.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deep-review batch off master. Highlights:
Correctness fixes (F1–F47)
IC generators
Code generation
Tests
Tooling / CI
IC hover preview (this session)
i18n
🤖 Generated with Claude Code