feat: add structured logging and tracing spans to act refactor validation

[coderabbitai]

Summary

PR #102 introduces a validation gate for weaver act refactor (provider, refactoring, and required flags), but no observability infrastructure accompanies it. Operators and developers have no insight into validation failures in production or test environments beyond the returned error message.

Work Required

• Add tracing spans around the validation flow in crates/weaverd/src/dispatch/act/refactor/
• Emit structured debug!/warn! events at each validation decision point (e.g. missing flag detected, unsupported provider received)
• Introduce failure-type metrics covering at minimum:
  • missing_required_flags
  • unsupported_provider
  • unsupported_refactoring
  • malformed_trailing_arguments
• Ensure new instrumentation does not alter the existing DispatchError contract or JSONL transport envelope

Background

Raised as an out-of-scope follow-up from PR #102.
Requested by @leynos.

[leynos]

@CodeRabbit plan

[coderabbitai]

Coding Plan

Summary

• Extend the proven metrics.rs pattern with four new atomic counters and trait methods, reusing the existing increment-and-log helper
• Apply #[tracing::instrument] to parse_refactor_args, validate_provider, and validate_refactoring for span coverage
• Emit structured warn! events with event = "..." keys at each rejection point, aligning with health_event! conventions
• Keep all changes internal to dispatch/act/refactor/; do not touch DispatchError variants or the JSONL envelope

Implementation Steps

Phase 1: Extend Validation Metrics Infrastructure

Establish the four new failure counters and their trait surface inside the existing metrics.rs module, following the PositionMetrics precedent exactly.

Task 1: Add Global Atomic Counters

Declare four new AtomicU64 statics and corresponding public read accessors for observability and test assertions.

• Add static MISSING_REQUIRED_FLAGS_COUNT, UNSUPPORTED_PROVIDER_COUNT, UNSUPPORTED_REFACTORING_COUNT, and MALFORMED_TRAILING_ARGUMENTS_COUNT as AtomicU64::new(0) in crates/weaverd/src/dispatch/act/refactor/metrics.rs
• Provide public fn missing_required_flags_count() -> u64 and equivalents, mirroring position_parse_error_count()

Task 2: Extend the Metrics Trait

Expose increment methods on the existing trait so callers remain decoupled from the statics.

• Add increment_missing_required_flags(), increment_unsupported_provider(), increment_unsupported_refactoring(), and increment_malformed_trailing_arguments() to the PositionMetrics trait (or introduce a new sibling trait if separation is preferred)
• Implement these methods on AtomicPositionMetrics using the shared increment_error_counter helper (which already emits a debug! log per increment)
• Implement no-op stubs on NullPositionMetrics and, if present, CountingPositionMetrics for test determinism

🤖 Prompt for AI agents

Extend `crates/weaverd/src/dispatch/act/refactor/metrics.rs` to introduce four
new atomic failure counters and their trait surface, following the existing
`PositionMetrics` pattern exactly.

**Add Global Atomic Counters**
- Declare four new `static` variables — `MISSING_REQUIRED_FLAGS_COUNT`,
`UNSUPPORTED_PROVIDER_COUNT`, `UNSUPPORTED_REFACTORING_COUNT`, and
`MALFORMED_TRAILING_ARGUMENTS_COUNT` — each initialised as `AtomicU64::new(0)`
- Add corresponding public read accessor functions (`fn
missing_required_flags_count() -> u64`, and equivalents for the other three),
mirroring the existing `position_parse_error_count()` accessor

**Extend the Metrics Trait**
- Add four new increment methods to the `PositionMetrics` trait:
`increment_missing_required_flags()`, `increment_unsupported_provider()`,
`increment_unsupported_refactoring()`, and
`increment_malformed_trailing_arguments()`; introduce a new sibling trait if
separation is preferred
- Implement these methods on `AtomicPositionMetrics` using the existing shared
`increment_error_counter` helper (which already emits a `debug!` log per
increment)
- Add no-op stubs for all four methods on `NullPositionMetrics` and, if present,
`CountingPositionMetrics`, to preserve test determinism

Phase 2: Instrument the Validation Pipeline

Attach tracing spans to validation entry points and emit structured events at each decision branch, wiring in the new metrics at failure sites.

Task 1: Instrument `parse_refactor_args` Entry Point

Wrap the top-level argument parser in a span so all nested validation is contextualised.

