Skip to content

SK-3061: revert fields→tokens rename, restore data field on bulkInsert response - #408

Merged
Devesh-Skyflow merged 15 commits into
flowvault-release/26.8.13from
devesh/sk-3061
Aug 13, 2026
Merged

SK-3061: revert fields→tokens rename, restore data field on bulkInsert response#408
Devesh-Skyflow merged 15 commits into
flowvault-release/26.8.13from
devesh/sk-3061

Conversation

@Devesh-Skyflow

@Devesh-Skyflow Devesh-Skyflow commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the deviations from the FlowVault API contract flagged in this Slack thread, plus a documentation/samples cleanup pass that grew out of it.

⚠️ Breaking change (item 6), already regenerated: InsertResponseRecord/BulkInsertResponseRecord's deprecated constructors and getFields() had their tokens generics change. The japicmp baseline (flowvault/api-report/skyflow-flowvault-java.baseline.jar) has been regenerated via scripts/contract-snapshot-update.sh flowvault and is included in this PR — mvn -pl common,flowvault -am verify passes clean against it (confirmed locally once a JDK/Maven became available partway through review; see item 6 and the Testing section for the full picture, including which parts of this turned out to be avoidable and were declined on purpose).

1. Response contract fix

  1. fieldstokens: the SDK had renamed the API's tokens response key to fields, so callers needed getFields() instead of the API-matching getTokens(). Reverted the rename; getFields() stays as a @Deprecated alias that logs a warning and delegates to getTokens() — existing callers keep working unchanged, detokenize/deleteTokens untouched throughout.
  2. data field restored: the API's data field was silently dropped from the bulk insert response. It's wired back through from V1RecordResponseObject.getData(), which already carried it — nothing in Utils.formatBulkInsertResponse was reading it.

