Apply GPT-5 review fixes for issue #1196

Address review feedback from specs/issue-1196-collecting-deserialization-errors-claude-review-gpt-5.md:

API Surface & Delegation:
- Fix ObjectReader.readValueCollecting() to use public API `this.with(perCallAttrs).readValue(p)` instead of protected `_new(...)` factory
- Maintains consistency with Jackson's builder pattern and public surface

Limit Resolution:
- Read max-problem cap from per-call reader config instead of base _config
- Properly honors per-call attribute overrides
- Affects both normal completion and hard failure paths

Javadoc Enhancements:
- Add comprehensive class-level Javadoc to DeferredBindingException with usage examples
- Enhance CollectedProblem Javadoc explaining all fields, truncation, and immutability
- Expand CollectingProblemHandler Javadoc detailing design, recoverable errors, and DoS protection
- Improve ObjectReader.readValueCollecting() Javadoc noting behavior without collectErrors() and parser filtering differences

Testing:
- All 27 CollectingErrorsTest tests pass
- Full suite: 4,662 tests pass, 0 failures, 0 errors
- No regressions introduced

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: sri-adarsh-kumar

src/main/java/tools/jackson/databind/ObjectReader.java

@@ -1386,16 +1386,30 @@ public <T> T readValue(TokenBuffer src) throws JacksonException
      * errors if encountered. If any problems were collected, throws
      * {@link tools.jackson.databind.exc.DeferredBindingException} with all problems.
      *
