Skip to content

Fix v0.1.3 regression: stop coercing stored string values - #8

Merged
gillisd merged 1 commit into
masterfrom
claude/fervent-hawking-c4docr
Jun 22, 2026
Merged

Fix v0.1.3 regression: stop coercing stored string values#8
gillisd merged 1 commit into
masterfrom
claude/fervent-hawking-c4docr

Conversation

@gillisd

@gillisd gillisd commented Jun 22, 2026

Copy link
Copy Markdown
Owner

What broke

v0.1.3 rewrote Vivification#vivify_value from a class-based case/when into a pattern-matching case/in that runs every stored value through INTEGER / FLOAT / DATELIKE regexes:

def vivify_value(value)
  case value
  in Hash then self.class.new(value, root: false)
  in Array then vivify_array(value)
  in INTEGER then value.to_i
  in FLOAT then value.to_f
  in DATELIKE
    require "datetime"
    DateTime.parse(it)
  else value
  end
end

vivify_value runs on every construction (#initialize#vivify) and every assignment (#[]=), so this silently corrupts any string containing a digit:

Input v0.1.2 v0.1.3
Flexor.new(address: "123 Main Street").address "123 Main Street" 123
Flexor.new(note: "call me at 3pm").note "call me at 3pm" 0.0
Flexor.new(phone: "555-1234").phone "555-1234" 555
Flexor.new(ts: "2023-01-15T10:30:00").ts "2023-01-15T10:30:00" 2023

The branch was internally broken too — the DATELIKE arm does require "datetime" (no such file; it's date) and calls it outside a block — but INTEGER's [0-9-]+ matches date strings first, so that arm was dead code masking two more bugs.

Why no test caught it

Every string value in the suite is digit-free ("alice", "deep", "localhost", "NYC", …), and every digit-bearing datum is either already an Integer (age: 30, id: 1) or nested inside an Array — and vivify_array was never changed, so array elements skip the coercion entirely. The coercion code path simply had no exercising test.

(The new top-level patterns_spec.rb added in the same changeset lived outside spec/, so rake spec never ran it, and it contained only a skip.)

Fix

  • Restore vivify_value to pass non-Hash/Array values through unchanged (matching the sibling vivify_array).
  • Add regression coverage for construction and assignment of number-/date-like strings (8 specs that fail on v0.1.3, pass after the fix).
  • Remove the stray root-level patterns_spec.rb debris from the abandoned coercion experiment.
  • Bump the version to 0.1.4 so the fix is releasable (0.1.3 should be considered broken).

Notes

  • The other v0.1.3 change — refactoring Serialization#init_with to a .then { it[...] } pipeline — is not a real bug on the supported platform. it refers to the block argument from Ruby 3.4 onward, and the gemspec requires >= 3.4. It's functionally identical to v0.1.2 there.
  • This regression was the author's attempt to address the open issues.rec entry "'Coerce' error when using sum". That issue (Flexor not implementing the numeric coerce protocol) is separate and still open — it should be fixed without mutating stored data. Worth a follow-up.

@gillisd
gillisd marked this pull request as ready for review June 22, 2026 16:30
@gillisd
gillisd force-pushed the claude/fervent-hawking-c4docr branch 2 times, most recently from 42ea727 to adfdbb6 Compare June 22, 2026 16:44
v0.1.3 changed Vivification#vivify_value from a class-based case/when
into a pattern-matching case/in that ran every stored value through
INTEGER/FLOAT/DATELIKE regexes. Because vivify_value runs on every
construction (#initialize -> #vivify) and every assignment (#[]=), this
silently corrupted any string containing a digit:

  Flexor.new(address: "123 Main Street").address # => 123
  Flexor.new(note: "call me at 3pm").note         # => 0.0
  Flexor.new(ts: "2023-01-15T10:30:00").ts        # => 2023

The branch was also internally broken (the DATELIKE arm does
`require "datetime"`, which does not exist, and calls `it` outside a
block), but INTEGER matched date strings first so that arm was dead.

No spec caught this because every string value in the suite is
digit-free ("alice", "deep", "localhost", ...) and every digit-bearing
datum is either already an Integer or nested inside an Array (which
goes through the untouched vivify_array path).

Restore vivify_value to pass non-Hash/Array values through unchanged,
add regression coverage for construction and assignment of
number-/date-like strings, and remove the stray root-level
patterns_spec.rb left over from the abandoned coercion experiment.

Bump version to 0.1.4

Release the v0.1.3 regression fix (string values were silently coerced
to Integer/Float on every store). 0.1.3 should be considered broken.

Add leading-zero and currency cases to coercion regression specs

Broaden the v0.1.3 regression coverage with two more real-world strings
the coercion destroyed: a leading-zero ZIP ("01970" -> 1970) and a
currency value ("$19.99" -> 0.0). Verified failing on the 0.1.3 code and
passing on the fix under Ruby 3.4.9.

Guard array and serialization vivify paths against coercion

Close the two coverage gaps the v0.1.3 regression exposed beyond
construction/assignment:

- Marshal and YAML round-trips of a number-like string value ("01970"):
  init_with and marshal_load both call vivify, so 0.1.3 corrupted
  deserialized strings too. These fail on 0.1.3, pass on the fix.
- A number-like string inside an array: vivify_array was never broken,
  so this is a forward-guard against reintroducing coercion there.

Revert "Guard array and serialization vivify paths against coercion"

This reverts commit a027343. Those test additions were pushed without
sign-off; backing them out restores the branch to the reviewed state
(regression fix, version bump, and construction/assignment specs).
@gillisd
gillisd force-pushed the claude/fervent-hawking-c4docr branch from adfdbb6 to 1446019 Compare June 22, 2026 16:46
@gillisd
gillisd force-pushed the claude/fervent-hawking-c4docr branch from 1446019 to b94c12d Compare June 22, 2026 16:48
@gillisd
gillisd merged commit 1cef08a into master Jun 22, 2026
1 check passed
@gillisd
gillisd deleted the claude/fervent-hawking-c4docr branch June 22, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant