feat(cli): Accept "formula" alias for putf batch ops + add --dry-run flag

[arcaputo3]

• Accept "formula" as alias for "value" in putf batch JSON ops, so
  {"op":"putf","ref":"A1","formula":"=SUM(B1:B5)"} works alongside
  the existing "value" field
• Add --dry-run flag for batch command that validates JSON and shows
  operation summary without reading/writing any files
• Extract formatSummary into BatchParser for reuse across paths
• Update SKILL.md and CLAUDE.md docs with new features

https://claude.ai/code/session_01JeW4MZBQx2Bktmg9HamTK3

[claude]

PR Review: feat(cli) formula alias and dry-run

Overall this is a clean, useful addition. The formatSummary extraction is a nice refactor, and test coverage for the alias feature is solid. A few issues worth addressing.

Bug / Architecture: Duplicate batch subcommand

The PR registers two Opts.subcommand(batch) entries -- batchDryRunOpts and the existing batchCmd inside sheetWriteOpts -- combined via orElse. This is fragile: correctness depends on orElse ordering, and Decline may behave unexpectedly with duplicate subcommand names.

A simpler approach: batchCmd already parses dryRunOpt (you added it). The dryRun field should be checked in the existing dispatch logic -- when dryRun=true and no --file is provided, run the dry-run path. This removes the duplicate subcommand entirely.

Side Effect Inside IO.map

In batchDryRun: result.warnings.foreach(System.err.println) inside .map is a raw side effect violating the IO effect model. Use IO.delay or flatMap instead.

Code Smell: @unchecked Pattern Match

In runBatchDryRun: val CliCommand.Batch(source, _) = cmd: @unchecked crashes at runtime if cmd is not a Batch. Prefer explicit pattern matching with IO.raiseError fallback.

Minor: Inconsistent dryRunOpt usage

batchDryRunOpts re-declares Opts.flag(dry-run) inline instead of reusing the already-extracted dryRunOpt val defined a few lines later.

Missing Test Coverage

• No integration test for the --dry-run CLI flag dispatch path (existing tests cover parseBatchJson and formatSummary in isolation, not the CLI parser wiring)
• No test for --dry-run with invalid JSON

What Is Good

• formatSummary extraction is the right call: clean refactor, no logic change
• formula alias is additive and non-breaking; value correctly takes precedence when both are present
• knownPutfProps updated to avoid spurious unknown-property warnings -- good catch
• Error message now mentions formula as an alternative, user-friendly
• Test coverage for the alias is thorough (single ref, dragging, and value-wins-over-formula precedence cases)

[claude]

PR Review: feat(cli): Accept "formula" alias for putf batch ops + add --dry-run flag

Good feature additions overall — both the formula alias and --dry-run flag fill real usability gaps. Test coverage is solid. A few issues worth addressing:

---

Bug: Side effect inside .map {} violates purity

Main.scala, batchDryRun method:

private def batchDryRun(source: String): IO[String] =
  BatchParser.readBatchInput(source).flatMap { input =>
    BatchParser.parseBatchOperations(input).map { result =>
      result.warnings.foreach(System.err.println)  // ← impure side effect in pure .map!
      ...
    }
  }

System.err.println inside .map {} is a hidden effect — it executes eagerly during the mapping step, not as a managed IO effect. Per the project's purity charter, all effects belong in flatMap/IO(...). Fix:

BatchParser.parseBatchOperations(input).flatMap { result =>
  val printWarnings = result.warnings.traverse_(w => IO(System.err.println(w)))
  printWarnings.as {
    val summary = BatchParser.formatSummary(result.ops)
    s"Dry run - ${result.ops.size} operations parsed:\n$summary"
  }
}

---

Code smell: @unchecked pattern match

Main.scala, runBatchDryRun:

private def runBatchDryRun(cmd: CliCommand): IO[ExitCode] =
  val CliCommand.Batch(source, _) = cmd: @unchecked
  batchDryRun(source).flatMap(IO.println).as(ExitCode.Success)

This function is only ever called from batchDryRunOpts.map(runBatchDryRun) which always produces a CliCommand.Batch, but the @unchecked is a future maintenance hazard. Since only source is needed, simplify to:

private def runBatchDryRun(cmd: CliCommand.Batch): IO[ExitCode] =
  batchDryRun(cmd.source).flatMap(IO.println).as(ExitCode.Success)

Or inline batchDryRun(source) directly and change the .map lambda accordingly.

---

Design concern: Duplicate subcommand registration

Two separate Opts.subcommand("batch", batchHelp) registrations exist in the same orElse chain:

// batchDryRunOpts (requires --dry-run flag)
val batchDryRunOpts = Opts.subcommand("batch", batchHelp) { (batchArg, dryRunFlag).mapN(...) }

// batchCmd (orFalse --dry-run)
val batchCmd = Opts.subcommand("batch", batchHelp) { (batchArg, dryRunOpt).mapN(...) }

This works with Decline's orElse semantics but is unconventional — the routing logic is implicit (first one requiring --dry-run, second being the fallback). A single subcommand that checks the dryRun boolean in its handler would be cleaner:

// Single registration
val batchCmd = Opts.subcommand("batch", batchHelp) { (batchArg, dryRunOpt).mapN(CliCommand.Batch.apply) }

// In dispatch:
case CliCommand.Batch(source, true) => batchDryRun(source).flatMap(IO.println).as(ExitCode.Success)
case CliCommand.Batch(source, false) => requireOutput(outputOpt, ...) { ... }

This also fixes the silent behavior where xl batch --dry-run --file foo.xlsx --output bar.xlsx currently goes through batchDryRunOpts and ignores --file/--output without any warning.

---

Minor: dryRun field never used in write path

In both executeHeadlessCommand and executeSheetWriteCommand:

case CliCommand.Batch(source, _) =>  // dryRun is silently discarded

Once the design is consolidated to a single routing point (above), these become moot.

---

What's good

• The formula/value alias implementation in requireStringValue is clean and the precedence (value wins) is correct and tested.
• Extracting formatSummary to BatchParser is the right refactoring — good DRY improvement.
• Test coverage covers the alias, dragging with alias, both-fields precedence, and summary format. The "both fields present" test is especially important to have.
• Error message update (Missing 'value' (or 'formula') field) is user-friendly.
• Documentation updates in CLAUDE.md and SKILL.md are clear.

The purity violation is the most important fix needed before merge given the project's stated non-negotiables.