fix(cli): Handle qualified ranges in eval sheet inference (GH-210)

[arcaputo3]

Summary

• containsUnqualifiedCellReferences now checks ArgValue.Range location (Local vs CrossSheet) instead of treating all ranges as unqualified
• Fixes eval requiring -s for fully-qualified formulas like =SUM(Revenue!A1:A3)
• Replaces wb.sheets.head with headOption pattern match to silence WartRemover warning

Follow-up to #216 which introduced the method but had a bug in the ArgValue.Range case.

Test plan

• xl -f test.xlsx eval "=SUM(Revenue!A1:A3)" works without -s flag
• 174/174 evaluator tests pass
• 317/317 CLI tests pass
• Format check clean

🤖 Generated with Claude Code

[claude]

Code Review

Summary

This PR correctly fixes a bug in containsUnqualifiedCellReferences introduced in #216. The fix properly distinguishes between local and cross-sheet ranges, allowing fully-qualified formulas like =SUM(Revenue!A1:A3) to work without the -s flag.

✅ Strengths

1. Correct Logic Implementation (xl-evaluator/src/com/tjclp/xl/formula/graph/DependencyGraph.scala:210-213)

• The fix properly pattern matches on ArgValue.Range(loc) to check RangeLocation.Local vs RangeLocation.CrossSheet
• Aligns with existing logic for TExpr.Aggregate cases (lines 196, 202)
• Maintains consistency: Local ranges need ambient sheet context, CrossSheet ranges are self-contained

2. Safe WartRemover Compliance (xl-cli/src/com/tjclp/xl/cli/commands/ReadCommands.scala:452-453)

• Replaces wb.sheets.head with headOption pattern match to avoid WartRemover warning
• The @unchecked annotation is safe here because wb.sheets.isEmpty is checked on line 444
• Comment clearly documents why the match is exhaustive

3. Test Coverage

• 174/174 evaluator tests pass
• 317/317 CLI tests pass
• Format checks clean

4. Follows Project Standards

• Adheres to purity charter (total functions, no partial operations)
• Proper use of pattern matching over conditionals
• Clear, explanatory comments

---

💡 Suggestions

1. Consider Adding Unit Tests

While integration tests pass, consider adding explicit unit tests for containsUnqualifiedCellReferences:

// xl-evaluator/test/src/com/tjclp/xl/formula/graph/DependencyGraphSuite.scala
test("containsUnqualifiedCellReferences - CrossSheet ranges are qualified") {
  val expr = FormulaParser.parse("=SUM(Revenue!A1:A3)").toOption.get
  assert(!DependencyGraph.containsUnqualifiedCellReferences(expr))
}

test("containsUnqualifiedCellReferences - Local ranges are unqualified") {
  val expr = FormulaParser.parse("=SUM(A1:A3)").toOption.get
  assert(DependencyGraph.containsUnqualifiedCellReferences(expr))
}

This would catch regressions more directly than CLI integration tests.

2. Pattern Match Simplification (Optional)

The match at DependencyGraph.scala:210-213 could use inline pattern matching:

case ArgValue.Range(TExpr.RangeLocation.Local(_)) => true
case ArgValue.Range(TExpr.RangeLocation.CrossSheet(_, _)) => false

Current version is fine (perhaps more readable), but this is slightly more concise.

3. Documentation Enhancement

Consider updating the docstring for containsUnqualifiedCellReferences to mention the ArgValue.Range behavior:

/**
 * Check if an expression contains any unqualified cell references that require an ambient sheet
 * context.
 *
 * Unqualified references include:
 * - Cell refs: A1, $B$2
 * - Range refs: A1:B10
 * - Local ranges in function args: SUM(A1:A10)
 *
 * Qualified references that do NOT require ambient sheet:
 * - Sheet-prefixed refs: Revenue!A1
 * - Cross-sheet ranges: Revenue!A1:A10
 *
 * ...

---

🔍 Security & Performance

• Security: No concerns. Pure functions, no external I/O
• Performance: No impact. Simple pattern match adds negligible overhead
• Memory: No allocations introduced

---

📋 Checklist

• Fixes the stated bug (enhancement(cli): CLI polish — eval sheet inference, style log label, rasterizers exit code #210 follow-up)
• Maintains type safety and purity
• Follows project style guide
• All tests pass (491/491 total)
• Format checks clean
• No breaking changes

---

Verdict: ✅ LGTM - Approve with Minor Suggestions

The fix is correct, safe, and follows XL's rigorous standards. The suggestions above are optional enhancements for future robustness, not blockers for this PR.

Great work catching and fixing this quickly after #216! 🎉