Actionable comments posted: 1

coordinator/internal/logic/provertask/chunk_prover_task.go (2)

27-32: Consider making universal parallelism configurable.

Hard-coding 2 at package scope makes tuning production behavior require code changes. A config-backed value (with sane default) would be easier to operate under varying prover capacity and API latency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@coordinator/internal/logic/provertask/chunk_prover_task.go` around lines 27 -
32, The package currently hard-codes applyUniversalMaxParallelism = 2 and
constructs witnessSemaphore at package init; make this configurable by reading a
config/env/flag value at startup and initializing applyUniversalMaxParallelism
and witnessSemaphore there instead of at package scope. Specifically: add a
config key (or env var/flag) for universal max parallelism with a sane default,
validate it (ensure >=1), and create witnessSemaphore = make(chan struct{}, max)
during application initialization (e.g., in an init hook or the coordinator
startup/new constructor) so the semaphore capacity reflects the configured
value; update any code that references applyUniversalMaxParallelism to use the
configured variable.

---

212-215: Release the semaphore immediately after applyUniversal completes.

Line 215 defers release until Assign returns, so post-processing (DB insert/metrics/logging) still occupies a slot. Scope the acquire/release around cp.applyUniversal only to keep throughput predictable.

💡 Proposed refactor

 	if getTaskParameter.Universal {
 		var metadata []byte
-
-		select {
-		case witnessSemaphore <- struct{}{}:
-			// Released when Assign returns (defer).
-			defer func() { <-witnessSemaphore }()
-		case <-ctx.Done():
-			log.Warn("context canceled waiting for witness semaphore", "task_id", chunkTask.Hash, "err", ctx.Err())
-			cp.recoverActiveAttempts(ctx, chunkTask)
-			return nil, ctx.Err()
-		}
-
-		taskMsg, metadata, err = cp.applyUniversal(taskMsg)
+		taskMsg, metadata, err = func() (*coordinatorType.GetTaskSchema, []byte, error) {
+			select {
+			case witnessSemaphore <- struct{}{}:
+				defer func() { <-witnessSemaphore }()
+			case <-ctx.Done():
+				log.Warn("context canceled waiting for witness semaphore", "task_id", chunkTask.Hash, "err", ctx.Err())
+				if taskCtx.hasAssignedTask == nil {
+					cp.recoverActiveAttempts(ctx, chunkTask)
+				}
+				return nil, nil, ctx.Err()
+			}
+			return cp.applyUniversal(taskMsg)
+		}()
 		if err != nil {
 			cp.recoverActiveAttempts(ctx, chunkTask)
 			log.Error("Generate universal prover task failure", "task_id", chunkTask.Hash, "type", "chunk", "err", err)
 			return nil, ErrCoordinatorInternalFailure
 		}
 		proverTask.Metadata = metadata
 	}

Also applies to: 222-223

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@coordinator/internal/logic/provertask/chunk_prover_task.go` around lines 212
- 215, The semaphore acquire around the witness work currently holds the slot
until Assign returns because the release is deferred; change the scope so
witnessSemaphore is acquired before calling cp.applyUniversal and released
immediately after cp.applyUniversal completes (before any DB
inserts/metrics/logging or before calling Assign) instead of deferring until
Assign returns; update both places that acquire witnessSemaphore (the block
around cp.applyUniversal and the similar block later) so the defer is removed
and a direct <-witnessSemaphore is executed right after cp.applyUniversal
returns to limit semaphore occupancy to only the applyUniversal duration.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@coordinator/internal/logic/provertask/chunk_prover_task.go`:
- Around line 217-219: The early return on context cancellation is
unconditionally calling cp.recoverActiveAttempts, which must only run when this
request performed the increment (the increment happens in the
taskCtx.hasAssignedTask == nil path). Update the context-cancel branch in
chunk_prover_task.go to check taskCtx.hasAssignedTask == nil (or another flag
that indicates this request incremented attempts) before calling
cp.recoverActiveAttempts(ctx, chunkTask) and before returning; leave the return
of ctx.Err() unchanged when no increment occurred. Ensure the guard uses the
same condition used where attempts are incremented so accounting stays
consistent.

---

Nitpick comments:
In `@coordinator/internal/logic/provertask/chunk_prover_task.go`:
- Around line 27-32: The package currently hard-codes
applyUniversalMaxParallelism = 2 and constructs witnessSemaphore at package
init; make this configurable by reading a config/env/flag value at startup and
initializing applyUniversalMaxParallelism and witnessSemaphore there instead of
at package scope. Specifically: add a config key (or env var/flag) for universal
max parallelism with a sane default, validate it (ensure >=1), and create
witnessSemaphore = make(chan struct{}, max) during application initialization
(e.g., in an init hook or the coordinator startup/new constructor) so the
semaphore capacity reflects the configured value; update any code that
references applyUniversalMaxParallelism to use the configured variable.
- Around line 212-215: The semaphore acquire around the witness work currently
holds the slot until Assign returns because the release is deferred; change the
scope so witnessSemaphore is acquired before calling cp.applyUniversal and
released immediately after cp.applyUniversal completes (before any DB
inserts/metrics/logging or before calling Assign) instead of deferring until
Assign returns; update both places that acquire witnessSemaphore (the block
around cp.applyUniversal and the similar block later) so the defer is removed
and a direct <-witnessSemaphore is executed right after cp.applyUniversal
returns to limit semaphore occupancy to only the applyUniversal duration.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 9b2b5e0 and f8cec5d.

• common/version/version.go
• coordinator/internal/logic/provertask/chunk_prover_task.go