Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e62adc4-2b63-412e-a3ec-2bb39bc2d6f5
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e62adc4-2b63-412e-a3ec-2bb39bc2d6f5
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #741 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 583 583
Lines 62930 62930
=======================================
Hits 62930 62930
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
martintmk
left a comment
There was a problem hiding this comment.
Posted by an AI agent
Reviewed the exact PR head across API design, correctness, tests, performance, naming, telemetry, resilience, consistency, public API, and public documentation. I found two blocking issues and one non-blocking documentation issue, all anchored inline.
Focused validation: cargo check -p arty passed. The paired cargo public-api and rustdoc comparisons found no public Rust API or rustdoc changes.
| Arty types must be unwind-safe unless documented otherwise. Foundational I/O contracts in | ||
| `arty_io_core` must also be panic-safe. | ||
|
|
||
| Every spawned task has a join handle that returns a `JoinError` when the task does not complete |
There was a problem hiding this comment.
Posted by an AI agent
The panic policy presents planned runtime behavior as a current guarantee
Problem
PANICS.md now says every spawned task returns JoinError, the first panic starts shutdown, and criticality is inherited; DESIGN.md states the same behavior in the present tense. At this head, arty still exposes only thread-awareness and optional time re-exports, with no spawn API, JoinError, critical-task API, or observed dependency, while STABILIZATION.md explicitly calls the runtime surface planned.
Why this matters
These policy pages are linked from the published crate documentation, so consumers can read fail-fast shutdown, join-error classification, and critical-task completion as guarantees of the current crate even though none of those APIs or behaviors is available.
Suggested fix
Mark the new task, panic, and shutdown sections explicitly as planned design until the corresponding runtime surface is implemented and tested; keep the existing current unwind-safety requirements separate.
| in `arty_io_core`. | ||
|
|
||
| Runtime shutdown is fail-fast for ordinary application work. The first task panic starts | ||
| shutdown, after which non-critical tasks stop being polled. Critical tasks continue until they |
There was a problem hiding this comment.
Posted by an AI agent
Inherited critical tasks can prevent fail-fast shutdown from completing
Problem
The design says every descendant spawned from a critical task is also critical and that all critical tasks run until they finish, but it defines neither a way to spawn ordinary work from that context nor a bounded drain deadline. A critical operation that calls code spawning unrelated or never-ready work can therefore make the entire descendant tree exempt from shutdown indefinitely.
Why this matters
The first panic is supposed to trigger fail-fast shutdown, yet one detached or stalled descendant can turn that path into a permanent hang and prevent telemetry flushing or other high-value cleanup from completing.
Suggested fix
Define a bounded critical-drain phase and an explicit spawn escape hatch that clears inherited criticality for unrelated work. Specify the terminal outcome for critical tasks and join handles when the drain deadline expires.
|
|
||
| Arty's planned runtime surface exposes and uses the telemetry emitter API now provided by | ||
| `observed`, including its `Sink` and `emit!` surface. This API previously lived in | ||
| `observer_core` and is treated as a highly stable dependency surface. Changes to it must preserve |
There was a problem hiding this comment.
Posted by an AI agent · Non-blocking
observer_core names a nonexistent predecessor for the telemetry API
Problem
The stabilization note says the Sink/emit! surface previously lived in observer_core, but repository history identifies the predecessor as the emit crate family (including Emitter being renamed to Sink); no observer_core crate or path exists in the relevant history.
Why this matters
This note is intended to identify the compatibility lineage maintainers must preserve, so the incorrect name sends readers to the wrong contract and obscures the actual rename history.
Suggested fix
Replace the provenance sentence with the established emit → observed and Emitter → Sink lineage, or omit the historical claim.
Summary
JoinErroroutcomes for task panics and runtime shutdownobservedValidation
just package=arty formatjust package=arty readmejust package=arty spellcheckjust package=arty clippyReview
observer_coreprovenance statement; automation reviewers could not verify it from this repository history