sangkyoonnam opened a new pull request, #1153: URL: https://github.com/apache/flink-agents/pull/1153
Linked issue: #1152 ### Purpose of change #### User-visible outcome Two `MemoryRef` values are now equal only when both the memory type and the path are equal; `hashCode` and `toString` follow. `MemoryRef.create` rejects a null type or path with an `IllegalArgumentException`. Previously a sensory and a short-term reference to the same path compared equal, hashed together and printed identically, and a null type surfaced later as a `NullPointerException` from `resolve` or from the Jackson serializer. #### Intent A reference names a slot in one memory. `equals` was written when the class had only a path (#110); the sensory-memory change added the type without revisiting it. The Python `MemoryRef` is a frozen pydantic model and already compares and hashes both fields, so this aligns the Java side with it. #### Runtime flow No call path changes. `MemoryObjectImpl#get(MemoryRef)` and `EventAttachmentUtils` read the type and path through getters and are unaffected. The JSON form already carries `memory_type`, so the existing `EventTest` round-trip assertion still holds. #### Key decisions - Keep `MemoryObject#get(MemoryRef)` resolving by path only, in both languages. Its contract says it "represents the path of the given reference"; whether a reference should be rejected when resolved against the other memory is a separate question. - `IllegalArgumentException` for a null type or path, matching the class's own `Deserializer` and the other recent argument checks in `api`. ### Behavioral Semantics #### Interaction decisions Equality is `type == other.type && path.equals(other.path)`; `hashCode` is `Objects.hash(type, path)`; `toString` prints both. `create` checks the type first, then the path. #### Behavioral contracts 1. References with different memory types and the same path are not equal and do not share a hash code. 2. References with the same memory type and path are equal and share a hash code (unchanged). 3. `toString` names the memory type and the path. 4. `create(null, path)` and `create(type, null)` raise `IllegalArgumentException`. #### Failure behavior The only new failure path is `create` with a null argument, which now throws at construction; nothing is created, logged or absorbed. The `NullPointerException` a null type used to cause in `resolve` and in serialization is no longer reachable. ### Tests #### Contracts to tests | Contract | Tests | |---|---| | 1 | `MemoryRefTest.testRefEqualityDistinguishesMemoryType` | | 2 | `MemoryRefTest.testRefEqualityAndHashing` (pre-existing) | | 3 | `MemoryRefTest.testToStringNamesMemoryType` | | 4 | `MemoryRefTest.testCreateRejectsNullTypeOrPath` | #### Coverage and what was not verified `mvn -pl api install && mvn -pl runtime test`: 983, 0 failures. `mvn -pl api,plan test`: 454 and 397, 0 failures. `spotless:check` passes. Without the `MemoryRef` change exactly the three new tests fail. The Python side was checked directly against `flink_agents.api.memory_reference`: a sensory and a short-term reference to the same path are not equal, a set of both keeps two entries, `repr` shows the memory type, and `create(None, path)` raises `ValidationError`. The Python model is unchanged by this PR. Not verified: no production code in `api`, `plan` or `runtime` keeps references in a `Set` or `Map` today, so the collection behavior is covered at the unit level only. <details> <summary>Implementation invariants and supporting evidence</summary> - `type` is an enum, so identity comparison is exact. - The `Deserializer` already rejects a missing `memory_type` or `path`, so `create` now matches the JSON path. - The pre-existing `testRefEqualityAndHashing` uses `SHORT_TERM` only, which is why it did not catch this. </details> ### API #### Compatibility impact `MemoryRef` is public API. Only code that relied on two references of different types comparing equal changes behavior; no such use exists in the repository. `create` now throws on a null type or path instead of deferring the failure. Python is unchanged. ### Documentation <!-- Do not remove this section. Check the proper box only. --> - [ ] `doc-needed` <!-- Your PR changes impact docs --> - [x] `doc-not-needed` <!-- Your PR changes do not impact docs --> - [ ] `doc-included` <!-- Your PR already contains the necessary documentation updates --> ### Was this patch authored or co-authored using generative AI tooling? <!-- Do not remove this section. Check the proper box only. --> - [x] Yes - [ ] No Generated-by: Claude Code 2.1.280 (Claude Fable 5.1) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