• Apply #[tracing::instrument(level = "debug", skip(arguments, metrics))] to parse_refactor_args in arguments.rs
• Record argument_count = arguments.len() as a span field

Task 2: Add Structured Events to Flag Validation

Emit debug! on successful flag consumption and warn! when validation rejects input.

• In RefactorArgsBuilder::build() (arguments.rs), emit tracing::debug!(target: DISPATCH_TARGET, flag = "...", "flag accepted") after each successful extraction
• Before returning Err(missing_requirements_error()), emit tracing::warn!(target: DISPATCH_TARGET, event = "missing_required_flags", ...) and call metrics.increment_missing_required_flags()
• In validate_trailing_extra_arguments, emit tracing::warn!(target: DISPATCH_TARGET, event = "malformed_trailing_arguments", offending = ?tokens, ...) on failure and call metrics.increment_malformed_trailing_arguments()

Task 3: Instrument Provider and Refactoring Validators

Add spans and failure events to the allow-list checks in requirements.rs.

• Apply #[tracing::instrument(level = "debug", fields(provider = %provider))] to validate_provider
• Emit tracing::warn!(target: DISPATCH_TARGET, event = "unsupported_provider", provider = %provider) and call metrics.increment_unsupported_provider() before returning the error
• Mirror the above for validate_refactoring with unsupported_refactoring event and counter
• Ensure DISPATCH_TARGET is imported from crate::dispatch::router

Task 4: Thread Metrics Through Validation Calls

Pass the metrics handle to helper functions that now require it.

• Modify RefactorArgsBuilder::build to accept &dyn PositionMetrics (or the extended trait) so it can call the new increment methods
• Update the call site in parse_refactor_args accordingly
• For validate_provider and validate_refactoring, either pass metrics as an argument or have them return a richer result that the caller uses to increment (choose the approach that minimises churn)

Task 5: Add Unit Tests for New Metrics

Verify counters increment correctly on each failure path.

• In the existing test module under refactor/, add tests exercising each invalid-input scenario
• Assert the corresponding public counter accessor increments by one
• Use #[serial] if tests share global state, consistent with existing PositionMetrics tests

🤖 Prompt for AI agents

Instrument the validation pipeline in `arguments.rs` and `requirements.rs` with
tracing spans and structured log events, and wire the new metrics counters into
every failure site.

**Instrument `parse_refactor_args` (`arguments.rs`)**
- Apply `#[tracing::instrument(level = "debug", skip(arguments, metrics))]` to
`parse_refactor_args`
- Record `argument_count = arguments.len()` as a span field

**Add Structured Events to Flag Validation (`arguments.rs`)**
- In `RefactorArgsBuilder::build()`, emit `tracing::debug!(target:
DISPATCH_TARGET, flag = "...", "flag accepted")` after each successful flag
extraction
- Before returning `Err(missing_requirements_error())`, emit
`tracing::warn!(target: DISPATCH_TARGET, event = "missing_required_flags", ...)`
and call `metrics.increment_missing_required_flags()`
- In `validate_trailing_extra_arguments`, emit `tracing::warn!(target:
DISPATCH_TARGET, event = "malformed_trailing_arguments", offending = ?tokens,
...)` on failure and call `metrics.increment_malformed_trailing_arguments()`

**Instrument Provider and Refactoring Validators (`requirements.rs`)**
- Apply `#[tracing::instrument(level = "debug", fields(provider = %provider))]`
to `validate_provider`
- Emit `tracing::warn!(target: DISPATCH_TARGET, event = "unsupported_provider",
provider = %provider)` and call `metrics.increment_unsupported_provider()`
before returning the error
- Mirror the above for `validate_refactoring` with the `unsupported_refactoring`
event and its corresponding counter
- Import `DISPATCH_TARGET` from `crate::dispatch::router`

**Thread Metrics Through Validation Calls**
- Modify `RefactorArgsBuilder::build` to accept `&dyn PositionMetrics` (or the
extended trait) so it can invoke the new increment methods
- Update the call site in `parse_refactor_args` to pass the metrics handle
through
- For `validate_provider` and `validate_refactoring`, pass metrics as an
argument or return a richer result for the caller to increment — choose
whichever approach minimises churn

**Add Unit Tests for New Metrics**
- In the existing test module under `refactor/`, add tests that exercise each
invalid-input scenario
- Assert that the corresponding public counter accessor increments by exactly
one per failure
- Apply `#[serial]` to tests that share global atomic state, consistent with the
existing `PositionMetrics` test suite

