Support variables within inline object list filter

[VectorTetra]

No description provided.

[xperiandri]

Copilot review:

Here’s a review of the changes in this PR, focusing on possible issues, risks, and things to double-check:

---

1. Major Theme: Support for Input Custom Types (esp. ObjectListFilter)

• Addition of InputCustomDefinition<'Val>, InputCustomDef, and plumbing through the type system and execution logic to support custom input object types.
• Refactoring of ObjectListFilterType to use the new input custom type infrastructure.
• Introduction of new static methods on InputValue for conversion from objects and JSON.

Issues/Concerns:

• Backward compatibility: Changing the type of ObjectListFilterType from ScalarDefinition to InputCustomDefinition and updating its registration. If any code (including user schemas) depended on the old shape, it could break. Check: Are all usages updated? Are there docs that need updating?
• Type Safety: The new InputCustomDefinition<'Val> system relies on boxing/unboxing and uses type checks at runtime (e.g., in OfObject). These can be brittle if types don't match exactly. Check: Are the error messages clear? Are there edge cases where type mismatches could silently fail?
• Error Handling: There are new error cases (e.g., variable not found), but most error handling seems to return only a message. Check: Are these errors surfaced to the user in the expected way? Are paths and details included for debugging?
• Performance: The new system does more reflection and boxing/unboxing. For large or frequent queries, this could have a small perf impact.

---

2. Changes in Middleware and Schema Definitions

• coerceObjectListFilterInput now takes variables as a parameter and can resolve variables recursively.
• Matching on InputCustom in introspection and validation.

Issues/Concerns:

• Recursion/Stack Safety: The new recursive variable handling (especially in coerceObjectListFilterInput) could, in pathological cases, cause stack overflows if deeply nested or cyclic variable references occur. Is there a cycle check?
• Error Propagation: Are all errors from Result appropriately propagated, or could some be lost if not handled properly in the new mapping logic?
• Test Coverage: The new test seems to cover the "variable in input object" case. Are there tests for error cases (missing variable, wrong type, etc.)?

---

3. Execution Changes

• coerceVariableValue now uses a context record instead of a parameter list.
• Special handling for InputCustom.

Issues/Concerns:

• Null Handling: There are several places where nullability is checked, and errors are generated. These look correct, but are all old edge cases (null variables, missing properties) still handled as before?
• ReflectionHelper: Still used for matching types. If the custom input type's output is not assignable to the expected .NET type, it will error. Check: Are all assignability checks robust?
• Compatibility: Are there any plugins, extensions, or downstream consumers that depend on the old execution signature or error details?

---

4. AST and InputValue Extensions

• Adds static members for object and JSON conversion.
• Handles arrays, dictionaries, and primitive types.

Issues/Concerns:

• Type Handling: The new OfObject and OfJsonElement methods are more generic, but the fallback for unknown types is a failwith. Is this what you want in production? Should it be a proper error?
• Generic Type Detection: The code tests for IReadOnlyDictionary<string, obj> via GetGenericTypeDefinition—could this throw for non-generic types? Should you guard with IsGenericType first?

---

5. Tests

• Adds a test for parsing an object list filter via variable.

Issues/Concerns:

• Coverage: Only positive ("happy path") is covered with the new test. Consider tests for:
  • Variable not found
  • Variable is wrong type
  • Null/empty variable
  • Deeply nested structures
• Edge Cases: Are there tests where a variable is used in multiple places, or where variable resolution is ambiguous?

---

6. General / Miscellaneous

• Removal of unused imports (System.Collections.Generic, System.Text.Json where not needed).
• Some formatting/indentation changes.

---

Summary Table

Area	Potential Issue/Risk	Action/Double-Check
Type System	Breaking changes	Review usages, docs, and versioning
Error Handling	Clarity/completeness	Add more tests for error paths
Reflection	Type safety, performance	Fuzz with many types, review error messages
Recursion	Stack safety	Check for cycles/deep nesting
AST Extensions	Fallback is a failwith	Consider result-based errors instead
Testing	Coverage of error/edge cases	Add more negative/edge tests

---

Overall:
The approach is a solid improvement for supporting custom input types and variable resolution. The main risks are around type safety, error propagation, and breaking changes for downstream consumers. The implementation is quite intricate, so extra tests for negative and edge cases would be wise.

Let me know if you’d like a targeted review of a specific file or function, or if you want help drafting additional tests!