-     * <p>On hard failures (non-recoverable errors), the original exception
-     * is thrown with collected problems attached as suppressed exceptions.
+     * <p><b>Usage</b>: This method should be called on an ObjectReader created via
+     * {@link #collectErrors()} or {@link #collectErrors(int)}. If called on a regular
+     * reader (without error collection enabled), it behaves the same as
+     * {@link #readValue(JsonParser)} since no handler is registered.
+     *
+     * <p><b>Error handling</b>:
+     * <ul>
+     * <li>Recoverable errors are accumulated and thrown as
+     *     {@link tools.jackson.databind.exc.DeferredBindingException} after parsing</li>
+     * <li>Hard (non-recoverable) failures throw immediately, with collected problems
+     *     attached as suppressed exceptions</li>
+     * <li>When the configured limit is reached, collection stops</li>
+     * </ul>
      *
      * <p><b>Thread-safety</b>: Each call allocates a fresh problem bucket,
      * so multiple concurrent calls on the same reader instance are safe.
      *
-     * <p>This method should only be called on an ObjectReader created via
-     * {@link #collectErrors()}. If called on a regular reader, it behaves
-     * the same as {@link #readValue(JsonParser)}.
+     * <p><b>Parser filtering</b>: Unlike convenience overloads ({@link #readValueCollecting(String)},
+     * {@link #readValueCollecting(byte[])}, etc.), this method does <i>not</i> apply
+     * parser filtering. Callers are responsible for filter wrapping if needed.
      *
+     * @param <T> Type to deserialize
+     * @param p JsonParser to read from (will not be closed by this method)
+     * @return Deserialized object
      * @throws tools.jackson.databind.exc.DeferredBindingException if recoverable problems were collected
      * @throws tools.jackson.databind.DatabindException if a non-recoverable error occurred
      * @since 3.1
@@ -1410,21 +1424,17 @@ public <T> T readValueCollecting(JsonParser p) throws JacksonException {
         ContextAttributes perCallAttrs = _config.getAttributes()
             .withPerCallAttribute(tools.jackson.databind.deser.CollectingProblemHandler.class, bucket);
 
-        // Create a temporary ObjectReader with per-call attributes
-        // This matches the existing API surface (no new internal methods needed)
-        ObjectReader perCallReader = _new(this,
-            _config.with(perCallAttrs),
-            _valueType, _rootDeserializer, _valueToUpdate,
-            _schema, _injectableValues);
+        // Create a temporary ObjectReader with per-call attributes using public API
+        ObjectReader perCallReader = this.with(perCallAttrs);
 
         try {
             // Delegate to the temporary reader's existing readValue method
             T result = perCallReader.readValue(p);
 
             // Check if any problems were collected
             if (!bucket.isEmpty()) {
-                // Check if limit was reached
-                Integer maxProblems = (Integer) _config.getAttributes()
+                // Check if limit was reached - read from per-call config to honor overrides
+                Integer maxProblems = (Integer) perCallReader.getConfig().getAttributes()
                     .getAttribute(tools.jackson.databind.deser.CollectingProblemHandler.ATTR_MAX_PROBLEMS);
                 boolean limitReached = (maxProblems != null &&
                     bucket.size() >= maxProblems);
@@ -1440,7 +1450,8 @@ public <T> T readValueCollecting(JsonParser p) throws JacksonException {
         } catch (DatabindException e) {
             // Hard failure occurred; attach collected problems as suppressed
             if (!bucket.isEmpty()) {
-                Integer maxProblems = (Integer) _config.getAttributes()
+                // Read from per-call config to honor overrides
+                Integer maxProblems = (Integer) perCallReader.getConfig().getAttributes()
                     .getAttribute(tools.jackson.databind.deser.CollectingProblemHandler.ATTR_MAX_PROBLEMS);
                 boolean limitReached = (maxProblems != null &&
                     bucket.size() >= maxProblems);

src/main/java/tools/jackson/databind/deser/CollectingProblemHandler.java

@@ -19,9 +19,38 @@
  * problems into a per-call bucket stored in {@link DeserializationContext}
  * attributes.
  *
- * <p>Designed for use with {@link tools.jackson.databind.ObjectReader#collectErrors()}.
+ * <p><b>Design</b>: This handler is completely stateless. The problem collection
+ * bucket is allocated per-call by
+ * {@link tools.jackson.databind.ObjectReader#readValueCollecting ObjectReader.readValueCollecting(...)}
+ * and stored in per-call {@link tools.jackson.databind.cfg.ContextAttributes ContextAttributes},
+ * ensuring thread-safety and call isolation.
+ *
+ * <p><b>Usage</b>: This class is internal infrastructure, registered automatically by
+ * {@link tools.jackson.databind.ObjectReader#collectErrors() ObjectReader.collectErrors()}.
+ * Users should not instantiate or register this handler manually.
+ *
+ * <p><b>Recoverable errors handled</b>:
+ * <ul>
+ * <li>Unknown properties ({@link #handleUnknownProperty handleUnknownProperty}) - skips children</li>
+ * <li>Type coercion failures ({@link #handleWeirdStringValue handleWeirdStringValue},
+ *     {@link #handleWeirdNumberValue handleWeirdNumberValue}) - returns defaults</li>
+ * <li>Map key coercion ({@link #handleWeirdKey handleWeirdKey}) - returns {@code NOT_HANDLED}</li>
+ * <li>Instantiation failures ({@link #handleInstantiationProblem handleInstantiationProblem}) -
+ *     returns null when safe</li>
+ * </ul>
+ *
+ * <p><b>Default values</b>: Primitives receive zero/false defaults; reference types
+ * (including boxed primitives) receive {@code null} to avoid masking nullability issues.
+ *
+ * <p><b>DoS protection</b>: Collection stops when the configured limit (default 100)
+ * is reached, preventing memory/CPU exhaustion attacks.
+ *
+ * <p><b>JSON Pointer</b>: Paths are built from parser context following RFC 6901,
+ * with proper escaping of {@code ~} and {@code /} characters.
  *
  * @since 3.1
+ * @see tools.jackson.databind.ObjectReader#collectErrors()
+ * @see tools.jackson.databind.exc.DeferredBindingException
  */
 public class CollectingProblemHandler extends DeserializationProblemHandler {
 

src/main/java/tools/jackson/databind/exc/CollectedProblem.java

@@ -11,7 +11,28 @@
  * Immutable value object capturing details about a single deserialization
  * problem encountered during error-collecting mode.
  *
+ * <p><b>Contents</b>: Each problem records:
+ * <ul>
+ * <li>{@link #getPath() path} - RFC 6901 JSON Pointer to the problematic field
+ *     (e.g., {@code "/items/2/price"})</li>
+ * <li>{@link #getMessage() message} - Human-readable error description</li>
+ * <li>{@link #getTargetType() targetType} - Expected Java type (may be null)</li>
+ * <li>{@link #getLocation() location} - Source location in JSON (line/column)</li>
+ * <li>{@link #getRawValue() rawValue} - Original value from JSON that caused the error
+ *     (truncated if > 200 chars)</li>
+ * <li>{@link #getToken() token} - JSON token type at error location</li>
+ * </ul>
+ *
+ * <p><b>Truncation</b>: String values longer than {@value #MAX_RAW_VALUE_LENGTH}
+ * characters are truncated with "..." suffix to prevent memory issues.
+ *
+ * <p><b>Unknown properties</b>: For unknown property errors, {@code rawValue}
+ * is {@code null} since the property name is already in the path.
+ *
+ * <p><b>Immutability</b>: All instances are immutable and thread-safe.
+ *
  * @since 3.1
+ * @see DeferredBindingException#getProblems()
  */
 public final class CollectedProblem {
     /**

src/main/java/tools/jackson/databind/exc/DeferredBindingException.java

@@ -8,11 +8,39 @@
 
 /**
  * Exception that aggregates multiple recoverable deserialization problems
- * encountered during error-collecting mode (enabled via
- * {@link tools.jackson.databind.ObjectReader#collectErrors()}).
+ * encountered during error-collecting mode.
  *
- * <p>Each problem is captured as a {@link CollectedProblem} containing
- * the error location, message, and context.
+ * <p><b>Usage</b>: This exception is thrown by
+ * {@link tools.jackson.databind.ObjectReader#readValueCollecting ObjectReader.readValueCollecting(...)}
+ * when one or more recoverable errors were collected during deserialization.
+ * Enable error collection via {@link tools.jackson.databind.ObjectReader#collectErrors()}.
+ *
+ * <p><b>Problem access</b>: Each problem is captured as a {@link CollectedProblem}
+ * containing the JSON Pointer path, error message, location, target type, raw value, and token.
+ * Access problems via {@link #getProblems()}.
+ *
+ * <p><b>Limit handling</b>: When the configured problem limit is reached, collection
+ * stops and {@link #isLimitReached()} returns {@code true}. This indicates additional
+ * errors may exist beyond those collected.
+ *
+ * <p><b>Message formatting</b>: The exception message shows:
+ * <ul>
+ * <li>For 1 problem: the single error message</li>
+ * <li>For multiple: count + first 5 problems + "...and N more" suffix</li>
+ * <li>A "limit reached" note if applicable</li>
+ * </ul>
+ *
+ * <p><b>Example</b>:
+ * <pre>{@code
+ * try {
+ *     MyBean bean = reader.collectErrors()
+ *                         .readValueCollecting(json);
+ * } catch (DeferredBindingException e) {
+ *     for (CollectedProblem p : e.getProblems()) {
+ *         System.err.println("Error at " + p.getPath() + ": " + p.getMessage());
+ *     }
+ * }
+ * }</pre>
  *
  * @since 3.1
  */