spikerj/spikersoft-issues
1
0
Fork 0
Code Issues 196 Pull Requests Actions Packages Projects Releases Wiki Activity

[Correctness][Go-live] Repair or gate non-functional features (Trellis 3D, decompiler, phone lookup) #412

New Issue
Open
opened 2026-07-05 20:24:40 +00:00 by spikerj · 6 comments
spikerj commented 2026-07-05 20:24:40 +00:00
Owner
Copy Link
Copy Source

Problem — capabilities that present as shipped but do not work:

  1. Trellis 3D never completes — a bad datetime.timezone call throws on every progress/complete write, and the generation coroutine is scheduled onto a blocked event loop, so messages are acked with zero work done.
  2. The decompiler returns fabricated stub source instead of decompiling (the real library is imported, never called).
  3. Phone-number lookup always reports success with invented data, never calling Twilio.

Evidence:

  • SpikerSoft.EventHandlers.Trellis3D.Python/trellis_3d_service.py:371,431,565-579,676
  • SpikerSoft.Workers.Decompile/Services/DecompilationService.cs:397-428
  • SpikerSoft.Workers.Notifications/Handlers/LookupPhoneNumberCommandHandler.cs:82-99

Fix: Trellis — use timezone.utc; run the blocking pika consumer in a thread and marshal to the loop via run_coroutine_threadsafe. Decompiler — call CSharpDecompiler.DecompileWholeModuleAsString over a PEFile (folds into CR-4). Phone lookup — implement the real Twilio Lookup or return an explicit not-implemented failure.

Acceptance criteria: Trellis produces a model and a Completed status; the decompiler returns real source; phone lookup returns real carrier data or an honest failure (never fabricated success).

Effort: M · Related: CR-4.

**Problem — capabilities that present as shipped but do not work:** 1. Trellis 3D never completes — a bad `datetime.timezone` call throws on every progress/complete write, and the generation coroutine is scheduled onto a blocked event loop, so messages are acked with zero work done. 2. The decompiler returns fabricated stub source instead of decompiling (the real library is imported, never called). 3. Phone-number lookup always reports success with invented data, never calling Twilio. **Evidence:** - `SpikerSoft.EventHandlers.Trellis3D.Python/trellis_3d_service.py:371,431,565-579,676` - `SpikerSoft.Workers.Decompile/Services/DecompilationService.cs:397-428` - `SpikerSoft.Workers.Notifications/Handlers/LookupPhoneNumberCommandHandler.cs:82-99` **Fix:** Trellis — use `timezone.utc`; run the blocking pika consumer in a thread and marshal to the loop via `run_coroutine_threadsafe`. Decompiler — call `CSharpDecompiler.DecompileWholeModuleAsString` over a `PEFile` (folds into CR-4). Phone lookup — implement the real Twilio Lookup or return an explicit not-implemented failure. **Acceptance criteria:** Trellis produces a model and a `Completed` status; the decompiler returns real source; phone lookup returns real carrier data or an honest failure (never fabricated success). **Effort:** M · Related: CR-4.
spikerj added the agentic label 2026-07-05 20:24:40 +00:00
spikerj commented 2026-07-06 15:57:41 +00:00
Author
Owner
Copy Link
Copy Source

Slice 1 shipped — PR #140 (phone lookup). The handler returned Success=true + hardcoded CountryCode="US" for any number without ever calling Twilio; now returns an explicit NOT_IMPLEMENTED failure (per this ticket's AC: honest failure is acceptable, fabricated success is not). Attempt still audit-logged; the two tests that codified the fake success were updated. 9/9 pass.

Remaining, per feature:

  1. Phone lookup ✅ (#140)
  2. Decompiler (DecompilationService.cs:397-428) — same fabrication pattern (returns stub source; real ICSharpCode.Decompiler imported but never called). Note the real fix here (CSharpDecompiler.DecompileWholeModuleAsString over a PEFile) is metadata-only — it doesn't Assembly.Load/execute — so repairing it properly also advances #406's "metadata-only inspection" ask. I can do either the honest-failure gate (quick) or the real metadata-only decompile (bigger, folds into #406). Which do you want?
  3. Trellis 3D — the platform-aware method work (#378) just merged and materially repairs the routing; the residual Python timezone/blocked-event-loop bugs in trellis_3d_service.py are GPU/broker-verification-gated (can't validate headless).
**Slice 1 shipped — PR #140 (phone lookup).** The handler returned `Success=true` + hardcoded `CountryCode="US"` for any number without ever calling Twilio; now returns an explicit `NOT_IMPLEMENTED` failure (per this ticket's AC: honest failure is acceptable, fabricated success is not). Attempt still audit-logged; the two tests that codified the fake success were updated. 9/9 pass. **Remaining, per feature:** 1. ~~Phone lookup~~ ✅ (#140) 2. **Decompiler** (`DecompilationService.cs:397-428`) — same fabrication pattern (returns stub source; real `ICSharpCode.Decompiler` imported but never called). Note the *real* fix here (`CSharpDecompiler.DecompileWholeModuleAsString` over a `PEFile`) is **metadata-only** — it doesn't `Assembly.Load`/execute — so repairing it properly also advances #406's "metadata-only inspection" ask. I can do either the honest-failure gate (quick) or the real metadata-only decompile (bigger, folds into #406). Which do you want? 3. **Trellis 3D** — the platform-aware method work (#378) just merged and materially repairs the routing; the residual Python `timezone`/blocked-event-loop bugs in `trellis_3d_service.py` are GPU/broker-verification-gated (can't validate headless).
spikerj commented 2026-07-06 16:05:56 +00:00
Author
Owner
Copy Link
Copy Source

Slice 2 shipped — PR #141 (decompiler). It was both fabricating (skeleton source) and Assembly.Load-ing the untrusted uploaded DLL (#406, reachable by any authenticated user). Removed the load and gated to an honest "Decompilation is currently unavailable." — one fix serving both this ticket and #406. 67/67 Decompile tests pass.

Status:

  1. Phone lookup ✅ sql: theme-switch UX polish — feedback, output reset, lesson instructions reload (#140)
  2. Decompiler ✅ #141 (gated; real metadata-only repair tracked under #406, since that is the restore path)
  3. Trellis 3D — routing materially repaired by #378 (platform-aware method); the residual Python timezone/blocked-event-loop bugs in trellis_3d_service.py need GPU/broker verification (can't validate headless), so they stay flagged.

With #140 + #141, both fabricated-success features fail honestly. Once they merge, the only #412 remainder is the GPU-verification-gated Trellis Python bug — recommend either closing this ticket and tracking that under #411's Trellis/GPU work, or keeping it open narrowly for that one item. Your call.

**Slice 2 shipped — PR #141 (decompiler).** It was both fabricating (skeleton source) *and* `Assembly.Load`-ing the untrusted uploaded DLL (#406, reachable by any authenticated user). Removed the load and gated to an honest `"Decompilation is currently unavailable."` — one fix serving both this ticket and #406. 67/67 Decompile tests pass. **Status:** 1. ~~Phone lookup~~ ✅ #140 2. ~~Decompiler~~ ✅ #141 (gated; real metadata-only repair tracked under #406, since that *is* the restore path) 3. **Trellis 3D** — routing materially repaired by #378 (platform-aware method); the residual Python `timezone`/blocked-event-loop bugs in `trellis_3d_service.py` need GPU/broker verification (can't validate headless), so they stay flagged. With #140 + #141, both fabricated-success features fail honestly. Once they merge, the only #412 remainder is the GPU-verification-gated Trellis Python bug — recommend either closing this ticket and tracking that under #411's Trellis/GPU work, or keeping it open narrowly for that one item. Your call.
spikerj commented 2026-07-06 16:12:15 +00:00
Author
Owner
Copy Link
Copy Source

Trellis Bug 1 shipped — PR #142 (the datetime.timezone.utc → timezone.utc crash at :371/:431). Deterministic; verified by local repro + py_compile (no GPU/broker needed).

Bug 2 spec (needs a GPU/broker smoke test — flagging, not shipping blind):
main() (:676) calls consumer.start_consuming() synchronously inside the asyncio.run(main()) loop. channel.start_consuming() is blocking, so it starves the event loop; when a message arrives, on_message (:565, a pika-thread callback) does asyncio.create_task(process_generation_request(...)) (:574) — but the loop never regains control to run it, and the message is basic_ack'd immediately (:578) → acked with zero work done.

Fix pattern:

  1. In main(), capture the loop (loop = asyncio.get_running_loop()) and pass it to the consumer; run the blocking consumer off the loop: await loop.run_in_executor(None, consumer.start_consuming).
  2. In on_message, replace asyncio.create_task(coro) with asyncio.run_coroutine_threadsafe(coro, self._loop) so the coroutine runs on the now-free loop.
  3. (Separate reliability follow-up, #411-adjacent) move the basic_ack to after the coroutine completes — and note pika requires acks on the connection thread (connection.add_callback_threadsafe), which is the subtle part that most wants runtime verification.

I can implement (1)+(2) on a go-ahead if someone can run a GPU/broker smoke test on the branch; the ack-timing (3) I'd keep as its own change.

#412 overall: phone lookup ✅ (#140), decompiler ✅ (#141), Trellis Bug 1 ✅ (#142); remaining is Trellis Bug 2 (specced above, verification-gated).

**Trellis Bug 1 shipped — PR #142** (the `datetime.timezone.utc` → `timezone.utc` crash at `:371`/`:431`). Deterministic; verified by local repro + `py_compile` (no GPU/broker needed). **Bug 2 spec (needs a GPU/broker smoke test — flagging, not shipping blind):** `main()` (`:676`) calls `consumer.start_consuming()` synchronously inside the `asyncio.run(main())` loop. `channel.start_consuming()` is blocking, so it **starves the event loop**; when a message arrives, `on_message` (`:565`, a pika-thread callback) does `asyncio.create_task(process_generation_request(...))` (`:574`) — but the loop never regains control to run it, and the message is `basic_ack`'d immediately (`:578`) → **acked with zero work done**. Fix pattern: 1. In `main()`, capture the loop (`loop = asyncio.get_running_loop()`) and pass it to the consumer; run the blocking consumer off the loop: `await loop.run_in_executor(None, consumer.start_consuming)`. 2. In `on_message`, replace `asyncio.create_task(coro)` with `asyncio.run_coroutine_threadsafe(coro, self._loop)` so the coroutine runs on the now-free loop. 3. (Separate reliability follow-up, #411-adjacent) move the `basic_ack` to *after* the coroutine completes — and note pika requires acks on the connection thread (`connection.add_callback_threadsafe`), which is the subtle part that most wants runtime verification. I can implement (1)+(2) on a go-ahead if someone can run a GPU/broker smoke test on the branch; the ack-timing (3) I'd keep as its own change. **#412 overall:** phone lookup ✅ (#140), decompiler ✅ (#141), Trellis Bug 1 ✅ (#142); remaining is Trellis Bug 2 (specced above, verification-gated).
spikerj commented 2026-07-06 16:35:12 +00:00
Author
Owner
Copy Link
Copy Source

Decompiler upgraded from gate → real fix — PR #143. The fabricated // Method implementation would be here skeletons are gone; each type now carries its real DecompileTypeAsString output, served verbatim by GetDecompiledFileAsync. Feature restored, and it closes the #406 Assembly.Load concern properly (metadata-only). 65/65 tests pass.

#412 status: phone lookup ✅ (#140), decompiler real fix ✅ (#143, replacing the #141 gate), Trellis crash ✅ (#142). The only remaining item is the Trellis event-loop refactor (Bug 2) — specced above, GPU/broker-verification-gated. Once #143 merges, everything here except that one verification-gated item is done.

**Decompiler upgraded from gate → real fix — PR #143.** The fabricated `// Method implementation would be here` skeletons are gone; each type now carries its real `DecompileTypeAsString` output, served verbatim by `GetDecompiledFileAsync`. Feature restored, and it closes the #406 `Assembly.Load` concern properly (metadata-only). 65/65 tests pass. **#412 status:** phone lookup ✅ (#140), decompiler real fix ✅ (#143, replacing the #141 gate), Trellis crash ✅ (#142). The only remaining item is the **Trellis event-loop refactor (Bug 2)** — specced above, GPU/broker-verification-gated. Once #143 merges, everything here except that one verification-gated item is done.
spikerj commented 2026-07-18 18:00:02 +00:00
Author
Owner
Copy Link
Copy Source

Carry-over from #525 (assets→S3, closed in the #413 epic accuracy pass): when Trellis is repaired, its worker must mirror GLB outputs to the assets bucket worker-side (backend #259 already gave ThreeDModelAssetStore a bucket fallback for GLB downloads, and file-movement routes /app/assets — but the Trellis worker writing assets/3d-models directly bypasses both, so its outputs won't reach the bucket unless the worker uploads them). Please include that in the repair's acceptance so the #413 cutover doesn't strand fresh Trellis outputs on disk.

Carry-over from #525 (assets→S3, closed in the #413 epic accuracy pass): when Trellis is repaired, its **worker must mirror GLB outputs to the `assets` bucket worker-side** (backend #259 already gave `ThreeDModelAssetStore` a bucket fallback for GLB downloads, and file-movement routes `/app/assets` — but the Trellis worker writing `assets/3d-models` directly bypasses both, so its outputs won't reach the bucket unless the worker uploads them). Please include that in the repair's acceptance so the #413 cutover doesn't strand fresh Trellis outputs on disk.
spikerj commented 2026-07-22 02:58:15 +00:00
Author
Owner
Copy Link
Copy Source

Board-sweep status (2026-07-22): decompiler real (#143), phone-lookup honest failure (#140), Trellis TZ crash fixed (#142). REMAINING: Trellis repair-vs-retire decision (CI is dispatch-only) + GLB→assets-bucket mirror carry-over.

Board-sweep status (2026-07-22): decompiler real (#143), phone-lookup honest failure (#140), Trellis TZ crash fixed (#142). REMAINING: Trellis repair-vs-retire decision (CI is dispatch-only) + GLB→assets-bucket mirror carry-over.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels agentic
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#412