Research

The weaverd crate contains a layered dispatch architecture under crates/weaverd/src/dispatch/act/refactor/. Validation logic resides in arguments.rs (flag parsing, position/trailing-arg contracts) and requirements.rs (provider and refactoring allow-lists). A PositionMetrics trait in metrics.rs already demonstrates the canonical pattern: global AtomicU64 statics, a trait abstraction for production vs test implementations, increment helpers that emit tracing::debug!, and public read accessors. Tracing uses named targets (e.g. DISPATCH_TARGET), the #[tracing::instrument] macro with skip and deferred Empty fields, and structured key=value event fields throughout.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Extend `crates/weaverd/src/dispatch/act/refactor/metrics.rs` to introduce four
new atomic failure counters and their trait surface, following the existing
`PositionMetrics` pattern exactly.

**Add Global Atomic Counters**
- Declare four new `static` variables — `MISSING_REQUIRED_FLAGS_COUNT`,
`UNSUPPORTED_PROVIDER_COUNT`, `UNSUPPORTED_REFACTORING_COUNT`, and
`MALFORMED_TRAILING_ARGUMENTS_COUNT` — each initialised as `AtomicU64::new(0)`
- Add corresponding public read accessor functions (`fn
missing_required_flags_count() -> u64`, and equivalents for the other three),
mirroring the existing `position_parse_error_count()` accessor

**Extend the Metrics Trait**
- Add four new increment methods to the `PositionMetrics` trait:
`increment_missing_required_flags()`, `increment_unsupported_provider()`,
`increment_unsupported_refactoring()`, and
`increment_malformed_trailing_arguments()`; introduce a new sibling trait if
separation is preferred
- Implement these methods on `AtomicPositionMetrics` using the existing shared
`increment_error_counter` helper (which already emits a `debug!` log per
increment)
- Add no-op stubs for all four methods on `NullPositionMetrics` and, if present,
`CountingPositionMetrics`, to preserve test determinism
===============================================================================

Task: 2

Instrument the validation pipeline in `arguments.rs` and `requirements.rs` with
tracing spans and structured log events, and wire the new metrics counters into
every failure site.

**Instrument `parse_refactor_args` (`arguments.rs`)**
- Apply `#[tracing::instrument(level = "debug", skip(arguments, metrics))]` to
`parse_refactor_args`
- Record `argument_count = arguments.len()` as a span field

**Add Structured Events to Flag Validation (`arguments.rs`)**
- In `RefactorArgsBuilder::build()`, emit `tracing::debug!(target:
DISPATCH_TARGET, flag = "...", "flag accepted")` after each successful flag
extraction
- Before returning `Err(missing_requirements_error())`, emit
`tracing::warn!(target: DISPATCH_TARGET, event = "missing_required_flags", ...)`
and call `metrics.increment_missing_required_flags()`
- In `validate_trailing_extra_arguments`, emit `tracing::warn!(target:
DISPATCH_TARGET, event = "malformed_trailing_arguments", offending = ?tokens,
...)` on failure and call `metrics.increment_malformed_trailing_arguments()`

**Instrument Provider and Refactoring Validators (`requirements.rs`)**
- Apply `#[tracing::instrument(level = "debug", fields(provider = %provider))]`
to `validate_provider`
- Emit `tracing::warn!(target: DISPATCH_TARGET, event = "unsupported_provider",
provider = %provider)` and call `metrics.increment_unsupported_provider()`
before returning the error
- Mirror the above for `validate_refactoring` with the `unsupported_refactoring`
event and its corresponding counter
- Import `DISPATCH_TARGET` from `crate::dispatch::router`

**Thread Metrics Through Validation Calls**
- Modify `RefactorArgsBuilder::build` to accept `&dyn PositionMetrics` (or the
extended trait) so it can invoke the new increment methods
- Update the call site in `parse_refactor_args` to pass the metrics handle
through
- For `validate_provider` and `validate_refactoring`, pass metrics as an
argument or return a richer result for the caller to increment — choose
whichever approach minimises churn

**Add Unit Tests for New Metrics**
- In the existing test module under `refactor/`, add tests that exercise each
invalid-input scenario
- Assert that the corresponding public counter accessor increments by exactly
one per failure
- Apply `#[serial]` to tests that share global atomic state, consistent with the
existing `PositionMetrics` test suite

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!