Compatibility note: flowvault/pom.xml runs a japicmp binary/source-compatibility check against flowvault/api-report/*.baseline.jar for com.skyflow.vault.data. Rather than changing the existing InsertResponseRecord/BulkInsertResponseRecord constructor signatures (a breaking change under that gate), the old (data-less) constructor overloads are kept as @Deprecated pass-throughs, and data is added via new overloads. No baseline regeneration should be required.

2. flowvault/README.md accuracy pass

Went through the README against the actual source rather than just fixing the two items above:

  • Version snippets said 1.0.0; pom.xml is 1.0.1.
  • CustomHeaderKey sample used names (SkyflowAccountId, etc.) that don't exist — the real enum is SKYFLOW_ACCOUNT_ID/SKYFLOW_ACCOUNT_NAME/REQUEST_ID_HEADER. The README's own code block didn't compile.
  • "vault() takes no arguments, use one client per vault" was false — vault(String vaultId) exists and is tested for multi-vault use on one client. Documented it.
  • "updateType accepts UPDATE (the default)" overstated the SDK's behavior — it omits the field when unset rather than sending "UPDATE" itself; reworded.
  • getHttpStatus() example used "BAD_REQUEST"; the actual hardcoded validation-error string is "Bad Request".
  • Bulk Insert tokens JSON example showed a flat string per column; the real shape is always a list of {token, tokenGroupName} entries per column (one per token group), per a dedicated regression test (testBulkInsert_successWithListOfMapsTokenShape). Fixed the example and added a snippet showing how to read it (there's no typed accessor yet — same gap raised in the Slack thread).
  • Added a "Schema vs. schemaless vaults" table (bulkInsert = structured, bulkTokenize/bulkDeleteTokens = schemaless, bulkDetokenize = both) — flagged in the thread as a known doc gap, confirmed against git history (SK-2646, which shipped tokenize/delete-tokens specifically as "Schemaless vault apis") since there's no code-level enforcement of it. Repeated as a one-line note on each of the four operation sections so it's visible without reading top-to-bottom.
  • Bulk Detokenize metadata JSON example initially backed out a claim about its typical content (only skyflowId was grounded in code; tableName was just a Javadoc description with nothing behind it), then re-grounded it against flowdb_dp_apis.proto's own literal example value for the field, {"table": "table1", "skyflowID": "4524524534623"} — stronger evidence than the description text, which is itself misleading (it says "tableName", the actual wire key is table; Utils.java only renames skyflowIDskyflowId, table passes through unrenamed). Updated the JSON example to match.

3. Samples cleanup (flowvault/samples/)

  • Deleted BearerTokenExpiryExample.java — despite its name, it never touches BearerToken/Token.isExpired(); it's a generic "retry once on 401" wrapper, redundant with BearerTokenGenerationExample's real expiry pattern and the README's own retry guidance.
  • Rewrote samples/README.md — it referenced DetokenizeExample.java, InsertExample.java, GetByIdExample.java, etc., none of which exist in this module (stale boilerplate from a different layout). Replaced with an accurate index of the real samples and correct Maven run instructions.
  • Added response iteration (summary + per-record/per-token walk + retry) to BulkInsertSync/Async, BulkMultiTableInsertSync/Async, BulkDetokenizeSync/Async, and CustomHeaderExample — these previously only did System.out.println(response). BulkTokenizeSync/Async and BulkDeleteTokensSync/Async already had this pattern.

4. Testing

  • Updated ResponseComponentTests, BulkResponseTests, UtilsTests, VaultControllerTests for the rename/restore, including a dedicated test for the deprecated constructor + getFields() alias.
  • Codecov flagged 2 uncovered lines in InsertResponseRecord.java (its own deprecated constructor — never exercised directly, since BulkInsertResponseRecord's deprecated constructor bypasses it). Added a direct test for it.
  • Actually verified, not just written: a JDK/Maven became available partway through this PR's review (none were available in the sandbox this branch was authored in until then). mvn -pl common,flowvault -am test692 tests, 0 failures. mvn -pl common,flowvault -am verify (full reactor, japicmp included) → BUILD SUCCESS against the regenerated baseline from item 6.
    • One unrelated pre-existing failure surfaced along the way and had to be excluded to get a clean run: common's TokenTests.testExpiredTokenForIsExpiredToken reads a TEST_EXPIRED_TOKEN value from a .env file that isn't committed to the repo (a CI-only secret) — nothing to do with this PR, com.skyflow.serviceaccount.util.Token is a pre-existing, unrelated class (JWT expiry check) that this PR never touches.

5. Bug fix: Skyflow.getVaultConfig() crashed on an empty vault list

Found while double-checking the vault()/vault(String) claim above — a different, unrelated method with a real bug:

public VaultConfig getVaultConfig() {
    Object[] array = this.builder.vaultConfigMap.values().toArray();
    return (VaultConfig) array[0];   // unguarded — throws ArrayIndexOutOfBoundsException if empty
}

build() never validates that at least one vault was registered, so a client built with zero addVaultConfig(...) calls is a valid, reachable state — and calling getVaultConfig() on one crashed with an unchecked ArrayIndexOutOfBoundsException rather than failing predictably. Traced every call site in the codebase first: all of them are on VaultController (a different, already-safe method with the same name — it just returns its own single stored config, no lookup involved), never on Skyflow directly. So this had zero usages and zero test coverage anywhere in the suite.

Fixed by mirroring the sibling method's already-correct convention — BaseSkyflow.getVaultConfig(String) is a plain map.get(vaultId) that returns null when absent, no exception, no signature change:

public VaultConfig getVaultConfig() {
    return this.builder.vaultConfigMap.values().stream().findFirst().orElse(null);
}

Deliberately did not make it throw SkyflowException like vault()/vault(String) do — that would diverge from its own sibling method's contract and require adding a checked exception to the signature, a source-incompatible change under this module's japicmp gate, for no real benefit. This fix is implementation-only (same signature), so no baseline update needed. Added 7 tests in SkyflowTests covering the regression case plus the existing untested gaps on the by-id overload.

6. InsertResponseRecord.getTokens() is now typed (Map<String, List<Token>>)

The generic tokens: Map<String, Object> (item 2 above) still required casting/iterating by hand to reach token/tokenGroupName — the exact usability complaint in the Slack thread. First pass at fixing this added a separate typed accessor (getTokenDetails()) alongside the untouched generic getTokens(), keeping both per the thread's "generic for flexibility, typed for UX" clarification. Per explicit direction, went further: getTokens() itself is now the typed oneMap<String, List<Token>> — matching the shape this field had before insert was reworked (flowvault's own pre-rework "v3" Success.tokens was Map<String, List<Token>>; getTokenDetails() no longer exists).

This is the one genuinely breaking change in this PR — confirmed against the actual japicmp diff once a JDK/Maven became available (not just reasoned about in the abstract):

  • getTokens() itself was never breaking. It's reported as a brand-new method against the committed baseline — the pre-SK-3061 codebase only ever had getFields(), so any shape getTokens() returns is additive. My initial writeup here overstated this, assuming a same-arity generic-erasure conflict that doesn't actually apply to a method with zero parameters (erasure conflicts are a parameter-list problem, not a return-type one — a method can't be overloaded by return type at all, regardless of arity, so this was never really in question).
  • What is flagged, all under com.skyflow.vault.data: InsertResponseRecord's deprecated 6-arg constructor and BulkInsertResponseRecord's deprecated 8-arg constructor (both had their tokens parameter generics change), and getFields() (return type generics changed, since it just delegates to getTokens()).
  • These three were avoidable with modest extra code — the deprecated constructors are a different arity than the primary ones, so nothing forces their parameter type to change; getFields() could reverse-map back to the original shape instead of delegating directly. Flagged this and asked; declined in favor of just regenerating the baseline (see the warning banner above and the regenerated flowvault/api-report/skyflow-flowvault-java.baseline.jar in this PR).
  • This also reverses the "keep contract exactly like API" decision made earlier in this same thread — the API's own wire contract types a column's tokens as generic Object. Accepted as the tradeoff for matching the requested typed shape.

Token (getToken()/getTokenGroupName()) isn't a new invention — it's a straight reintroduction of a class flowvault itself had before insert was reworked (deleted in 685a82ff), with final fields and a toString() added to match TokenizeResponseToken's established convention in this package.

The actual parsing (a column's raw wire value can be a list of {token, tokenGroupName} entries, a single such entry, or a bare value) now lives in a new static utility, Token.parseTokens(Map<String, Object>), called once by Utils.formatBulkInsertResponse when constructing each record — not recomputed on every getTokens() call like the first pass's getTokenDetails() was.

Found and fixed a real landmine while adding the Token class: ResponseComponentTests.java had a dangling {@link Token} Javadoc reference left over from the class's original removal, which would have silently started resolving to this new class (while the surrounding comment still said Token "was removed") had I not updated it.

Updated every test that constructed a record with a raw Map<String,Object> tokens value or asserted on the old generic return type, BulkInsertSync/Async, BulkMultiTableInsertSync/Async, and CustomHeaderExample to use getTokens() directly (matching BulkTokenizeSync.java's existing typed per-token loop), and closed every branch in the new parsing logic with tests (a per-column null value, a null element inside a token-group list, a Map entry missing one of its two expected keys). VaultControllerTests.testBulkInsert_successWithListOfMapsTokenShape simplified nicely as a result — no more manual casting needed to assert on it.

Also updated outside this repo

The FlowVault 1.0.0 migration Confluence doc (linked from the Slack thread) has been corrected to match the response-contract change and flags the Object-typing gap (now closed by item 6, but the doc itself is unchanged since this PR).

Not in this PR (flagged as follow-ups)

  • A CI safeguard test to catch future wire-type fields silently going unread (the same failure mode data had here).

🤖 Generated with Claude Code

…rt response

The flowvault SDK renamed the API's `tokens` response key to `fields`,
so callers had to use getFields() instead of the API-matching
getTokens(). Separately, the API's `data` field was silently dropped
from the response entirely. Both were flagged in Slack by a customer
comparing SDK output to the raw API contract.

- InsertResponseRecord/BulkInsertResponseRecord: add tokens/getTokens()
  matching the API. getFields() stays as a @deprecated alias that logs
  a warning and delegates to getTokens() - existing callers keep
  working unchanged.
- Add data/getData(), wired from V1RecordResponseObject.getData() in
  Utils.formatBulkInsertResponse (the wire type already carried it;
  nothing read it).
- Old constructor overloads (without `data`) are kept, also
  @deprecated, rather than changing existing constructor signatures -
  this avoids a binary/source-incompatible change against the
  japicmp baseline in flowvault/pom.xml.
- detokenize/deleteTokens are untouched; they never had this issue.
- Updated README and tests accordingly, including a dedicated test for
  the deprecated constructor + getFields() alias.

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

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.36%. Comparing base (1145b80) to head (f326ec2).

Files with missing lines Patch % Lines
...lt/src/main/java/com/skyflow/vault/data/Token.java 94.87% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@                       Coverage Diff                       @@
##             flowvault-release/26.8.13     #408      +/-   ##
===============================================================
+ Coverage                        91.30%   91.36%   +0.06%     
- Complexity                           0      475     +475     
===============================================================
  Files                              157      158       +1     
  Lines                             6392     6440      +48     
  Branches                           850      859       +9     
===============================================================
+ Hits                              5836     5884      +48     
+ Misses                             364      362       -2     
- Partials                           192      194       +2     
Flag Coverage Δ
common 88.39% <100.00%> (+<0.01%) ⬆️
flowvault 88.88% <96.55%> (+0.23%) ⬆️
skyvault 94.72% <ø> (ø)
unittests-flowvault 89.85% <96.22%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Module: common 88.39% <100.00%> (+<0.01%) ⬆️
Module: skyvault 94.72% <ø> (ø)
Module: flowvault 88.88% <96.55%> (+0.23%) ⬆️
Service Account 86.69% <ø> (ø)
Vault Data 91.66% <96.07%> (+0.23%) ⬆️
Vault Tokens 99.03% <ø> (ø)
Vault Connection 100.00% <ø> (ø)
Vault Controller 85.31% <ø> (ø)
Detect 100.00% <ø> (ø)
Audit 100.00% <ø> (ø)
BIN Lookup 100.00% <ø> (ø)
Config 96.26% <ø> (ø)
Utils 89.22% <100.00%> (+<0.01%) ⬆️
Errors 100.00% <ø> (ø)
Enums 100.00% <ø> (ø)
Logs 95.34% <100.00%> (+0.01%) ⬆️
Files with missing lines Coverage Δ
...ommon/src/main/java/com/skyflow/logs/InfoLogs.java 100.00% <100.00%> (ø)
flowvault/src/main/java/com/skyflow/Skyflow.java 93.68% <100.00%> (+2.01%) ⬆️
...owvault/src/main/java/com/skyflow/utils/Utils.java 87.58% <100.00%> (+0.02%) ⬆️
...m/skyflow/vault/data/BulkInsertResponseRecord.java 100.00% <100.00%> (ø)
...a/com/skyflow/vault/data/InsertResponseRecord.java 100.00% <100.00%> (ø)
...lt/src/main/java/com/skyflow/vault/data/Token.java 94.87% <94.87%> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1145b80...f326ec2. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Devesh-Skyflow and others added 14 commits August 13, 2026 17:20
…dd response iteration to samples

README (flowvault/README.md):
- Version snippets said 1.0.0; pom.xml is 1.0.1.
- CustomHeaderKey enum names were wrong (PascalCase vs actual SCREAMING_SNAKE_CASE) -
  the sample code block did not compile.
- "vault() takes no arguments, use one client per vault" was false: Skyflow.vault(String
  vaultId) exists and is tested for multi-vault use on one client. Documented it.
- "updateType accepts UPDATE (the default)" overstated what the SDK does - it omits the
  field when unset rather than sending "UPDATE"; reworded to say so.
- getHttpStatus() example used "BAD_REQUEST"; the actual hardcoded validation-error
  string is "Bad Request".

Samples:
- Deleted BearerTokenExpiryExample.java: despite its name, it never touches BearerToken
  or Token.isExpired() at all - it's a generic "retry once on 401" wrapper around
  bulkDetokenize, redundant with both BearerTokenGenerationExample's real expiry-check
  pattern and the README's own retry guidance.
- Rewrote samples/README.md: it referenced DetokenizeExample.java, InsertExample.java,
  GetByIdExample.java etc. - files that don't exist anywhere in this module (leftover
  boilerplate from a different samples layout). Replaced with an accurate index of the
  actual serviceaccount/ and vault/ samples plus correct Maven run instructions.
- Added response iteration (summary + per-record/per-token walk + retry) to
  BulkInsertSync/Async, BulkMultiTableInsertSync/Async, BulkDetokenizeSync/Async, and
  CustomHeaderExample - they previously only printed the raw response object.
  BulkTokenizeSync/Async and BulkDeleteTokensSync/Async already had this and are
  untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Bulk Insert: tokens example showed a flat string per column
  ("card_number": "5484-..."), but the value is always a LIST of
  {token, tokenGroupName} entries - one per token group configured on
  that column, even when there's only one. This is the exact shape a
  dedicated regression test (testBulkInsert_successWithListOfMapsTokenShape)
  guards, and it's what the API's own generic Object typing is for.
  Added a populated hashedData example and a code snippet showing how
  to read a tokens entry, since there's no typed accessor for it yet.
- Bulk Detokenize: metadata example showed {} on a successful record,
  but metadata normally carries skyflowId/tableName on success (per
  the wire type's own Javadoc and the key-rename Utils.java performs
  on it), and the real "nothing there" case is null, not {}.

Field names/order for all four response record types (verified against
InsertResponseRecord/BulkInsertResponseRecord, TokenizeResponseRecord/
TokenizeResponseToken, DetokenizeResponseRecord/BaseDetokenizeRecordResponse,
DeleteTokensRecord) and the requestId null-on-success/populated-on-error-only
behavior were already correct - no changes needed there.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was flagged in the Slack thread (Saketh's clarification point 1:
"We have the SDK interface to schema/schemaless vault mapping already
but we haven't added it in the readme, we will add it") and was still
missing - grepping the README for "schema" turned up nothing.

Added a table documenting which of the four bulk operations apply to
which vault type, matching Devesh's original clarification in the
thread: insert is structured/schema-only, tokenize and deleteTokens
are schemaless-only (confirmed by git history - SK-2646 shipped them
specifically as "Schemaless vault apis"), and detokenize works with
both since it only needs the token itself, not a table.

Verified this isn't enforced anywhere in Validations.java, so worded
it as supported/intended usage rather than something the SDK validates
or blocks.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov flagged 2 uncovered lines in InsertResponseRecord.java. The
existing deprecated-constructor test only goes through
BulkInsertResponseRecord's deprecated 8-arg constructor, which
delegates straight to the new 9-arg constructor -> new 7-arg super
constructor, never touching InsertResponseRecord's own deprecated
6-arg constructor. Nothing else in the codebase constructs
InsertResponseRecord directly (it's only ever used via the Bulk
subclass), so that constructor was genuinely untested. Added a test
that instantiates it directly and asserts data defaults to null.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous commit asserted metadata "typically carries skyflowId/
tableName" with equal confidence for both keys. Re-checking: skyflowId
is grounded in real code (Utils.java renames a skyflowID key to
skyflowId when present, which only exists because that key is known
to show up), but tableName is only mentioned in one line of Javadoc
on the generated wire type - no transform logic touches it and no
test in the suite constructs or asserts a tableName key anywhere in
metadata. That's a docstring example, not a verified contract.

Reverting the example and the claim rather than asserting a shape I
can't actually back up.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n section

The "Schema vs. schemaless vaults" table lives under "VaultController -
Bulk operations", but a reader jumping straight to e.g. "# Bulk
Tokenize" via the TOC or a search never sees it. Added a one-line
"Vault type supported" note at the top of each of the four operation
sections (Insert/Tokenize/Detokenize/Delete Tokens), linking back to
the consolidated table for the full picture.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Insert/Tokenize/Detokenize repeated the explanation already in the
consolidated table; shortened to one line each, consistent with the
delete-tokens note.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI spellcheck flagged flowvault/samples/README.md:38 - "codehaus" from
the org.codehaus.mojo:exec-maven-plugin groupId in the sample run
instructions. Legitimate Maven groupId, not a typo; added alongside
the other domain-specific terms already in the word list (jfrog,
sonatype, etc.).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Skyflow.getVaultConfig() did:
    Object[] array = this.builder.vaultConfigMap.values().toArray();
    return (VaultConfig) array[0];
- an unguarded array[0] access. A client built without any
addVaultConfig(...) call (build() never validates this) throws
ArrayIndexOutOfBoundsException instead of failing predictably.

Traced every call site of "getVaultConfig()" in the codebase first:
all of them are on VaultController (which has its own, unrelated,
already-safe getVaultConfig() returning its single stored config -
no lookup involved), never on Skyflow directly. So this method had
zero usages and zero test coverage anywhere in the suite.

Fixed by mirroring the sibling method's established, already-correct
convention: BaseSkyflow.getVaultConfig(String) is a plain
vaultConfigMap.get(vaultId), returning null when absent - no
exception, no signature change. Rewrote the no-arg overload the same
way (.stream().findFirst().orElse(null)) rather than making it throw
SkyflowException like vault()/vault(String) do, since that would
diverge from its own sibling's contract and require adding a checked
exception to the signature - a source-incompatible change under this
module's japicmp gate for no real benefit. This fix needs no baseline
update: same signature, implementation-only.

Added 7 tests covering: single vault, first-of-several (consistent
with vault()'s "first" semantics, and identity-matched against
getVaultConfig(id)), the empty-client regression case itself, null
after removing the only vault, falling back to the remaining vault
after the first is removed, and the two existing gaps on the
by-id overload (unknown id, empty client).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Previously backed out a claim about metadata's typical content since
only skyflowId was grounded (a rename in Utils.java) and tableName
was just a Javadoc description with no code or test behind it.

flowdb_dp_apis.proto's metadata field has its own literal example
value, not just a free-text description: {"table": "table1",
"skyflowID": "4524524534623"}. That's stronger evidence than the
description text, and it reveals the description itself is misleading
- it says "such as tableName or skyflowID" but the actual example key
is "table", not "tableName". Utils.java only renames "skyflowID" to
"skyflowId"; "table" passes through unrenamed.

Documented the real shape and explained the rename explicitly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds getTokenDetails() to InsertResponseRecord (inherited by
BulkInsertResponseRecord) alongside the existing generic getTokens():
Map<String, Object>. It parses the same data into
Map<String, List<Token>>, so callers get getToken()/getTokenGroupName()
instead of casting Map entries by hand - the exact "known gap" flagged
in the Slack thread and the earlier README note.

Token is a straight reintroduction of flowvault's own pre-rework class
(deleted in 685a82f when insert was reworked around the current
InsertResponseRecord/tokens map), not a new invention - same shape the
user specified, with final fields and a toString() added to match
TokenizeResponseToken's established convention in this package.

getTokenDetails() is purely additive (new method, no signature
changes to anything existing) and computed fresh from getTokens() on
every call rather than stored separately, so the generic and typed
views can never disagree. It normalizes every shape getTokens()'s
value is known to take - a list of {token, tokenGroupName} entries, a
single such entry not wrapped in a list, or a bare token value with no
group info - into a consistent List<Token>, returning null only when
getTokens() itself is null.

Fixed a real landmine along the way: ResponseComponentTests.java had a
dangling {@link Token} javadoc reference left over from the class's
removal - harmless while there was no Token class to resolve to, but
it would have silently started resolving to this new class instead
(with the surrounding comment still saying it "was removed"). Updated
the comment to describe what actually happened.

Added 9 tests covering Token itself and every shape getTokenDetails()
normalizes, including one against BulkInsertResponseRecord directly
(not just the base class) to confirm the inherited method works for
the type callers actually receive. Updated the README's Bulk Insert
section to document getTokenDetails() in place of the manual
cast-it-yourself snippet from the previous commit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two changes landed together since both were already staged:

1. BulkInsertSync/Async, BulkMultiTableInsertSync/Async, and
   CustomHeaderExample previously printed record.getTokens() (the raw
   Map<String, Object>) directly. Updated all five to walk
   getTokenDetails() instead - a nested loop over
   Map<String, List<Token>>, printing token.getTokenGroupName()/
   token.getToken() per column, matching the pattern
   BulkTokenizeSync.java already uses for its own typed per-token loop.

2. 3 more tests, closing every branch the previous commit's 9 tests
   left untouched in InsertResponseRecord's new parsing logic:
   - parseTokenEntries's own null check (a column present in the map
     with a null value, as opposed to the whole tokens map being null,
     which was already covered) - that column is now omitted from
     getTokenDetails() rather than appearing with a null/empty entry.
   - toToken's final `return null` and the corresponding "skip adding"
     branch in parseTokenEntries's list loop - a null element sitting
     inside a column's token-group list, which the loop must skip
     rather than NPE on.
   - The tokenGroupName-absent ternary branch in toToken's Map-entry
     parsing - every existing Map-entry test populated both "token"
     and "tokenGroupName" keys, so the "key missing" path was never
     exercised.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…en>>), matching v3

Per explicit direction: retype tokens/getTokens() itself to
Map<String, List<Token>> - matching the pre-rework ("v3") shape -
instead of keeping it generic and exposing a separate getTokenDetails()
accessor alongside it (the previous commit's approach).

BREAKING CHANGE - flagged and confirmed before implementing:
- getTokens()'s return type changes (Map<String,Object> ->
  Map<String,List<Token>>).
- Both InsertResponseRecord constructors' `tokens` parameter type
  changes for the same reason. This isn't a choice - Map<String,Object>
  and Map<String,List<Token>> erase to the same raw `Map` type, so
  Java forbids two constructor overloads at the same arity that differ
  only in that generic parameter. There is no way to add this as a
  new overload alongside the old one; the parameter type has to change
  in place. Same for BulkInsertResponseRecord's two constructors.
- getFields() (deprecated alias) now returns the typed map too, since
  it just delegates to getTokens().

This diverges from "keep contract exactly like API" (the API's own
wire contract types a column's tokens as generic Object) - accepted
as the tradeoff for matching v3's typed shape.

Moved the parsing logic (a column's raw value can be a list of
{token, tokenGroupName} entries, a single such entry, or a bare
value) from InsertResponseRecord into a new static utility,
Token.parseTokens(Map<String, Object>): Map<String, List<Token>>.
Utils.formatBulkInsertResponse now calls it to convert the wire
type's raw tokens map before constructing BulkInsertResponseRecord -
parsing happens once at construction time instead of on every
getTokens() call.

Updated every test that constructed an InsertResponseRecord/
BulkInsertResponseRecord with a raw Map<String,Object> tokens value,
or asserted on the old generic return type. The former
getTokenDetails()-specific tests now test Token.parseTokens()
directly. VaultControllerTests.testBulkInsert_successWithListOfMapsTokenShape
simplifies nicely: it no longer needs to cast the parsed result, since
getTokens() itself is the typed view now.

Updated README and the 5 insert samples (BulkInsertSync/Async,
BulkMultiTableInsertSync/Async, CustomHeaderExample) to use
getTokens() directly instead of the now-removed getTokenDetails().

Needs a japicmp baseline regeneration (scripts/contract-snapshot-
update.sh flowvault) before merge - this is a genuine, intentional
break of the existing contract, unlike every other change in this
branch so far. I could not run mvn/java in this sandbox to regenerate
or verify it myself.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Regenerated via scripts/contract-snapshot-update.sh flowvault, now
that a JDK/Maven are available to run it. Verified mvn -pl
common,flowvault -am verify passes clean (692 tests, japicmp included)
against this new baseline.

The 3 intentional incompatibilities this baseline now accepts, per
the full japicmp diff (flowvault/target/japicmp/default-cli.diff):
- InsertResponseRecord's deprecated 6-arg constructor: tokens param
  generics changed (Map<String,Object> -> Map<String,List<Token>>).
- BulkInsertResponseRecord's deprecated 8-arg constructor: same.
- getFields(): return type generics changed to match.

getTokens() itself was never actually flagged - it's reported as a
brand new method against this baseline (the pre-SK-3061 codebase only
ever had getFields()), so making it typed was non-breaking on its
own. The three real breaks above are avoidable (the deprecated
constructors are a different arity than the primary ones, so no
erasure conflict forces their parameter type to change; getFields()
could reverse-map back to the old shape instead of delegating
directly) - flagged and declined in favor of just regenerating the
baseline, per explicit direction.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Contract baseline change detected (flowvault)

This PR updates flowvault/api-report/skyflow-flowvault-java.baseline.jar (the approved public API contract). Here is exactly what it changes, comparing the baseline on flowvault-release/26.8.13 against the baseline committed in this PR:

Compatibility Report

semver MINOR

Summary

Warning

Compatible changes found while checking backward compatibility of version skyflow-flowvault-java.baseline with the previous version old-baseline.

Expand to see options used.
  • Report only summary: No
  • Report only changes: Yes
  • Report only binary-incompatible changes: No
  • Access modifier filter: PROTECTED
  • Old archives:
    • old-baseline unknown
  • New archives:
    • skyflow-flowvault-java.baseline unknown
  • Evaluate annotations: Yes
  • Include synthetic classes and class members: No
  • Include specific elements: Yes
    • com.skyflow.Skyflow
    • com.skyflow.config
    • com.skyflow.enums
    • com.skyflow.errors
    • com.skyflow.serviceaccount.util
    • com.skyflow.vault.audit
    • com.skyflow.vault.bin
    • com.skyflow.vault.connection
    • com.skyflow.vault.controller
    • com.skyflow.vault.data
    • com.skyflow.vault.detect
    • com.skyflow.vault.tokens
  • Exclude specific elements: No
  • Ignore all missing classes: Yes
  • Ignore specific missing classes: No
  • Treat changes as errors:
    • Any changes: No
    • Binary incompatible changes: No
    • Source incompatible changes: No
    • Incompatible changes caused by excluded classes: Yes
    • Semantically incompatible changes: No
    • Semantically incompatible changes, including development versions: No
  • Classpath mode: TWO_SEPARATE_CLASSPATHS
  • Old classpath:
/home/runner/.m2/repository/com/skyflow/common/1.0.0/common-1.0.0.jar:/home/runner/.m2/repository/com/fasterxml/jackson/core/jackson-databind/2.17.2/jackson-databind-2.17.2.jar:/home/runner/.m2/repository/com/fasterxml/jackson/core/jackson-annotations/2.17.2/jackson-annotations-2.17.2.jar:/home/runner/.m2/repository/com/fasterxml/jackson/core/jackson-core/2.17.2/jackson-core-2.17.2.jar:/home/runner/.m2/repository/com/fasterxml/jackson/datatype/jackson-datatype-jdk8/2.18.6/jackson-datatype-jdk8-2.18.6.jar:/home/runner/.m2/repository/com/fasterxml/jackson/datatype/jackson-datatype-jsr310/2.18.6/jackson-datatype-jsr310-2.18.6.jar:/home/runner/.m2/repository/io/github/cdimascio/dotenv-java/2.2.0/dotenv-java-2.2.0.jar:/home/runner/.m2/repository/com/google/code/gson/gson/2.10.1/gson-2.10.1.jar:/home/runner/.m2/repository/com/squareup/okhttp3/okhttp/4.12.0/okhttp-4.12.0.jar:/home/runner/.m2/repository/com/squareup/okio/okio/3.6.0/okio-3.6.0.jar:/home/runner/.m2/repository/com/squareup/okio/okio-jvm/3.6.0/okio-jvm-3.6.0.jar:/home/runner/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib-common/1.9.10/kotlin-stdlib-common-1.9.10.jar:/home/runner/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib-jdk8/1.8.21/kotlin-stdlib-jdk8-1.8.21.jar:/home/runner/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib/1.8.21/kotlin-stdlib-1.8.21.jar:/home/runner/.m2/repository/org/jetbrains/annotations/13.0/annotations-13.0.jar:/home/runner/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib-jdk7/1.8.21/kotlin-stdlib-jdk7-1.8.21.jar:/home/runner/.m2/repository/io/jsonwebtoken/jjwt/0.12.6/jjwt-0.12.6.jar:/home/runner/.m2/repository/io/jsonwebtoken/jjwt-api/0.12.6/jjwt-api-0.12.6.jar:/home/runner/.m2/repository/io/jsonwebtoken/jjwt-impl/0.12.6/jjwt-impl-0.12.6.jar:/home/runner/.m2/repository/io/jsonwebtoken/jjwt-jackson/0.12.6/jjwt-jackson-0.12.6.jar:/home/runner/.m2/repository/junit/junit/4.13.2/junit-4.13.2.jar:/home/runner/.m2/repository/org/hamcrest/hamcrest-core/1.3/hamcrest-core-1.3.jar:/home/runner/.m2/repository/org/powermock/powermock-module-junit4/2.0.9/powermock-module-junit4-2.0.9.jar:/home/runner/.m2/repository/org/powermock/powermock-module-junit4-common/2.0.9/powermock-module-junit4-common-2.0.9.jar:/home/runner/.m2/repository/org/powermock/powermock-reflect/2.0.9/powermock-reflect-2.0.9.jar:/home/runner/.m2/repository/net/bytebuddy/byte-buddy/1.10.14/byte-buddy-1.10.14.jar:/home/runner/.m2/repository/net/bytebuddy/byte-buddy-agent/1.10.14/byte-buddy-agent-1.10.14.jar:/home/runner/.m2/repository/org/powermock/powermock-core/2.0.9/powermock-core-2.0.9.jar:/home/runner/.m2/repository/org/javassist/javassist/3.27.0-GA/javassist-3.27.0-GA.jar:/home/runner/.m2/repository/org/powermock/powermock-api-mockito2/2.0.9/powermock-api-mockito2-2.0.9.jar:/home/runner/.m2/repository/org/powermock/powermock-api-support/2.0.9/powermock-api-support-2.0.9.jar:/home/runner/.m2/repository/org/mockito/mockito-core/3.3.3/mockito-core-3.3.3.jar:/home/runner/.m2/repository/org/objenesis/objenesis/2.6/objenesis-2.6.jar
  • New classpath:
/home/runner/.m2/repository/com/skyflow/common/1.0.0/common-1.0.0.jar:/home/runner/.m2/repository/com/fasterxml/jackson/core/jackson-databind/2.17.2/jackson-databind-2.17.2.jar:/home/runner/.m2/repository/com/fasterxml/jackson/core/jackson-annotations/2.17.2/jackson-annotations-2.17.2.jar:/home/runner/.m2/repository/com/fasterxml/jackson/core/jackson-core/2.17.2/jackson-core-2.17.2.jar:/home/runner/.m2/repository/com/fasterxml/jackson/datatype/jackson-datatype-jdk8/2.18.6/jackson-datatype-jdk8-2.18.6.jar:/home/runner/.m2/repository/com/fasterxml/jackson/datatype/jackson-datatype-jsr310/2.18.6/jackson-datatype-jsr310-2.18.6.jar:/home/runner/.m2/repository/io/github/cdimascio/dotenv-java/2.2.0/dotenv-java-2.2.0.jar:/home/runner/.m2/repository/com/google/code/gson/gson/2.10.1/gson-2.10.1.jar:/home/runner/.m2/repository/com/squareup/okhttp3/okhttp/4.12.0/okhttp-4.12.0.jar:/home/runner/.m2/repository/com/squareup/okio/okio/3.6.0/okio-3.6.0.jar:/home/runner/.m2/repository/com/squareup/okio/okio-jvm/3.6.0/okio-jvm-3.6.0.jar:/home/runner/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib-common/1.9.10/kotlin-stdlib-common-1.9.10.jar:/home/runner/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib-jdk8/1.8.21/kotlin-stdlib-jdk8-1.8.21.jar:/home/runner/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib/1.8.21/kotlin-stdlib-1.8.21.jar:/home/runner/.m2/repository/org/jetbrains/annotations/13.0/annotations-13.0.jar:/home/runner/.m2/repository/org/jetbrains/kotlin/kotlin-stdlib-jdk7/1.8.21/kotlin-stdlib-jdk7-1.8.21.jar:/home/runner/.m2/repository/io/jsonwebtoken/jjwt/0.12.6/jjwt-0.12.6.jar:/home/runner/.m2/repository/io/jsonwebtoken/jjwt-api/0.12.6/jjwt-api-0.12.6.jar:/home/runner/.m2/repository/io/jsonwebtoken/jjwt-impl/0.12.6/jjwt-impl-0.12.6.jar:/home/runner/.m2/repository/io/jsonwebtoken/jjwt-jackson/0.12.6/jjwt-jackson-0.12.6.jar:/home/runner/.m2/repository/junit/junit/4.13.2/junit-4.13.2.jar:/home/runner/.m2/repository/org/hamcrest/hamcrest-core/1.3/hamcrest-core-1.3.jar:/home/runner/.m2/repository/org/powermock/powermock-module-junit4/2.0.9/powermock-module-junit4-2.0.9.jar:/home/runner/.m2/repository/org/powermock/powermock-module-junit4-common/2.0.9/powermock-module-junit4-common-2.0.9.jar:/home/runner/.m2/repository/org/powermock/powermock-reflect/2.0.9/powermock-reflect-2.0.9.jar:/home/runner/.m2/repository/net/bytebuddy/byte-buddy/1.10.14/byte-buddy-1.10.14.jar:/home/runner/.m2/repository/net/bytebuddy/byte-buddy-agent/1.10.14/byte-buddy-agent-1.10.14.jar:/home/runner/.m2/repository/org/powermock/powermock-core/2.0.9/powermock-core-2.0.9.jar:/home/runner/.m2/repository/org/javassist/javassist/3.27.0-GA/javassist-3.27.0-GA.jar:/home/runner/.m2/repository/org/powermock/powermock-api-mockito2/2.0.9/powermock-api-mockito2-2.0.9.jar:/home/runner/.m2/repository/org/powermock/powermock-api-support/2.0.9/powermock-api-support-2.0.9.jar:/home/runner/.m2/repository/org/mockito/mockito-core/3.3.3/mockito-core-3.3.3.jar:/home/runner/.m2/repository/org/objenesis/objenesis/2.6/objenesis-2.6.jar

Results

Status Type Serialization Compatibility Changes
Modified com.skyflow.vault.data.BulkInsertResponseRecord Not serializable Annotation deprecated added Method parameter generics changed
Modified com.skyflow.vault.data.InsertResponseRecord Not serializable Annotation deprecated added Method return type generics changed Method parameter generics changed Method added to public class
Added com.skyflow.vault.data.Token Not serializable Method added to public class
Expand for details.

com.skyflow.vault.data.BulkInsertResponseRecord

  • Binary-compatible
  • Source-compatible
  • Serialization-compatible
Status Modifiers Type Name Extends JDK Serialization Compatibility Changes
Modified public Class BulkInsertResponseRecord InsertResponseRecord JDK 8 Not serializable No changes

Constructors

Status Modifiers Generics Constructor Annotations Throws Compatibility Changes
Source-incompatible public BulkInsertResponseRecord(int, String, String, Map<String, Object>Map<String, List<Token>>, Map<String, Object>, int, String, String) Deprecated: forRemoval=true, since="1.0.2" Annotation deprecated added Method parameter generics changed
Added public BulkInsertResponseRecord(int, String, String, Map<String, List<Token>>, Map<String, Object>, Map<String, Object>, int, String, String) No changes

com.skyflow.vault.data.InsertResponseRecord

  • Binary-compatible
  • Source-compatible
  • Serialization-compatible
Status Modifiers Type Name Extends JDK Serialization Compatibility Changes
Modified public Class InsertResponseRecord Object JDK 8 Not serializable No changes

Constructors

Status Modifiers Generics Constructor Annotations Throws Compatibility Changes
Source-incompatible public InsertResponseRecord(String, String, Map<String, Object>Map<String, List<Token>>, Map<String, Object>, int, String) Deprecated: forRemoval=true, since="1.0.2" Annotation deprecated added Method parameter generics changed
Added public InsertResponseRecord(String, String, Map<String, List<Token>>, Map<String, Object>, Map<String, Object>, int, String) No changes

Methods

Status Modifiers Generics Type Method Annotations Throws Compatibility Changes
Added public Map<String, Object> getData() Method added to public class
Source-incompatible public Map<String, Object>Map<String, List<Token>> getFields() Deprecated: forRemoval=true, since="1.0.2" Annotation deprecated added Method return type generics changed
Added public Map<String, List<Token>> getTokens() Method added to public class

com.skyflow.vault.data.Token

  • Binary-compatible
  • Source-compatible
  • Serialization-compatible
Status Modifiers Type Name Extends JDK Serialization Compatibility Changes
Added public Class Token Object JDK 8 Not serializable No changes

Constructors

Status Modifiers Generics Constructor Annotations Throws Compatibility Changes
Added public Token(String, String) No changes

Methods

Status Modifiers Generics Type Method Annotations Throws Compatibility Changes
Added public String getToken() Method added to public class
Added public String getTokenGroupName() Method added to public class
Added static public Map<String, List<Token>> parseTokens(Map<String, Object>) Method added to public class
Added public String toString() Method added to public class

Warning

All missing classes, i.e. superclasses and interfaces that could not be found on the classpath were ignored.

Hence changes caused by these superclasses and interfaces are not reflected in the output.


Generated on: 2026-08-13 15:21:49.670+0000.

@Devesh-Skyflow
Devesh-Skyflow merged commit b17c441 into flowvault-release/26.8.13 Aug 13, 2026
27 of 28 checks passed
Devesh-Skyflow added a commit that referenced this pull request Aug 18, 2026
…he response (#411)

The snippet added when getTokens() was made typed (#408) used `record`
without ever introducing it - it reads as if it stood alone, but record
only exists inside a loop over insertResponse.getRecords(). Wrapped it in
that outer loop (plus a null check, since getTokens() is null on a failed
record), matching the pattern the Bulk Detokenize section's equivalent
metadata snippet already follows.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Devesh-Skyflow added a commit that referenced this pull request Aug 24, 2026
* SK-3061: revert fields→tokens rename, restore data field on bulkInsert response (#408)

* SK-3061: revert fields->tokens rename, restore data field on bulkInsert response

The flowvault SDK renamed the API's `tokens` response key to `fields`,
so callers had to use getFields() instead of the API-matching
getTokens(). Separately, the API's `data` field was silently dropped
from the response entirely. Both were flagged in Slack by a customer
comparing SDK output to the raw API contract.

- InsertResponseRecord/BulkInsertResponseRecord: add tokens/getTokens()
  matching the API. getFields() stays as a @deprecated alias that logs
  a warning and delegates to getTokens() - existing callers keep
  working unchanged.
- Add data/getData(), wired from V1RecordResponseObject.getData() in
  Utils.formatBulkInsertResponse (the wire type already carried it;
  nothing read it).
- Old constructor overloads (without `data`) are kept, also
  @deprecated, rather than changing existing constructor signatures -
  this avoids a binary/source-incompatible change against the
  japicmp baseline in flowvault/pom.xml.
- detokenize/deleteTokens are untouched; they never had this issue.
- Updated README and tests accordingly, including a dedicated test for
  the deprecated constructor + getFields() alias.

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

* SK-3061: fix flowvault README inaccuracies, drop misleading sample, add response iteration to samples

README (flowvault/README.md):
- Version snippets said 1.0.0; pom.xml is 1.0.1.
- CustomHeaderKey enum names were wrong (PascalCase vs actual SCREAMING_SNAKE_CASE) -
  the sample code block did not compile.
- "vault() takes no arguments, use one client per vault" was false: Skyflow.vault(String
  vaultId) exists and is tested for multi-vault use on one client. Documented it.
- "updateType accepts UPDATE (the default)" overstated what the SDK does - it omits the
  field when unset rather than sending "UPDATE"; reworded to say so.
- getHttpStatus() example used "BAD_REQUEST"; the actual hardcoded validation-error
  string is "Bad Request".

Samples:
- Deleted BearerTokenExpiryExample.java: despite its name, it never touches BearerToken
  or Token.isExpired() at all - it's a generic "retry once on 401" wrapper around
  bulkDetokenize, redundant with both BearerTokenGenerationExample's real expiry-check
  pattern and the README's own retry guidance.
- Rewrote samples/README.md: it referenced DetokenizeExample.java, InsertExample.java,
  GetByIdExample.java etc. - files that don't exist anywhere in this module (leftover
  boilerplate from a different samples layout). Replaced with an accurate index of the
  actual serviceaccount/ and vault/ samples plus correct Maven run instructions.
- Added response iteration (summary + per-record/per-token walk + retry) to
  BulkInsertSync/Async, BulkMultiTableInsertSync/Async, BulkDetokenizeSync/Async, and
  CustomHeaderExample - they previously only printed the raw response object.
  BulkTokenizeSync/Async and BulkDeleteTokensSync/Async already had this and are
  untouched.

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

* SK-3061: fix inaccurate JSON response examples in flowvault README

- Bulk Insert: tokens example showed a flat string per column
  ("card_number": "5484-..."), but the value is always a LIST of
  {token, tokenGroupName} entries - one per token group configured on
  that column, even when there's only one. This is the exact shape a
  dedicated regression test (testBulkInsert_successWithListOfMapsTokenShape)
  guards, and it's what the API's own generic Object typing is for.
  Added a populated hashedData example and a code snippet showing how
  to read a tokens entry, since there's no typed accessor for it yet.
- Bulk Detokenize: metadata example showed {} on a successful record,
  but metadata normally carries skyflowId/tableName on success (per
  the wire type's own Javadoc and the key-rename Utils.java performs
  on it), and the real "nothing there" case is null, not {}.

Field names/order for all four response record types (verified against
InsertResponseRecord/BulkInsertResponseRecord, TokenizeResponseRecord/
TokenizeResponseToken, DetokenizeResponseRecord/BaseDetokenizeRecordResponse,
DeleteTokensRecord) and the requestId null-on-success/populated-on-error-only
behavior were already correct - no changes needed there.

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

* SK-3061: document schema vs. schemaless vault support per operation

This was flagged in the Slack thread (Saketh's clarification point 1:
"We have the SDK interface to schema/schemaless vault mapping already
but we haven't added it in the readme, we will add it") and was still
missing - grepping the README for "schema" turned up nothing.

Added a table documenting which of the four bulk operations apply to
which vault type, matching Devesh's original clarification in the
thread: insert is structured/schema-only, tokenize and deleteTokens
are schemaless-only (confirmed by git history - SK-2646 shipped them
specifically as "Schemaless vault apis"), and detokenize works with
both since it only needs the token itself, not a table.

Verified this isn't enforced anywhere in Validations.java, so worded
it as supported/intended usage rather than something the SDK validates
or blocks.

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

* SK-3061: cover InsertResponseRecord's deprecated constructor directly

Codecov flagged 2 uncovered lines in InsertResponseRecord.java. The
existing deprecated-constructor test only goes through
BulkInsertResponseRecord's deprecated 8-arg constructor, which
delegates straight to the new 9-arg constructor -> new 7-arg super
constructor, never touching InsertResponseRecord's own deprecated
6-arg constructor. Nothing else in the codebase constructs
InsertResponseRecord directly (it's only ever used via the Bulk
subclass), so that constructor was genuinely untested. Added a test
that instantiates it directly and asserts data defaults to null.

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

* Revert unsubstantiated metadata content claim in flowvault README

The previous commit asserted metadata "typically carries skyflowId/
tableName" with equal confidence for both keys. Re-checking: skyflowId
is grounded in real code (Utils.java renames a skyflowID key to
skyflowId when present, which only exists because that key is known
to show up), but tableName is only mentioned in one line of Javadoc
on the generated wire type - no transform logic touches it and no
test in the suite constructs or asserts a tableName key anywhere in
metadata. That's a docstring example, not a verified contract.

Reverting the example and the claim rather than asserting a shape I
can't actually back up.

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

* SK-3061: repeat vault-type applicability inline on each bulk operation section

The "Schema vs. schemaless vaults" table lives under "VaultController -
Bulk operations", but a reader jumping straight to e.g. "# Bulk
Tokenize" via the TOC or a search never sees it. Added a one-line
"Vault type supported" note at the top of each of the four operation
sections (Insert/Tokenize/Detokenize/Delete Tokens), linking back to
the consolidated table for the full picture.

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

* SK-3061: trim vault-type notes to match the terse delete-tokens style

Insert/Tokenize/Detokenize repeated the explanation already in the
consolidated table; shortened to one line each, consistent with the
delete-tokens note.

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

* SK-3061: allowlist "codehaus" for cspell

CI spellcheck flagged flowvault/samples/README.md:38 - "codehaus" from
the org.codehaus.mojo:exec-maven-plugin groupId in the sample run
instructions. Legitimate Maven groupId, not a typo; added alongside
the other domain-specific terms already in the word list (jfrog,
sonatype, etc.).

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

* SK-3061: fix Skyflow.getVaultConfig() crashing on an empty vault list

Skyflow.getVaultConfig() did:
    Object[] array = this.builder.vaultConfigMap.values().toArray();
    return (VaultConfig) array[0];
- an unguarded array[0] access. A client built without any
addVaultConfig(...) call (build() never validates this) throws
ArrayIndexOutOfBoundsException instead of failing predictably.

Traced every call site of "getVaultConfig()" in the codebase first:
all of them are on VaultController (which has its own, unrelated,
already-safe getVaultConfig() returning its single stored config -
no lookup involved), never on Skyflow directly. So this method had
zero usages and zero test coverage anywhere in the suite.

Fixed by mirroring the sibling method's established, already-correct
convention: BaseSkyflow.getVaultConfig(String) is a plain
vaultConfigMap.get(vaultId), returning null when absent - no
exception, no signature change. Rewrote the no-arg overload the same
way (.stream().findFirst().orElse(null)) rather than making it throw
SkyflowException like vault()/vault(String) do, since that would
diverge from its own sibling's contract and require adding a checked
exception to the signature - a source-incompatible change under this
module's japicmp gate for no real benefit. This fix needs no baseline
update: same signature, implementation-only.

Added 7 tests covering: single vault, first-of-several (consistent
with vault()'s "first" semantics, and identity-matched against
getVaultConfig(id)), the empty-client regression case itself, null
after removing the only vault, falling back to the remaining vault
after the first is removed, and the two existing gaps on the
by-id overload (unknown id, empty client).

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

* SK-3061: document detokenize metadata shape, now grounded in the proto

Previously backed out a claim about metadata's typical content since
only skyflowId was grounded (a rename in Utils.java) and tableName
was just a Javadoc description with no code or test behind it.

flowdb_dp_apis.proto's metadata field has its own literal example
value, not just a free-text description: {"table": "table1",
"skyflowID": "4524524534623"}. That's stronger evidence than the
description text, and it reveals the description itself is misleading
- it says "such as tableName or skyflowID" but the actual example key
is "table", not "tableName". Utils.java only renames "skyflowID" to
"skyflowId"; "table" passes through unrenamed.

Documented the real shape and explained the rename explicitly.

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

* SK-3061: add typed Token accessor for insert response tokens

Adds getTokenDetails() to InsertResponseRecord (inherited by
BulkInsertResponseRecord) alongside the existing generic getTokens():
Map<String, Object>. It parses the same data into
Map<String, List<Token>>, so callers get getToken()/getTokenGroupName()
instead of casting Map entries by hand - the exact "known gap" flagged
in the Slack thread and the earlier README note.

Token is a straight reintroduction of flowvault's own pre-rework class
(deleted in 685a82f when insert was reworked around the current
InsertResponseRecord/tokens map), not a new invention - same shape the
user specified, with final fields and a toString() added to match
TokenizeResponseToken's established convention in this package.

getTokenDetails() is purely additive (new method, no signature
changes to anything existing) and computed fresh from getTokens() on
every call rather than stored separately, so the generic and typed
views can never disagree. It normalizes every shape getTokens()'s
value is known to take - a list of {token, tokenGroupName} entries, a
single such entry not wrapped in a list, or a bare token value with no
group info - into a consistent List<Token>, returning null only when
getTokens() itself is null.

Fixed a real landmine along the way: ResponseComponentTests.java had a
dangling {@link Token} javadoc reference left over from the class's
removal - harmless while there was no Token class to resolve to, but
it would have silently started resolving to this new class instead
(with the surrounding comment still saying it "was removed"). Updated
the comment to describe what actually happened.

Added 9 tests covering Token itself and every shape getTokenDetails()
normalizes, including one against BulkInsertResponseRecord directly
(not just the base class) to confirm the inherited method works for
the type callers actually receive. Updated the README's Bulk Insert
section to document getTokenDetails() in place of the manual
cast-it-yourself snippet from the previous commit.

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

* SK-3061: use typed Token accessor in samples, cover its parsing branches

Two changes landed together since both were already staged:

1. BulkInsertSync/Async, BulkMultiTableInsertSync/Async, and
   CustomHeaderExample previously printed record.getTokens() (the raw
   Map<String, Object>) directly. Updated all five to walk
   getTokenDetails() instead - a nested loop over
   Map<String, List<Token>>, printing token.getTokenGroupName()/
   token.getToken() per column, matching the pattern
   BulkTokenizeSync.java already uses for its own typed per-token loop.

2. 3 more tests, closing every branch the previous commit's 9 tests
   left untouched in InsertResponseRecord's new parsing logic:
   - parseTokenEntries's own null check (a column present in the map
     with a null value, as opposed to the whole tokens map being null,
     which was already covered) - that column is now omitted from
     getTokenDetails() rather than appearing with a null/empty entry.
   - toToken's final `return null` and the corresponding "skip adding"
     branch in parseTokenEntries's list loop - a null element sitting
     inside a column's token-group list, which the loop must skip
     rather than NPE on.
   - The tokenGroupName-absent ternary branch in toToken's Map-entry
     parsing - every existing Map-entry test populated both "token"
     and "tokenGroupName" keys, so the "key missing" path was never
     exercised.

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

* SK-3061: make InsertResponseRecord.tokens typed (Map<String, List<Token>>), matching v3

Per explicit direction: retype tokens/getTokens() itself to
Map<String, List<Token>> - matching the pre-rework ("v3") shape -
instead of keeping it generic and exposing a separate getTokenDetails()
accessor alongside it (the previous commit's approach).

BREAKING CHANGE - flagged and confirmed before implementing:
- getTokens()'s return type changes (Map<String,Object> ->
  Map<String,List<Token>>).
- Both InsertResponseRecord constructors' `tokens` parameter type
  changes for the same reason. This isn't a choice - Map<String,Object>
  and Map<String,List<Token>> erase to the same raw `Map` type, so
  Java forbids two constructor overloads at the same arity that differ
  only in that generic parameter. There is no way to add this as a
  new overload alongside the old one; the parameter type has to change
  in place. Same for BulkInsertResponseRecord's two constructors.
- getFields() (deprecated alias) now returns the typed map too, since
  it just delegates to getTokens().

This diverges from "keep contract exactly like API" (the API's own
wire contract types a column's tokens as generic Object) - accepted
as the tradeoff for matching v3's typed shape.

Moved the parsing logic (a column's raw value can be a list of
{token, tokenGroupName} entries, a single such entry, or a bare
value) from InsertResponseRecord into a new static utility,
Token.parseTokens(Map<String, Object>): Map<String, List<Token>>.
Utils.formatBulkInsertResponse now calls it to convert the wire
type's raw tokens map before constructing BulkInsertResponseRecord -
parsing happens once at construction time instead of on every
getTokens() call.

Updated every test that constructed an InsertResponseRecord/
BulkInsertResponseRecord with a raw Map<String,Object> tokens value,
or asserted on the old generic return type. The former
getTokenDetails()-specific tests now test Token.parseTokens()
directly. VaultControllerTests.testBulkInsert_successWithListOfMapsTokenShape
simplifies nicely: it no longer needs to cast the parsed result, since
getTokens() itself is the typed view now.

Updated README and the 5 insert samples (BulkInsertSync/Async,
BulkMultiTableInsertSync/Async, CustomHeaderExample) to use
getTokens() directly instead of the now-removed getTokenDetails().

Needs a japicmp baseline regeneration (scripts/contract-snapshot-
update.sh flowvault) before merge - this is a genuine, intentional
break of the existing contract, unlike every other change in this
branch so far. I could not run mvn/java in this sandbox to regenerate
or verify it myself.

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

* SK-3061: regenerate flowvault japicmp baseline for typed getTokens()

Regenerated via scripts/contract-snapshot-update.sh flowvault, now
that a JDK/Maven are available to run it. Verified mvn -pl
common,flowvault -am verify passes clean (692 tests, japicmp included)
against this new baseline.

The 3 intentional incompatibilities this baseline now accepts, per
the full japicmp diff (flowvault/target/japicmp/default-cli.diff):
- InsertResponseRecord's deprecated 6-arg constructor: tokens param
  generics changed (Map<String,Object> -> Map<String,List<Token>>).
- BulkInsertResponseRecord's deprecated 8-arg constructor: same.
- getFields(): return type generics changed to match.

getTokens() itself was never actually flagged - it's reported as a
brand new method against this baseline (the pre-SK-3061 codebase only
ever had getFields()), so making it typed was non-breaking on its
own. The three real breaks above are avoidable (the deprecated
constructors are a different arity than the primary ones, so no
erasure conflict forces their parameter type to change; getFields()
could reverse-map back to the old shape instead of delegating
directly) - flagged and declined in favor of just regenerating the
baseline, per explicit direction.

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

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* [AUTOMATED] Private Release 1.0.1-dev-b17c4417

* SK-3061: revert getFields() to Map<String, Object>, keep getTokens() typed (#409)

getTokens() itself (item 6, commit 8148504) stays Map<String, List<Token>>.
But getFields() delegating straight to it changed its return-type generics
from Object to List<Token> - a needless japicmp break for a deprecated
alias nobody should be adding new calls to anyway, and one this PR had
flagged as avoidable but initially left in favor of just regenerating the
baseline.

Reverted getFields() to its original Map<String, Object> contract by
rendering getTokens()'s typed data back into that raw shape via a new
package-private Token.toRawTokens(Map<String, List<Token>>) - the inverse
of Token.parseTokens(). Package-private keeps it outside the
accessModifier=protected japicmp contract, so it doesn't itself become a
compatibility commitment.

The round trip is lossless for map-shaped wire input (the normal case) but
lossy for the bare-value edge case parseTokens() also handles (a raw
"tok-abc" string loses its way back to {"token": "tok-abc", "tokenGroupName":
null} - still valid data, just not byte-identical to the original wire
shape). Updated every test asserting on getFields() accordingly, and added
direct toRawTokens() coverage (null input, single/multiple token groups,
round-trip-with-map-input) in ResponseComponentTests.

Regenerated flowvault/api-report/skyflow-flowvault-java.baseline.jar via
scripts/contract-snapshot-update.sh flowvault - this supersedes the
baseline regenerated in f326ec2, since that one still had getFields()
returning the typed map. mvn -pl common,flowvault -am verify passes clean
against it.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* [AUTOMATED] Private Release 1.0.1-dev-7f1a295d

* SK-3061: fix Bulk Insert README's Token snippet to actually iterate the response (#411)

The snippet added when getTokens() was made typed (#408) used `record`
without ever introducing it - it reads as if it stood alone, but record
only exists inside a loop over insertResponse.getRecords(). Wrapped it in
that outer loop (plus a null check, since getTokens() is null on a failed
record), matching the pattern the Bulk Detokenize section's equivalent
metadata snippet already follows.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* [AUTOMATED] Private Release 1.0.1-dev-f6c3370a

* SK-3061: make DetokenizeResponseRecord.getMetadata() typed (#410)

* SK-3061: make DetokenizeResponseRecord.getMetadata() typed

metadata was a raw Map<String, Object>, requiring callers to cast into it
to reach skyflowId/tableName - the same usability gap Token closed for
InsertResponseRecord.getTokens(). Added DetokenizeMetadata (getSkyflowId(),
getTableName()) and made getMetadata() itself return it, matching how
getTokens() was made typed directly rather than adding a second accessor.

DetokenizeMetadata.parseMetadata(Map<String, Object>) normalizes the wire
shape - the proto's own literal example uses {"table": ..., "skyflowID": ...},
but Utils.java's existing handling already renamed skyflowID -> skyflowId
before this reached the record constructor, so parseMetadata accepts either
casing for both keys and moves that normalization out of Utils.java and
into one place, alongside the typing.

This changes DetokenizeResponseRecord/BulkDetokenizeResponseRecord's
metadata constructor parameter and getMetadata()'s return type from
Map<String, Object> to DetokenizeMetadata - a japicmp-breaking change,
same shape as the earlier getTokens() typing. Regenerated
flowvault/api-report/skyflow-flowvault-java.baseline.jar via
scripts/contract-snapshot-update.sh flowvault.

Updated README's Bulk Detokenize section (JSON sample + a typed-access
snippet) and every test constructing a record with a raw metadata map.

mvn -pl common,flowvault -am test -> 702 tests, 0 failures.
mvn -pl common,flowvault -am verify -> BUILD SUCCESS against the
regenerated baseline.

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

* SK-3061: add flowdb/unrenamed to cspell dictionary

Flagged by the cspell CI check on PR #410 - both words come from the
DetokenizeMetadata Javadoc/test comments referencing flowdb_dp_apis.proto
and describing the wire's unrenamed table key.

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

* SK-3061: add matching response-iteration snippets to Bulk Tokenize/Delete Tokens

Bulk Insert and Bulk Detokenize's README sections each show a code snippet
iterating the response; Bulk Tokenize and Bulk Delete Tokens only had a
JSON sample with no matching Java. Added one to each, same pattern:

- Bulk Tokenize: outer loop over records, inner loop over each record's
  tokens (it reports at two levels - see the paragraph directly above).
- Bulk Delete Tokens: single loop over records, success/error branch on
  getError().

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

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* [AUTOMATED] Private Release 1.0.1-dev-05ce8d07

* SK-3061: fix bulkInsert/bulkInsertAsync's inconsistent exception handling (#412)

* SK-3061: make bulkInsertAsync's exception handling consistent with the other bulk async methods

bulkInsertAsync only caught ApiClientApiException. bulkDetokenizeAsync,
bulkDeleteTokensAsync, and bulkTokenizeAsync all additionally catch
SkyflowException (rethrow as-is, so it isn't double-wrapped) and generic
Exception (wrapped into SkyflowException) - so any unexpected failure in
their synchronous setup still surfaces as SkyflowException, matching the
method's declared contract.

bulkInsertAsync had neither, so anything thrown from its synchronous setup
that wasn't ApiClientApiException (e.g. a caller-supplied RequestInterceptor
throwing, since it's invoked synchronously per batch inside
insertBatchFutures) leaked out as its raw exception type instead of
SkyflowException. Added the same two catch blocks.

Batch-level failures inside the CompletableFuture pipeline itself are
unaffected - insertBatchFutures already turns those into error-shaped
BulkInsertResponse records via .exceptionally(), same as before.

Added a regression test that reproduces the leak via a throwing
interceptor and confirms it stays fixed (fails without the fix, verified
by temporarily reverting it locally before writing this up).

mvn -pl common,flowvault -am verify -> 704 tests, 0 failures, BUILD
SUCCESS. No japicmp impact - no signature changes, just added catch
blocks.

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

* SK-3061: fix the same exception leak in bulkInsert (sync), not just bulkInsertAsync

processBulkInsertSync called insertBatchFutures (which invokes the caller's
RequestInterceptor synchronously per batch) before its own try started, so
an interceptor that threw leaked its raw exception type straight out of
bulkInsert() - the sync twin of the bug this PR already fixed on the async
side. Every sibling operation's equivalent helper (processBulkDetokenizeSync,
processBulkDeleteTokensSync, processBulkTokenizeSync) already makes that
call inside its own try; insertBatchFutures itself was never missing
anything relative to its siblings - all four *BatchFutures helpers have
zero catch blocks of their own, identically. The asymmetry was purely
processBulkInsertSync's call-site placement.

Moved the insertBatchFutures call inside the try, matching the other three
helpers' structure exactly. Added a regression test mirroring
testBulkInsertAsync_unexpectedExceptionWrappedAsSkyflowException for the
sync path.

mvn -pl common,flowvault -am verify -> 705 tests, 0 failures, BUILD
SUCCESS. No japicmp impact - no signature changes.

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

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* [AUTOMATED] Private Release 1.0.1-dev-291cdd3c

* SK-3061: add path to Token; fix stale samples pom.xml version (#413)

* SK-3061: add path to Token, for tokens generated from nested/structured columns

flowdb_dp_apis.proto's own literal example response for insert includes a
"path" key on token entries for a structured column (e.g. an "address"
object tokenized per nested field: {"path": "street", "token": "...",
"tokenGroupName": "..."}) - present alongside token/tokenGroupName, absent
for flat columns. Token.toToken()/parseTokens() silently dropped it.

Added Token.getPath() (nullable) and a new 3-arg constructor
(Token(token, tokenGroupName, path)); the existing 2-arg constructor
delegates to it with path=null, so it stays fully backward compatible -
this is purely additive, no existing signature changed. Confirmed via
japicmp: BUILD SUCCESS, zero incompatibilities, no baseline regen needed.

Token.parseTokens() now reads "path" from each raw entry when present.
Token.toRawTokens() (getFields()'s deprecated rendering) only adds a
"path" key when the Token actually has one, rather than unconditionally -
so a path-less round trip stays exactly as lossless as it was before path
existed (confirmed by the existing
testToRawTokens_isTheInverseOfParseTokensForMapShapedInput test, which
would otherwise have gained a spurious "path": null key it never had).

Updated Token's Javadoc, README's Bulk Insert Token section, and added
tests for: the new constructor, parsing path when present/absent, and
toRawTokens() rendering path only when present.

mvn -pl common,flowvault -am verify -> 707 tests, 0 failures, BUILD
SUCCESS.

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

* SK-3061: fix samples pom.xml's stale skyflow-flowvault-java dev version

flowvault/samples/pom.xml pinned skyflow-flowvault-java to
3.0.0-beta.13-dev.18f8f1ba - a private dev-build version string carried
over by mistake from the unrelated flowvault-release/26.8.1 branch when
b3cf737 (SK-3002 release/26.8.1 #386) switched the sample's dependency
from skyflow-java (v2) to skyflow-flowvault-java. Pinned to 1.0.1, the
actual latest public GA release (main's [AUTOMATED] Public Release - 1.0.1).

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

* Revert: bulkInsert (sync) structural exception-handling fix from #412

#412's second commit moved processBulkInsertSync's call to
insertBatchFutures inside its own try, fixing a raw-exception leak via a
throwing RequestInterceptor. Further investigation showed the same gap
(missing SkyflowException-passthrough / generic Exception catch) exists
identically in bulkDetokenize/bulkDeleteTokens/bulkTokenize's own outer
catch lists too - not just bulkInsert's - so a narrower, operation-specific
structural fix isn't the right shape for this. Reverting it here pending a
uniform fix across all four sync methods.

Removed testBulkInsert_unexpectedExceptionWrappedAsSkyflowException, which
tested the now-reverted behavior.

mvn -pl common,flowvault -am test -> 708 tests, 0 failures, BUILD SUCCESS.

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

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* chore: trigger internal-release workflow (re-run after commit-message false-skip)

* chore: re-trigger internal-release workflow (#414)

* [AUTOMATED] Private Release 1.0.1-dev-75ede823

* SK-3061 Flowvault release/26.8.13.1 (#416)

* SK-3061 fix the unhandled exceptions

* [AUTOMATED] Private Release 1.0.1-dev-2448f08e

* SK-3061: fix bulkInsert(sync) interceptor-exception leak

processBulkInsertSync called insertBatchFutures - which synchronously
invokes the caller's RequestInterceptor - before entering its own try
block, so a throwing interceptor escaped bulkInsert() as a raw,
undeclared exception instead of the documented SkyflowException.

processBulkDetokenizeSync/processBulkDeleteTokensSync/processBulkTokenizeSync
already call their own *BatchFutures inside their own try/catch(Exception),
which is why they were not exploitable the same way. This moves
insertBatchFutures's call inside processBulkInsertSync's existing try,
matching that same structure, instead of adding a new catch-all to the
4 public sync methods (the broader fix reverted here).

Added a regression test asserting a throwing interceptor surfaces as
SkyflowException from bulkInsert. Confirmed via TDD: failed before this
change (raw IllegalStateException), passes after.

mvn -pl common,flowvault -am test -> 709 (flowvault) + 106
(common/ValidationsTests) tests, 0 failures, BUILD SUCCESS.

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

* [AUTOMATED] Private Release 1.0.1-dev-7133bb23

* SK-3061: close PR #416 Codecov patch-coverage gaps

Adds targeted unit tests for the new branches the Codecov patch report
flagged as uncovered on PR #416 (82.52% patch coverage, 43 lines missing):

- common/BearerToken: extractAccessToken() success branch (reflection).
- flowvault/VaultClient: updateExecutorInHTTP wraps the retry
  interceptor's IllegalArgumentException (negative maxRetries) as
  SkyflowException.
- flowvault/BulkInsertResponse & BulkDetokenizeResponse: buildSummary()
  with both records and originalPayload null (falls through the nested
  ternary's innermost 0 branch).
- skyvault/VaultController: ApiClientException (network-error) catch in
  insert/detokenize/get/update/delete/query/tokenize/uploadFile, plus
  getFormattedBatchInsertRecord's "Body present but not a JSON object"
  branch.
- skyvault/DetectController: ApiClientException catch in
  deidentifyText/reidentifyText/pollForResults/getDetectRun.
- skyvault/Validations: validateGetRequest's orderBy != null false
  branch, unreachable through the public builder (which coalesces null
  to ORDER_ASCENDING) so forced via reflection.

Two gaps were left uncovered on purpose because they are dead code, not
missing tests (confirmed empirically, not just by inspection):

- flowvault/VaultController's 4 new catch (ApiClientException e) blocks
  in bulkInsert/bulkDetokenize/bulkDeleteTokens/bulkTokenize (sync) are
  unreachable: each process*Sync helper already wraps everything in its
  own catch (Exception e) before anything can reach the outer catch.
  The existing testBulkInsert_throwingInterceptorWrappedAsSkyflowException
  regression test demonstrates this directly.
- skyvault/Validations' validateTokensForInsertRequest has
  "tokensMap == null || valuesMap == null"; the valuesMap == null side
  can never be true when called from validateInsertRequest, since that
  method already rejects any null entry in `values` earlier in the same
  call.

mvn -o test (common, flowvault, skyvault): 0 failures, 0 errors.

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

* [AUTOMATED] Private Release 1.0.1-dev-f26c4df0

---------

Co-authored-by: skyflow-bharti <skyflow-bharti@users.noreply.github.com>
Co-authored-by: Devesh Bhardwaj <devesh.bhardwaj@skyflow.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Devesh-Skyflow <Devesh-Skyflow@users.noreply.github.com>

* chore: re-trigger internal-release workflow (#418)

* chore: re-trigger internal-release workflow

* Update pom.xml

* [AUTOMATED] Private Release 1.0.1-dev-55bd5f19

* SK-3061: flatten Tokenize's response contract, generated type through public SDK shape (#419)

* chore: trigger internal-release workflow rerun

Empty commits don't trigger this workflow (paths-ignore treats a
zero-file diff as vacuously all-ignored), so this touches pom.xml
directly instead.

* SK-3061: fix stale nested Tokenize response type to match real API contract

V1FlowTokenizeResponseObject modeled a value + nested tokens[] array that
the real API has never sent since a 2026-03-17 contract flattening (proto
commit 7a656c0f, aligning Tokenize's shape with Detokenize's). The generated
type was built from a stale spec snapshot and never corrected, so the SDK
only produced correct output via a hand-written flatToken() workaround that
scavenged token/tokenGroupName/error/httpCode out of Jackson's
additionalProperties catch-all instead of reading real fields.

Confirmed against the live proto (skyflowapi/common), a captured production
response fixture already in this repo's tests, and the actual
skyflow-fern-config/schemaless OpenAPI spec - all agree the wire shape is
flat: token/value/tokenGroupName/error/httpCode as siblings, one row per
(value, token group).

Changes:
- V1FlowTokenizeResponseObject: replaced value+tokens(nested) with the real
  flat fields (token, value, tokenGroupName, error, httpCode), matching the
  sibling V1FlowDetokenizeResponseObject's existing shape/conventions.
- Deleted FlowTokenizeResponseObjectToken (the nested per-token type is no
  longer referenced anywhere).
- Utils.java: buildTokenizeResponseTokens() now reads the flat fields
  directly; deleted flatToken() and the nested-vs-flat branching entirely.
  groupTokenizeRows()/acceptsRow()/valuesMatch() are untouched - the
  value-matching/folding logic that reconstructs per-record grouping from
  flat rows was already correct and needed no changes.
- Updated the 6 tests that built the old nested shape directly to build the
  flat shape instead; replaced one test whose entire premise ("if the API
  ever returns the nested shape") is no longer a real code path with
  equivalent flat-shape coverage of the same folding behavior.

Deliberately scoped to only the Tokenize response type + its consumer in
Utils.java: Insert/Detokenize/DeleteToken and the FlowserviceClient/
VaultClient accessor structure are completely untouched, since they have no
known contract issue and a full client regeneration (evaluated and set
aside) would have meant unnecessary risk for operations that already work.

Verified: mvn test (708/708, excluding one pre-existing unrelated failure -
common's TokenTests.testExpiredTokenForIsExpiredToken depends on an
uncommitted .env secret); mvn verify/japicmp shows zero public API changes
(only diff present is an unrelated, already-merged Token.getPath() addition
from #413).

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

* SK-3061: flatten public BulkTokenizeResponse to match the API's row shape

BulkTokenizeResponse.getRecords() previously grouped rows back into a
records[].tokens[] structure keyed by submitted value. The real API has
never nested this way - it returns one flat row per (value, token group)
pair. This drops the SDK-side grouping: BulkTokenizeResponseRecord and
TokenizeResponseRecord are now flat (value, tokenGroupName, token,
httpCode, error, requestId) with index kept on BulkTokenizeResponseRecord
so rows from the same submitted value can still be correlated. Summary
classification (totalTokenized/totalPartial/totalFailed) and
getRecordsToRetry() are recomputed by grouping on index internally,
including the case where a value gets zero rows back.

TokenizeResponseToken is removed; its fields fold directly into
TokenizeResponseRecord. Utils.java's row-to-index correlation
(acceptsRow/valuesMatch/batch-splitting on duplicate values) is
unchanged - only the final emission step is flat instead of nested.

Samples and README updated to the flat shape. This is an intentional
breaking change to BulkTokenizeResponse's public constructors; the
japicmp baseline is regenerated accordingly.

* chore: whitelist noextension in cspell word list

DetectControllerTests.java's testGetBaseFileName_withoutExtensionReturnsWholeName
(already merged on flowvault-release/26.8.13, ahead of this branch) uses the
synthetic filename "noextension" as a test fixture, same pattern as the
existing nocreds/nodir entries. Not otherwise related to this PR's Tokenize
change; adding it here since it's what's blocking this PR's cspell check.

* SK-3061: add BYOT-with-single-invalid-group coverage

Closes the gap flagged earlier: every existing BYOT test only exercised
naming too many groups (the 'should contain one token group' rejection).
This covers a BYOT record naming exactly one group where that group
itself is invalid - same error shape as the non-BYOT case, just on a
BYOT record.

Fixture verified against a live call (dev vault, single BYOT record
naming one nonexistent group): the real API returns tokenGroupName=null,
token=null, httpCode=400, and 'Tokenize failed. Token group X is
invalid. Specify a valid token group.' - matching this test exactly.

* SK-3061: cover partial tokenize failure with a retryable group through VaultController

Closes the gap noted while discussing the retry path live: the only
existing coverage of 'one value, one group succeeds, one group fails
retryably (5xx)' was BulkResponseTests/BulkRetryAndSummaryTests
exercising BulkTokenizeResponse directly. This runs the same scenario
through VaultController.bulkTokenize() with a mocked raw client, so
batching/index-assignment and getRecordsToRetry() are exercised
together end to end, not just the summary math in isolation.

A real 5xx can't be forced from the live API on demand (server-side
failure, not something a crafted request triggers), so this is mocked,
same as the existing apiErrorCapturedInErrors tests in this file.

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* [AUTOMATED] Private Release 1.0.1-dev-34618022

* SK-3061: close tokenize patch-coverage gaps flagged on #420 (#421)

* SK-3061: close tokenize patch-coverage gaps flagged by codecov on #420

Traced each flagged line in BulkTokenizeResponse.java and Utils.java via
jacoco rather than guessing, and added targeted tests for the ones that
were real, in-scope gaps:

- BulkTokenizeResponse.buildSummary()/getRecordsToRetry(): the no-payload
  fallback branch (records present, originalPayload null - reachable via
  the public 2-arg constructor) was entirely untested, including the
  partial-outcome and null+null cases.
- Utils.tokenizeRecordsFromErrorBody(): 'response' present-but-empty,
  present-but-explicitly-null, and unparseable-shape all fall back to
  the status-code summary but none were tested.
- Utils.groupTokenizeRows(): the null-batch and empty-(non-null)-batch
  fallback (one record per row, no correlation possible) was untested.
- Utils.handleBulkTokenizeBatchException()/extractBatchErrorMessage():
  an explicitly-empty (non-null) token group list, and the nested
  {'error': {...}} object's 'error'-over-'message' preference and
  no-string-found fallback, were untested.
- Utils.formatBulkTokenizeResponse(null, ...) was untested.

Left alone, deliberately:
- VaultController.java's uncovered lines are all from #412's exception
  handling (bulkInsert/bulkInsertAsync), not from the tokenize work -
  out of scope here.
- Utils.java lines in formatBulkDeleteTokensResponse()/isFailedRecord()
  are DeleteTokens code, not tokenize.
- buildTokenizeResponseRecord()'s '.get() != null' check on an Optional
  already known to be present is structurally unreachable-false per
  Optional's own contract - not a real gap, can't be forced by a test.

* chore: whitelist deserialises in cspell word list

Same British-spelling pattern already whitelisted for serialise/
serialises/deserialise/deserialised; deserialises (third-person form)
was missing, flagged from a comment added in this branch's coverage-gap
tests.

* chore: whitelist unparseable in cspell word list

* [AUTOMATED] Private Release 1.0.1-dev-490ffb14

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Devesh-Skyflow <Devesh-Skyflow@users.noreply.github.com>
Co-authored-by: skyflow-bharti <118584001+skyflow-bharti@users.noreply.github.com>
Co-authored-by: skyflow-bharti <skyflow-bharti@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant