Nullable vs. Non-Required Fields in Typespecs

[feynmanliang]

In decoded data from an API, nil currently represents both null values and missing keys (non-required fields). Is there a better way to represent these states that would make them more distinguishable?

This discussion was converted from issue #3. Original issue text below:

---

When I mark a field as non-nullable in my OAI spec

          "stream": {
            "type": "boolean",
            "nullable": false
          },

I expect both the __fields__ and the module typespec to reflect it.

Instead, it seems that the generated module typespec remains nullable while the fields are non-nullable.

  @type t :: %__MODULE__{
          stream: boolean | nil
        }

...

  @doc false
  @spec __fields__(atom) :: keyword
  def __fields__(type \\ :t)

  def __fields__(:t) do
    [
      stream: :boolean
    ]
  end
end

[aj-foster]

Hi @feynmanliang, this occurs because the property is not set as required on the parent schema.

I chose to represent this state (a missing key in the API response) as nil in the typespec, the same as if the key were present but the value were null. There's definitely room for confusion here. It's also possible that I'm misunderstanding JSON Schema:

required

The value of this keyword MUST be an array. Elements of this array, if any, MUST be strings, and MUST be unique. An object instance is valid against this keyword if every item in the array is the name of a property in the instance. Omitting this keyword has the same behavior as an empty array.

I think this means a missing required key should have this behaviour (all properties are optional), but that may be wrong.

[feynmanliang]

