findings group P: liftBlob field gap (P-5), apply-slot hardening (P-1), chooser-held rotate (P-4)

- P-5: blobForItem returns "" for PCB_FIELD_T (the file writer cannot format a
  field standalone → hollow (kicad_pcb …) envelope); liftBlob skips empty blobs
  and STRUCT_DELETED roots with a console breadcrumb; the scalar added path and
  the seed never attach an empty blob.
- P-1 (not reproduced, structural): the runOnCoroutine apply body runs in
  try/catch(...) so an unwinding body still marks the slot done instead of
  wedging fit/pan/flushDiff/remote applies forever; new probe
  kicadCollabTestApplyQueueState() → {busy, queued}.
- P-4: wxwidgets 9666a743bc (TLW SetFocus delegates to a child,
  menubar never takes focus); probe kicadTestFocusWindow() (wx FindFocus).
- Specs: tests/kicad/findings-p.spec.ts (P-1 ×3, P-2 probes as fixme, P-3 ×2,
  P-4, P-5), tests/web/findings-p1.spec.ts (P-1 on the real standalone).
  P-2 verdict: stock KiCad candidate priority, text scales normally; P-3 not
  reproducible — see docs/features/findings/groups/P-editor-tools-embind-interaction.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmnMPkjugyGqe4LvUsBB14
This commit is contained in:
Gergő Törcsvári 2026-08-28 12:57:38 +02:00
commit 3d19117d88
No known key found for this signature in database
GPG key ID: 8E75F2CDE64E5322
6 changed files with 861 additions and 5 deletions

View file

@ -124,7 +124,22 @@ inline void drainApplies()
slot.cor = new COROUTINE<int, int>( []( int ) -> int
{
ApplySlot& sl = activeApplySlot();
( *sl.body )();
// P-1 hardening (findings group P): `done` is the ONLY completion signal, so a
// body that unwinds — a C++ exception, or a foreign (JS) exception thrown out of
// an EM_ASM wire callback inside flushDiff and surfacing here as `...` under
// wasm EH — would leave applyBusy() true for the page's life and silently
// queue every later job behind it: fit/pan from JS, remote applies AND the
// local-edit flushDiff (local edits stop syncing). Always mark the slot done.
try
{
( *sl.body )();
}
catch( ... )
{
EM_ASM( { console.error( '[pcbjam collab] apply body threw — slot released' ); } );
}
sl.done = true;
// If we suspended, no drain is pending by the time the body
@ -145,6 +160,14 @@ inline void drainApplies()
}
}
/** Test/diagnostic probe (P-1): is the apply slot busy and how many bodies wait behind it.
* A `busy` that never clears while `queued` grows is the wedge signature. */
inline std::string applyQueueStateJson()
{
return "{\"busy\":" + std::string( applyBusy() ? "true" : "false" )
+ ",\"queued\":" + std::to_string( applyQueue().size() ) + "}";
}
inline void runOnCoroutine( wxEvtHandler* aHandler, std::function<void()> aBody )
{
applyHandler() = aHandler;

View file

@ -502,6 +502,20 @@ static void collabSetViewport( double aCx, double aCy )
}
// Follow-user (collab-presence 0008).
// Test probe (findings P-4): which wx window holds keyboard focus — the wx-side
// `FindFocus()`, not the DOM's activeElement. Keys are delivered there
// (wx wasm app.cpp HandleKeyEvent), so a frame or NULL here means hotkeys are lost.
static std::string kicadTestFocusWindow()
{
wxWindow* w = wxWindow::FindFocus();
if( !w )
return "null";
return std::string( wxString( w->GetClassInfo()->GetClassName() ).utf8_str().data() )
+ ":" + std::string( w->GetName().utf8_str().data() );
}
static void collabFitViewport( double aCx, double aCy, double aHalfW, double aHalfH )
{
pcbEditorActive() ? pcbCollabFitViewport( aCx, aCy, aHalfW, aHalfH )
@ -614,6 +628,8 @@ EMSCRIPTEN_BINDINGS(kicad_editor) {
// Programmatic file open (preferred over UI automation from the web app).
function("kicadOpenFile", &kicadOpenFile PCBJAM_PARKER_POLICY);
function("kicadOpenFileBusy", &kicadOpenFileBusy);
function("kicadCollabTestApplyQueueState", &pcbjam_collab::applyQueueStateJson);
function("kicadTestFocusWindow", &kicadTestFocusWindow);
function("kicadTestSetOpenPark", &kicadTestSetOpenPark);
function("kicadTestArmTimerPark", &kicadTestArmTimerPark);
function("kicadTestTimerParkState", &kicadTestTimerParkState);

View file

@ -407,6 +407,15 @@ std::string wrapInBoardEnvelope( BOARD& aBoard, const std::string& aItemSexpr );
std::string blobForItem( BOARD* aBoard, BOARD_ITEM* aItem )
{
// P-5 (findings group P): the FILE writer cannot format a footprint FIELD standalone —
// PCB_IO_KICAD_SEXPR::Format has an explicit `case PCB_FIELD_T: break;` (fields are
// written by the footprint formatter), so a field reaching here produced a bare
// `(kicad_pcb … (layers …))` envelope with no uuid-bearing item, which the JS wire
// (unwrapWireItem) could not resolve. Fields only ever travel inside their footprint's
// blob; refuse to blob one on its own — callers skip an empty blob.
if( aItem->Type() == PCB_FIELD_T )
return "";
if( aItem->Type() == PCB_FOOTPRINT_T )
{
const FOOTPRINT* src = static_cast<const FOOTPRINT*>( aItem );
@ -866,7 +875,27 @@ void flushDiff()
if( !wDone.insert( rootId ).second )
return;
json w = json{ { "sexpr", blobForItem( board, live ) }, { "parent", nullptr } };
std::string sexpr = blobForItem( board, live );
// P-5: a root the writer cannot serialize standalone (a footprint FIELD whose parent
// is not a footprint, or a deleted item still resolvable through the id cache) must
// not reach the wire as a hollow envelope. Skip it and leave a breadcrumb — the
// trigger that lets a field arrive here unlifted is still unknown
// (memory: items-wire-batch-loss); this line is the hunt.
if( sexpr.empty() || ( live->GetFlags() & STRUCT_DELETED ) )
{
std::string why = sexpr.empty() ? "empty blob" : "STRUCT_DELETED";
std::string type = toUtf8( live->GetClass() );
int parentNull = live->GetParent() == nullptr ? 1 : 0;
EM_ASM( {
console.warn( '[pcbjam collab] P-5: skipped un-serializable dirty root',
{ uuid: UTF8ToString( $0 ), type: UTF8ToString( $1 ),
why: UTF8ToString( $2 ), parentNull: !!$3 } );
}, rootId.c_str(), type.c_str(), why.c_str(), parentNull );
return;
}
json w = json{ { "sexpr", sexpr }, { "parent", nullptr } };
// A lifted child means its (pre-existing) parent's CONTENT changed.
( lifted ? wChanged : aArr ).push_back( w );
@ -897,7 +926,12 @@ void flushDiff()
// wasted SaveSelection plus the envelope parse for those.
if( live && !isTrackType( live->Type() ) && live->Type() != PCB_ZONE_T
&& live->Type() != PCB_TEXT_T )
withBlob["sexpr"] = blobForItem( board, live );
{
std::string sexpr = blobForItem( board, live );
if( !sexpr.empty() ) // P-5: never attach a hollow envelope
withBlob["sexpr"] = sexpr;
}
added.push_back( withBlob );
}
@ -1520,7 +1554,10 @@ std::string pcbCollabSnapshotItems()
{
auto push = [&]( BOARD_ITEM* item )
{
added.push_back( json{ { "sexpr", blobForItem( board, item ) }, { "parent", nullptr } } );
std::string sexpr = blobForItem( board, item );
if( !sexpr.empty() ) // P-5: never seed a hollow envelope
added.push_back( json{ { "sexpr", sexpr }, { "parent", nullptr } } );
};
for( FOOTPRINT* fp : board->Footprints() ) push( fp );