Skip to content

json 3 raises on duplicate keys in model output: adopt strict parsing as policy, guard the journal writer #76

Open

Description

@minerva-sky

What changed

PR #75 moves the dev lockfile from json 2.7.1 to json 3.1. Two panel seats flagged the same behavior change: json 3.x raises JSON::ParserError on duplicate object keys (and on // comments), where 2.x silently took last-wins. The gem has no stated policy for this. It should, because most of our JSON.parse calls read model output, and models do emit duplicate keys when they revise a field mid-generation.

Where it bites

Every LLM-facing parse already rescues JSON::ParserError, so the change degrades rather than crashes:

  • lib/agentic/llm_client.rb:149 → Errors::LlmParseError, routed through handle_error
  • lib/agentic/llm_response.rb:157 → falls back to the raw string
  • lib/agentic/llm_assisted_composition_strategy.rb:107, learning/capability_optimizer.rb:417 and :486, capabilities/examples.rb:456, capabilities/file_generation_capability.rb:233, cli/progress_tracker.rb:294, cli/execution_observer.rb:539 → each has its own rescue

Reading the journal (execution_journal.rb:195) also rescues, and in :strict mode promotes to JournalDamagedError, which is the right call for a durable log.

The one spot with no rescue is the writer: execution_journal.rb:136 does JSON.generate(payload) inside record, which runs from after_task_success. A capability that returns a hash with :k and "k" as separate keys, or a BINARY-encoded non-ASCII string, now raises JSON::GeneratorError after the task already succeeded. json 2.7 wrote an ambiguous line instead. Nothing in lib/ produces such a value today, but capabilities are user-supplied, so the hole is real even if unexercised.

Lean: stay strict, don't pass allow_duplicate_key: true

Three reasons.

  1. Last-wins hides a model error. A response with two "status" keys is the model changing its mind. Dropping one silently gives us a plausible object that may not be what the model meant. An LlmParseError is honest, and the retry and fallback paths already exist for it.
  2. The option isn't portable. allow_duplicate_key: only exists from json 2.13 onward. We ship a gem, not an app: downstream bundles on 2.7 would ignore it or choke on it depending on version, so the flag would make behavior less predictable across the matrix, not more.
  3. The parse sites are already defensive. Nine rescues is nine places that already assumed the model could produce garbage. Strict parsing just adds one more kind of garbage to a category we already handle.

The journal writer is the exception: a logging side effect must never fail a task that succeeded. Proposed fix, one small PR:

def record(event, payload = {})
  line = begin
    JSON.generate({event: event, at: Time.now.utc.iso8601(3)}.merge(payload))
  rescue JSON::GeneratorError => e
    JSON.generate(event: event, at: Time.now.utc.iso8601(3), payload_error: e.class.name, payload_inspect: payload.inspect[0, 500])
  end
  # ...unchanged
end

plus a spec that records {k: 1, "k" => 2} and a BINARY string and asserts the journal line still lands and replays. If the policy above is accepted, I'll also add a one-paragraph "JSON from models is parsed strictly" note to the README's LLM section so the next person who hits a duplicate-key error knows it's deliberate.

Not proposing

  • A json floor of >= 3.0 in the gemspec. Nothing in lib/ needs 3.x features, and raising the floor shuts out every Rails 7.x app still on json 2.
  • A central Agentic::JSON.parse_llm helper. Nine call sites with slightly different fallback semantics is a refactor, not a policy. File it separately if the rescues ever drift.

Activity

  1. minerva-sky commented on Oct 17, 2026

    @minerva-sky
    CollaboratorAuthor

    Writer guard is up as #77 (rescue in ExecutionJournal#record, fallback line keeps task_id/description, two specs). The README note on strict LLM parsing waits on this policy being accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    loop:depsDependency / language upgrade loop

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions