PR #9 review follow-ups: nil-safe InTxn, doc rename catch-up, Entity[R] tests - #10
Merged
Conversation
The 0.9.0 rename sweep covered README and code comments but missed the two auxiliary docs: DEFAULTER.md still named LoadOrStore/LoadAndDelete, and VALIDATOR.md still listed the deleted InsertRaw. GetOrInsert now appears among the validated writes, since validateStruct runs there too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The package-level InTxn is the advertised public constructor for the typed layer, and wrapping a nil Txn deferred a nil-pointer panic to the first method call. A nil tx now yields a ClientTxn carrying a sentinel Txn with initErr set — the same deferred-error contract NewTxn uses for pool failures — so every method reports the error and Query returns nil, matching the failed-transaction contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
entity.go shipped in 0.9.0 with no direct tests; AsRecord's Record()
probing was exercised only through hand-rolled wrapper fakes. Pin the
Entity contract against a generated-style entity: record adoption
without copying, the documented pointer-receiver MarshalJSON pitfall
(a value-typed entity emits {}), zero-value marshaling to null, lazy
UnmarshalJSON allocation, in-place unmarshal of an adopted record,
Validate delegating the backing record to the StructValidator, and the
AsRecord bridge for a single entity and an element-wise slice.
Also add the 0.9.1 changelog entry for this follow-up set.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Embedded (file://) clients honor WithNamespace — the client opens the given namespace ID with full isolation from the default namespace (verified by an insert/query round-trip against both) — yet NewClient warned "namespace is set, but it is not supported in this version" unconditionally. Gate the warning on the dgraph:// prefix, where v25 namespace support genuinely is not wired up yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The README is this repo's primary documentation, so every exported symbol should be at least discoverable from it. A survey against go doc found the gaps this commit fills: - Configuration Options: add WithCacheSizeMB, WithMaxRecvMsgSize, WithGRPCDialOption, and WithNamespace (4 of 11 options were missing) - add a Defaulter subsection and link DEFAULTER.md; mention NewValidator and link VALIDATOR.md from the WithValidator section - Upserting Data: show typed *UniqueError handling via errors.As - add a Raw DQL Queries (QueryRaw) section, noting it rides ClientCore so it works on ClientTxn and the typed client - Limitations: document the one-embedded-engine-per-process rule (ErrSingletonOnly) and how Close/Shutdown release the engine - dgdao-gen: describe the Entity[R]/AsEntity/Record/AsRecord bridge - Typed Client: add a capability index table covering the full Query[T] builder surface, plus the SetTracer seam and the typed/filter and typed/search companion packages Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
staticcheck flags json.Marshal of a struct with no exported fields and no reachable custom marshaler — which is precisely the pointer-receiver pitfall TestEntity_MarshalJSONOnValueEmitsEmptyObject exists to pin. Silence the analyzer at that call site only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mlwelles
approved these changes
Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups from a post-merge review of #9, one commit per finding, plus README coverage work that came out of surveying the public API against it.
InTxn(nil)now errors instead of panicking. The package-levelInTxnis the advertised constructor for the typed layer, and wrapping a nilTxndeferred a nil-pointer panic to the first method call. A niltxnow yields aClientTxncarrying a sentinelTxnwithinitErrset (the same deferred-error contractNewTxnuses for pool failures), so every method reports the error andQueryreturns nil, matching the failed-transaction contract.Docs caught up to the 0.9.0 renames.
DEFAULTER.mdstill namedLoadOrStore/LoadAndDelete, andVALIDATOR.mdstill listed the deletedInsertRaw. Both now use the new names, andGetOrInsertappears among the validated writes sincevalidateStructruns there too.Direct tests for the
Entity[R]base.entity.goshipped in 0.9.0 with no direct coverage;AsRecord'sRecord()probing was exercised only through hand-rolled wrapper fakes. New tests pin the contract against a generated-style entity: record adoption without copying, the documented pointer-receiverMarshalJSONpitfall (a value-typed entity emits{}), zero-value marshaling tonull, lazyUnmarshalJSONallocation, in-place unmarshal of an adopted record,Validatedelegating the backing record to theStructValidator, and theAsRecordbridge for a single entity and an element-wise slice.WithNamespacewarning scoped to remote clients. Embedded (file://) clients honorWithNamespace— verified with an insert/query round-trip showing full isolation between a created namespace and the default one — yetNewClientwarned "not supported in this version" unconditionally. The warning now fires only fordgraph://URIs, where v25 namespace support genuinely isn't wired up.README public-API coverage. The README serves as the repo's primary documentation, so a survey checked every exported symbol in
dgdaoanddgdao/typedagainst it. Filled: the four missing client options (WithCacheSizeMB,WithMaxRecvMsgSize,WithGRPCDialOption,WithNamespace), aDefaultersubsection with the DEFAULTER.md link,NewValidatorand the VALIDATOR.md link, typed*UniqueErrorhandling, aQueryRawsection, the embedded-engine singleton limitation (ErrSingletonOnly,Shutdown), theEntity[R]/AsRecordgenerated-entity bridge under dgdao-gen, and a typed capability index table covering the fullQuery[T]builder surface plus theSetTracerseam and thetyped/filter/typed/searchcompanions.Changelog gets a 0.9.1 entry covering the set.
Testing
go test -short -race . ./typed/passes locally, matching the unit-test CI job.🤖 Generated with Claude Code