From 9ab950d767adcf69214dd847bebc689a50468fdd Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 2 Aug 2026 15:58:51 +0200 Subject: [PATCH] jobq: drop the append_unchecked redirect (mara review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Scheduler::append_unchecked` was a one-line pass-through to `Graph::insert_unchecked` with a single caller. `insert_job` is already a method on `Scheduler`, so it can borrow `self.graph` and call the graph directly — the wrapper bought nothing but a name. Also fixes `insert_job`'s doc, which still claimed every node goes through `Scheduler::append`; it goes straight to the graph's unchecked insert, and the reason belongs in that doc rather than on a wrapper. --- hive-jobq/src/scheduler.rs | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/hive-jobq/src/scheduler.rs b/hive-jobq/src/scheduler.rs index 12f061b4..337c6b2d 100644 --- a/hive-jobq/src/scheduler.rs +++ b/hive-jobq/src/scheduler.rs @@ -100,21 +100,6 @@ impl Scheduler { self.graph.insert(payload, deps, parent) } - /// [`Scheduler::append`] for a node the builder has already validated — - /// infallible, so the insert loop cannot abandon a half-built job. - /// - /// Not public: the only caller is [`Scheduler::insert_job`], feeding nodes - /// that [`crate::builder::check_job_shape`] has already proved - /// well-formed. See [`Graph::insert_unchecked`]. - pub(crate) fn append_unchecked( - &mut self, - payload: N, - deps: Vec>, - parent: Option, - ) -> NodeId { - self.graph.insert_unchecked(payload, deps, parent) - } - /// Insert a whole job under `root_parent`, returning the id each handle's /// node was minted as. /// @@ -125,10 +110,12 @@ impl Scheduler { /// insert one itself, so there is no way to end up with a job-shaped value /// being passed around as a spec. /// - /// The one insertion entry point: every node goes through - /// [`Scheduler::append`], so a caller never has to reach past the scheduler - /// at the graph underneath. Call [`Scheduler::settle`] afterwards to start - /// whatever became runnable. + /// The one insertion entry point for a job. Nodes go straight into + /// [`Graph::insert_unchecked`]: [`crate::builder::check_job_shape`] has + /// already decided every rejection the graph could raise, so re-validating + /// per node could only report a problem *after* the earlier nodes were + /// inserted. Call [`Scheduler::settle`] afterwards to start whatever became + /// runnable. /// /// **Atomic in the job's own shape.** A forward edge, a forward parent, or /// a request for a handle this job never declared is rejected *before* the @@ -145,8 +132,9 @@ impl Scheduler { ) -> Result, BuildError> { let job = JobBuilder::new(); let wanted = declare(&job); + let graph = &mut self.graph; job.insert_with(root_parent, &wanted, |payload, deps, parent| { - self.append_unchecked(payload, deps, parent) + graph.insert_unchecked(payload, deps, parent) }) }