That makes sense, thanks for the explanation. Could we do something like optional(:key_name) in the typespec instead for keys that are missing in required (e.g. https://elixirforum.com/t/typespec-for-map-w-both-required-and-optional-keys/21539/6). Currently representing a nullable: false but not required field using a null causes JSONSchema validation to fail:

[aj-foster]

I'm open to using optional(:key) if Dialyzer still accepts nil values in that case, however it doesn't look like this syntax is currently supported for structs (see comment).

---

After typing the above, I tested to see if Dialyzer would accept nil if the key is optional().

defmodule Test do
  @type t :: %{
          required(:a) => String.t() | nil,
          optional(:b) => String.t()
        }

  @spec thing :: t
  def thing do
    %{
      a: "Hello",
      b: nil
    }
  end
end

Unfortunately, Dialyzer catches this with the following error:

Invalid type specification for function 'Elixir.Test':thing/0. The success typing is 
          () -> #{'a' := <<_:40>>, 'b' := 'nil'}

So even if we use optional() for a key, we will still have to give a default type for the value (nil or otherwise).

[feynmanliang]

Maybe I'm misunderstanding, but isn't this expected behavior? b here is optional (i.e. can be omitted) but non-nullable (nil is absent in the union type, so if b is present it must not have value nil) hence providing nil is an error.

I haven't tried it, but I would expect the following structs to be members of type t

%{
  a: "Hello",
}

%{
  a: "Hello",
  b: "Hello"
}

%{
  a: nil
}

%{
  a: nil
  b: "Hello"
}

[feynmanliang]

Got it, thanks. I don't have a good solution in mind, but here's where this issue is coming up.

The non-required non-nullable field: https://github.com/feynmanliang/text-generation-inference/blob/d700ad6e9f1920a433e41b3915b93d8e5ea27b2e/docs/openapi.json#L388-L391

The nullable generated typespec: https://github.com/fmops/text_generation_inference.ex/blob/521142be9b0068143ace13cc9412ec5d43cfc39d/lib/text_generation_inference/compat_generate_request.ex#L10

[aj-foster]

Decided to convert this into a discussion so we can revisit the topic periodically. In general, I'm interested if there are better ways to represent both nullable and non-required fields in structs. It's similar to the difference between null and undefined in JavaScript, or a nil value and the complete absence of a key in an Elixir map. With structs we don't have the flexibility to omit keys in this way, but we can explore other options.

[feynmanliang]

Something I thought of, though it is a bit ugly, is to use a coproduct type eg:

@type t :: %__MODULE__{
          inputs: String.t(),
          options: TextGenerationInference.CompatGenerateOptions.t() | nil,
          parameters: TextGenerationInference.GenerateParameters.t() | nil,
          stream: boolean
        } | %__MODULE__{
          inputs: String.t(),
          options: TextGenerationInference.CompatGenerateOptions.t() | nil,
          parameters: TextGenerationInference.GenerateParameters.t() | nil,
        }

Not very scalable though; for this CompatGenerateRequest use case we'd have 2^3 different terms in the coproduct 😢

[aj-foster]

Indeed, the combinatorial explosion for something like GitHub's user model would be quite large. Somewhat related — mostly mentioning it because I find it quite interesting — the Erlang runtime has a configuration called SET_LIMIT set to 13 by default. If a union type gets to be 13 items or greater, Dialyzer replaces the type with a more general type. For example, a union of 13 atoms gets replaced with the general atom type. I think in the case of 13 structs in a union, the type would become a plain map with no keys specified. Just one of those little ways that Dialyzer tries to save memory and maintain speed while sacrificing the ability to find all of the issues.

[feynmanliang]

Ran into this issue again and thought to save us a repro

Using https://github.com/fmops/oapi_oai/tree/0.2.1, I expect the following to succeed

      Dreamcatcher.OpenAIAPI.create_chat_completion(
        %OpenAI.CreateChatCompletionRequest{
          model: "gpt-3.5-turbo",
          messages: [
            %OpenAI.ChatCompletionRequestMessage{
              role: "system",
              content: "tell me a joke"
            },
          ]
        },
        api_key: System.get_env("OPENAI_API_KEY")
      )

Instead, I am receiving the following error response body from OpenAI

%{"error" => %{"code" => nil, "message" => "None is not of type 'string' - 'messages.0.name'", "param" => nil, "type" => "invalid_request_error"}}

Digging deeper, it seems like the root cause is because OpenAI.ChatCompletionRequestMessage's name empty field is serialized as a null

iex(1)>%OpenAI.ChatCompletionRequestMessage{}
%OpenAI.ChatCompletionRequestMessage{content: nil, name: nil, role: nil}
iex(2)> %OpenAI.ChatCompletionRequestMessage{} |> Jason.encode!()
"{\"content\":null,\"name\":null,\"role\":null}"

The aforementioned OpenAPI spec (https://github.com/openai/openai-openapi/blob/master/openapi.yaml#L2299-L2304) marks the name field as non-required but non-nullable, so the API implementation is indeed compliant in rejecting a null value in a non-nullable field.

I understand that Elixir structs don't have a good way to distinguish between missing vs null fields, but is there any way we can make this case work without resorting to just using maps and foregoing type-safety?

[feynmanliang]

My current solution is to implement a custom encoder

defimpl Jason.Encoder, for: OpenAI.ChatCompletionRequestMessage do
  def encode(value, opts) do
    value
    |> Map.from_struct()
    |> case do
      %{name: nil} = map ->
        map |> Map.delete(:name)

      map ->
        map
    end
    |> Jason.encode!()
  end
end

I guess this is a datapoint against automatically generating these encoders like was suggesting in #11

[feynmanliang]

Ran into this issue again on https://github.com/fmops/text_generation_inference.ex/blob/main/lib/text_generation_inference/generate_parameters.ex#L9 with huggingface's TGI API responding

"Failed to deserialize the JSON body into the target type: parameters.details: invalid type: null, expected a boolean at line 1 column 208"

https://github.com/huggingface/text-generation-inference/blob/main/docs/openapi.json#L467-L470 suggests this field is non-nullable but a null is being passed through due to same logic as above.

[aj-foster]

If I'm understanding correctly, this example is a request body (as opposed to a response body). In #19 I suggested that maybe inputs should be typed as plain maps rather than structs — with no struct defined. This would allow the typespec to use required(:x) and optional(:y) keys, and it would be up to the caller to provide the correct keys.

(I also have a new discussion in #20 that is relevant to this.)

[feynmanliang]

I see. Yes, I think using typed maps would preserve the merits of inspecting an OAPI scheka (e.g. Hammox as well as better elixir-ls LSP autocompletions) while also mitigating the collision of both nullable and non-required fields getting encoded as JSON nulls.

Our current workaround in order to preserve structs at invocations within client code is to strip the nils in the generated OAPI library's client e.g. https://github.com/fmops/text_generation_inference.ex/blob/main/lib/text_generation_inference/client.ex#L48-L51

[flaviogrossi]

What is the best way to handle this in a generic way?

Consider the classic example of PATCH-ing a rest resource: how do you handle the case of nil fields in the body request when the schema is rendered as a struct? If you have a nil field it could be interpreted both as a request to set the field to NULL, or to just avoid updating that field.

Is there a way to force render that payload with a map or typed map instead of a struct?