Hacker News new | ask | show | jobs
by zbentley 3 days ago
This code seems to be fairly pervasively AI-written (both structurally/syntacticly, with many additional AI verbosity tells in the documentation, and going by the commit history).

That's a problem for me, given what this library does: arbitrary transformations of Python code at load time. The fragility, security risk, and complexity added by basically writing a subdialect of Python with new semantics is something I'd like to see more human attention on and maturity of before I consider using it. There are times when AI smell doesn't concern me much when deciding whether or not to use someone's code. This isn't one of those times.

1 comments

Sure man thanks for the critique, genuinely....

To be precise..the core concurrency shapes, disjoint-chunk commit ordering and DAG-level wavefront scheduling, are model-checked (SequentialEquivalence; DependencySafety + Termination), continuously re-verified in CI via a second independent executable encoding. The TLA+ file itself is bounded to a small instance and a 2-chunk model... the paired Python checker checks all contiguous chunk counts and all commit-order permutations for n up to 6.

Conflict detection isn't in the TLA+ file, it's verified separately .. unit and integration tests exercise the real dispatch code against an independent golden sequential run, covering duplicate-key writes, unresolved-index writes under an explicit "depend=none" assertion, and all three error modes (report/quiet/hard), plus a real module-import-level integration test and a concurrency stress test hammering the same conflict path across 10 threads x 6 repeats to catch anything nondeterministic.

Also a note: this is v1.1. The rigor was front-loaded by me, and not something that evolved over years ...so there can be a chance of more rigor or checks...currently working on that.

If you find a bug or a security issue, please open one, genuinely want the feedback.

I do appreciate the rigor that was included, and please don't take my comment above as saying "this is a bad project"; rather, it's not a project I personally am comfortable using at this time due to the combination of its purpose, maturity, and development methodology.

Formal verification is great, and I wish more authors included TLA+. However, changes to another platform's semantics are difficult to formally verify, since the scope of things that could affect those semantics is largely outside of the proof area for most specs. Put another way: you can verify the structures and behavior of input code you specify, but it's difficult (especially in a language with as much dynamism and action-at-a-distance as Python) to verify interactions with others' contexts.

Off the top of my head, some things that users might run into that the testing system doesn't yet take into account include: signal handling/interrupts, orphaned-thread capture/detection/reporting/killing when Lucen-rewritten pariters crash or are aborted (e.g. due to OOM), destructors that might end up being run in unexpected thread/process contexts (given the scope changes in the generated Python code), and potentially unexpected/extra-iteration behavior that occurs due to retries when dispatch to the executor fails partway. That last one might well be handled already via transactionality.

Some things that I think might already be handled, but wasn't quite sure of upon a cursory reading of the code include: operator overloading on user-provided things being accumulated/iterated over, user redefinition of builtin function or exception types (e.g. "def enumerate()"), and making sure the parallel executors are "eagerly" spawned (it sucks to find out mid-execution that you've hit a max-threads or max-processes limit).

Separately and entirely subjectively, a few improvements that might be nice (which I can file as feature requests if you'd like, but don't want to drive-by a bunch of stuff that's entirely personal preference):

I think I'd recommend using only threads for the backend unconditionally. ProcessPoolExecutor brings a lot of extra considerations, overhead, and risk that I think might both throw off your profitability analysis and bring more trouble than it's worth over the long term.

Additionally (though this is high-effort), I think it might be a usability improvement to use a codec for codegen rather than an import hook. Lots of big Python projects have a fairly difficult-to-control import order (because of e.g. frameworks/preloaders/multiple entry points). Unimporting/reimporting stuff is a struggle when it comes to locally activating Lucen on a per-file basis, and in those environments it may be a bit of a usability hassle to activate it centrally. Using a codec gives you per-Lucen-able file locality pretty well.

Lastly, it would also be a potential usability win to provide a real decorator or context manager which enables (in preprocessed files) or disables (nooping unless in preprocessed files) Lucen. That could be in addition to the magic comment, or instead of it (via some cheeky hack like failing to parse all syntax that doesn't refer to the fully qualified 'lucen.mydecorator' name, so you don't have to deal with full name resolution). I think such a decorator would be fairly simple to implement via a contextvar flag in the decorator which the generated code checks, and falls back to the original Python if it's unset, though that would expand the volume of generated code.