feat(ddic): add full DDIC object type support (DOMA, DTEL, TABL, TTYP)#84
Merged
ThePlenkov merged 5 commits intomainfrom Mar 16, 2026
Merged
feat(ddic): add full DDIC object type support (DOMA, DTEL, TABL, TTYP)#84ThePlenkov merged 5 commits intomainfrom
ThePlenkov merged 5 commits intomainfrom
Conversation
- Add XSD schemas for TABL (table/structure) and TTYP (table type) - types/dd02v.xsd: table header type - types/dd03p.xsd: table field type + Dd03pTableType wrapper - types/dd40v.xsd: table type header type - tabl.xsd: concrete document schema using xs:redefine - ttyp.xsd: concrete document schema using xs:redefine - Add abapGit handlers - tabl.ts: serialize/deserialize TABL objects (tables + structures) - ttyp.ts: serialize/deserialize TTYP objects (table types) - Register DOMA, DTEL, TABL, TTYP in objects/index.ts - Add test fixtures - tabl/zage_structure.tabl.xml (structure) - tabl/zage_transparent_table.tabl.xml (transparent table) - ttyp/zage_string_table.ttyp.xml (string table type) - ttyp/zage_struct_table_type.ttyp.xml (structure table type) - Add schema tests for TABL and TTYP - XSD validation, parse, validate content, build, round-trip - Regenerate all abapGit schemas (9 total: +tabl, +ttyp) All 56 tests pass. Build succeeds.
📝 WalkthroughWalkthroughAdds metadata-only DDIC support: new ADT contracts and response types, ADK models for Domain/DataElement/Table/Structure/TableType, abapGit handlers and XSDs/fixtures/tests, accept-header and crud helper updates, and small ADK save/unlock behavior changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer/Client
participant ADT as ADT Client (ddicContract)
participant ADK as ADK Models
participant ABAPGIT as abapGit Handlers
Dev->>ADK: AdkTable.get(name)
ADK->>ADT: ctx.client.adt.ddic.tables.get(name)
ADT-->>ADK: TableResponse (blueSource)
ADK-->>Dev: AdkTable instance
Dev->>ABAPGIT: export/import request
ABAPGIT->>ADK: toAbapGit/fromAbapGit(Adk* instance)
ABAPGIT-->>Dev: ABAP-XML fixture
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
Contributor
|
View your CI Pipeline Execution ↗ for commit 2816dd4
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Pull request overview
Adds end-to-end DDIC support (DOMA, DTEL, TABL/DT+DS, TTYP) across schemas, contracts, ADK models, and abapGit import/export tooling.
Changes:
- Generate and export new ADT XSD schemas/types for DDIC objects (domain, data elements, table type, table settings).
- Introduce DDIC REST contracts/modules and surface them through adt-client + ADK.
- Add abapGit XSDs, fixtures, schema tests, and object handlers for DDIC types.
Reviewed changes
Copilot reviewed 39 out of 57 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/adt-schemas/ts-xsd.config.ts | Adds DDIC XSDs to schema generation targets. |
| packages/adt-schemas/src/schemas/generated/types/sap/tabletype.types.ts | Generated TS types for ADT table type payloads. |
| packages/adt-schemas/src/schemas/generated/types/sap/tablesettings.types.ts | Generated TS types for ADT table settings payloads. |
| packages/adt-schemas/src/schemas/generated/types/sap/domain.types.ts | Generated TS types for ADT domain payloads. |
| packages/adt-schemas/src/schemas/generated/types/sap/dataelements.types.ts | Generated TS types for ADT data element payloads. |
| packages/adt-schemas/src/schemas/generated/typed.ts | Exposes typed schema exports for new DDIC schemas. |
| packages/adt-schemas/src/schemas/generated/schemas/sap/tabletype.ts | Generated runtime schema for ADT table type XML. |
| packages/adt-schemas/src/schemas/generated/schemas/sap/tablesettings.ts | Generated runtime schema for ADT table settings XML. |
| packages/adt-schemas/src/schemas/generated/schemas/sap/index.ts | Re-exports new SAP schemas. |
| packages/adt-schemas/src/schemas/generated/schemas/sap/domain.ts | Generated runtime schema for ADT domain XML. |
| packages/adt-schemas/src/schemas/generated/schemas/sap/dataelements.ts | Generated runtime schema for ADT data elements XML. |
| packages/adt-plugin-abapgit/xsd/types/dd40v.xsd | Adds abapGit type for DD40V (table type header). |
| packages/adt-plugin-abapgit/xsd/types/dd03p.xsd | Adds abapGit type for DD03P (+ table wrapper) (fields). |
| packages/adt-plugin-abapgit/xsd/types/dd02v.xsd | Adds abapGit type for DD02V (table/structure header). |
| packages/adt-plugin-abapgit/xsd/ttyp.xsd | Adds abapGit object schema for TTYP. |
| packages/adt-plugin-abapgit/xsd/tabl.xsd | Adds abapGit object schema for TABL. |
| packages/adt-plugin-abapgit/ts-xsd.config.ts | Includes tabl/ttyp in abapGit schema generation. |
| packages/adt-plugin-abapgit/tests/schemas/ttyp.test.ts | Schema-level parse/validate/round-trip tests for TTYP fixtures. |
| packages/adt-plugin-abapgit/tests/schemas/tabl.test.ts | Schema-level parse/validate/round-trip tests for TABL fixtures. |
| packages/adt-plugin-abapgit/tests/fixtures/ttyp/zage_struct_table_type.ttyp.xml | New TTYP fixture (structure row type). |
| packages/adt-plugin-abapgit/tests/fixtures/ttyp/zage_string_table.ttyp.xml | New TTYP fixture (string row type). |
| packages/adt-plugin-abapgit/tests/fixtures/tabl/zage_transparent_table.tabl.xml | New TABL fixture (transparent table). |
| packages/adt-plugin-abapgit/tests/fixtures/tabl/zage_structure.tabl.xml | New TABL fixture (structure). |
| packages/adt-plugin-abapgit/src/schemas/generated/types/ttyp.ts | Generated TS types for abapGit TTYP XML. |
| packages/adt-plugin-abapgit/src/schemas/generated/types/tabl.ts | Generated TS types for abapGit TABL XML. |
| packages/adt-plugin-abapgit/src/schemas/generated/types/index.ts | Re-exports generated TTYP/TABL types. |
| packages/adt-plugin-abapgit/src/schemas/generated/schemas/ttyp.ts | Generated runtime schema for abapGit TTYP XML. |
| packages/adt-plugin-abapgit/src/schemas/generated/schemas/tabl.ts | Generated runtime schema for abapGit TABL XML. |
| packages/adt-plugin-abapgit/src/schemas/generated/schemas/index.ts | Re-exports generated TTYP/TABL schemas. |
| packages/adt-plugin-abapgit/src/schemas/generated/index.ts | Registers new abapGit schema instances (tabl/ttyp). |
| packages/adt-plugin-abapgit/src/lib/handlers/objects/ttyp.ts | Adds abapGit handler for TTYP objects. |
| packages/adt-plugin-abapgit/src/lib/handlers/objects/tabl.ts | Adds abapGit handler for TABL objects. |
| packages/adt-plugin-abapgit/src/lib/handlers/objects/index.ts | Exports DDIC handlers from handler index. |
| packages/adt-plugin-abapgit/src/lib/handlers/objects/dtel.ts | Adds abapGit handler for DTEL objects. |
| packages/adt-plugin-abapgit/src/lib/handlers/objects/doma.ts | Adds abapGit handler for DOMA objects. |
| packages/adt-plugin-abapgit/src/lib/handlers/adk.ts | Re-exports new ADK DDIC object classes for handlers. |
| packages/adt-contracts/src/generated/schemas.ts | Exposes new DDIC schemas via contracts’ schema registry. |
| packages/adt-contracts/src/adt/index.ts | Adds ddic module to top-level ADT contract. |
| packages/adt-contracts/src/adt/ddic/tabletypes.ts | Adds CRUD contract for table types. |
| packages/adt-contracts/src/adt/ddic/tables.ts | Adds table contract (untyped responses pending schema). |
| packages/adt-contracts/src/adt/ddic/structures.ts | Adds structure contract (untyped responses pending schema). |
| packages/adt-contracts/src/adt/ddic/index.ts | Aggregates ddic subcontracts into a single module. |
| packages/adt-contracts/src/adt/ddic/domains.ts | Adds CRUD contract for domains. |
| packages/adt-contracts/src/adt/ddic/dataelements.ts | Adds CRUD contract for data elements. |
| packages/adt-client/src/index.ts | Re-exports DDIC response types for consumers. |
| packages/adk/src/objects/ddic/ttyp/ttyp.model.ts | Adds ADK model for TTYP + self-registration. |
| packages/adk/src/objects/ddic/ttyp/index.ts | Exports TTYP ADK model/types. |
| packages/adk/src/objects/ddic/tabl/tabl.model.ts | Adds ADK models for TABL tables + structures. |
| packages/adk/src/objects/ddic/tabl/index.ts | Exports TABL ADK models/types. |
| packages/adk/src/objects/ddic/index.ts | Exposes all DDIC ADK objects from a single entrypoint. |
| packages/adk/src/objects/ddic/dtel/index.ts | Exports DTEL ADK model/types. |
| packages/adk/src/objects/ddic/dtel/dtel.model.ts | Adds ADK model for DTEL + self-registration. |
| packages/adk/src/objects/ddic/doma/index.ts | Exports DOMA ADK model/types. |
| packages/adk/src/objects/ddic/doma/doma.model.ts | Adds ADK model for DOMA + self-registration. |
| packages/adk/src/index.ts | Re-exports DDIC ADK objects/types and response types. |
| packages/adk/src/base/kinds.ts | Adds new DDIC kinds + kind→object type mapping. |
| packages/adk/src/base/adt.ts | Wires ddic contracts into ADK’s contract facade + exports types. |
Comments suppressed due to low confidence (1)
packages/adk/src/objects/ddic/tabl/tabl.model.ts:85
AdkStructureis introduced with a distinctStructureKind, but onlyTableKindis registered for ADT main typeTABL. With the current registry implementation (maps by main type only),TABL/DScannot resolve toAdkStructure(andbyKind(Structure, ...)will throw), so structures will be instantiated asAdkTableand use the wrong endpoint (/ddic/tables). Consider making the registry subtype-aware (e.g., prefer full type mapping likeTABL/DTvsTABL/DS) and registeringAdkStructureaccordingly, or otherwise remove/avoid the separateStructureKinduntil resolution is implemented.
// Self-register with ADK registry
import { registerObjectType } from '../../../base/registry';
registerObjectType('TABL', TableKind, AdkTable);
// Note: Structure uses same main type TABL but different ADK kind
// Registration uses TABL main type - structures will be resolved via subtype logic
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| * | ||
| * ADT endpoint: /sap/bc/adt/ddic/tabletypes | ||
| * Content-Type: application/vnd.sap.adt.tabletype.v1+xml | ||
| * Object type: TTYP/DA (ttypda) |
Member
Author
There was a problem hiding this comment.
Fixed in 2816dd4 — comment now reads TTYP/TT.
Comment on lines
+26
to
+33
| fromAbapGit: ({ DD02V }) => | ||
| ({ | ||
| name: (DD02V?.TABNAME ?? '').toUpperCase(), | ||
| type: DD02V?.TABCLASS === 'INTTAB' ? 'TABL/DS' : 'TABL/DT', | ||
| description: DD02V?.DDTEXT, | ||
| language: sapLangToIso(DD02V?.DDLANGUAGE), | ||
| masterLanguage: sapLangToIso(DD02V?.DDLANGUAGE), | ||
| }) as { name: string } & Record<string, unknown>, |
Member
Author
There was a problem hiding this comment.
Valid observation, acknowledged for follow-up. The registry currently maps by main type only (TABL). fromAbapGit correctly returns type: TABL/DS for structures, so the data is preserved — constructor subtype-awareness is the missing piece. Tracked for a future PR.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
packages/adk/src/objects/ddic/doma/doma.model.ts (1)
43-45: Avoid untypedanyoncrudContractgetter.The
crudContractgetter returnsany, which bypasses type safety. Per coding guidelines,anyshould include a comment explaining why it's necessary, or the types should be fixed at source.Consider adding a type annotation or a comment explaining the limitation:
♻️ Suggested fix
- protected override get crudContract(): any { + // eslint-disable-next-line `@typescript-eslint/no-explicit-any` -- ADK base model expects generic CRUD contract + protected override get crudContract(): any { return this.ctx.client.adt.ddic.domains; }Alternatively, if there's a proper type available from the contract definitions, consider using that instead of
any.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adk/src/objects/ddic/doma/doma.model.ts` around lines 43 - 45, The getter get crudContract currently returns the untyped any; either replace any with the precise contract type exported by your ADT client (the type of this.ctx.client.adt.ddic.domains) or, if no suitable type exists, keep any but add a short explanatory comment above get crudContract documenting why any is required and a TODO to fix once the contract typings are added; update the signature from "protected override get crudContract(): any" to use the correct interface name (or add the explanatory comment) so the intention and limitation are explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/adk/src/objects/ddic/dtel/dtel.model.ts`:
- Around line 46-47: The getter protected override get crudContract in
dtel.model.ts uses an untyped any which violates strict-mode; fix by either
adding the eslint disable comment above the getter (// eslint-disable-next-line
`@typescript-eslint/no-explicit-any`) with a short justification, or better import
the proper contract type (e.g., import type { DataelementsContract } from
'@abapify/adt-contracts') and change the signature to protected override get
crudContract(): DataelementsContract { return
this.ctx.client.adt.ddic.dataelements; } so the getter is correctly typed;
update the dtel.model.ts getter accordingly.
In `@packages/adk/src/objects/ddic/tabl/tabl.model.ts`:
- Around line 82-84: The registry currently registers 'TABL' -> AdkTable via
registerObjectType('TABL', TableKind, AdkTable) but resolveType() only extracts
the main type and always returns AdkTable, so AdkStructure (for 'TABL/DS') is
unreachable; fix by either (A) registering the structure subtype explicitly
(registerObjectType('TABL/DS', TableKind, AdkStructure)) so the factory can
return AdkStructure, or (B) make resolveType() subtype-aware: parse the full
type token (e.g. split on '/') inside resolveType(), check for known subtypes
like 'DS' and map them to AdkStructure (while falling back to AdkTable for plain
'TABL'), and update registry lookup logic to prefer exact subtype keys before
main-type keys.
- Around line 43-44: Replace the loose any return type on the protected getter
get crudContract with the precise indexed type from the adt client rather than
any: use the type of this.ctx.client.adt.ddic['tables'] for the TablModel getter
(replace the any on get crudContract in the class that returns tables) and
likewise use this.ctx.client.adt.ddic['structures'] for the AdkStructure getter
(replace the any on its get crudContract), removing the any assertions and
returning/annotating the exact indexed contract type instead.
In `@packages/adk/src/objects/ddic/ttyp/ttyp.model.ts`:
- Around line 46-47: The getter override crudContract currently returns any
(protected override get crudContract(): any) which breaks strict linting; fix by
either (A) adding a one-line eslint disable with a brief justification above the
getter (e.g., eslint-disable-next-line `@typescript-eslint/no-explicit-any` //
wrapper for ADT client contract) or (B) properly typing it: export the concrete
TabletypesContract type from packages/adk/src/base/adt.ts, import that type into
this model, and change the getter signature to use that type and return
this.ctx.client.adt.ddic.tabletypes; apply the same pattern for the other
DDIC/repository overrides (tabl, dtel, doma, prog, intf, fugr, devc, clas).
In `@packages/adt-contracts/src/adt/ddic/structures.ts`:
- Around line 30-33: The request headers currently set 'Content-Type' to the
wildcard 'application/*' which is wrong for POSTs; update the headers object
used in the structures request so 'Content-Type' uses the concrete MIME value
(the contentType variable / declared structures MIME type) instead of
'application/*' — locate the headers definition in structures.ts (the headers: {
Accept: contentType, 'Content-Type': 'application/*' } block) and replace the
wildcard with the concrete contentType variable or the exact structures MIME
string so the POST sends the correct Content-Type.
In `@packages/adt-contracts/src/adt/ddic/tables.ts`:
- Line 17: The table-name path construction doesn't URL-encode names, so replace
uses of nameTransform (or direct n.toLowerCase()) in ADT path builders with
encodeURIComponent(name.toLowerCase()) to safely handle special characters and
namespaced names; update all path construction sites that build ADT endpoints
for tables (references around the nameTransform usage and the functions that
construct table paths) to call encodeURIComponent on the lowercased table name
so paths use encodeURIComponent(name.toLowerCase()) consistently with
packages/adk.
- Around line 27-30: The post and put handlers for the tables contract currently
set body: undefined making create/update non-functional; update the table client
in packages/adt-contracts/src/adt/ddic/tables.ts so post and put accept a table
payload parameter and pass it as the request body following the existing crud()
helper pattern (mirror how crud() constructs body/response mappings for
create/update endpoints), update the method signatures for post and put to
include options and a payload (e.g., post: (payload, options?) / put: (id,
payload, options?)) and ensure responses remain correctly typed, and add a
corresponding contract test file for tables that asserts method signatures,
paths, headers, query params, and response schemas per the contract testing
conventions used elsewhere in the repo.
In `@packages/adt-contracts/src/adt/ddic/tabletypes.ts`:
- Line 6: Update the module documentation in tabletypes.ts to correct the
object-type label from "TTYP/DA" to "TTYP/TT" so it matches the implemented
table type kind used by the new handler/model flow; locate the doc comment that
currently reads TTYP/DA and replace it with TTYP/TT to keep the module docs
consistent with the table type kind.
In `@packages/adt-plugin-abapgit/tests/schemas/tabl.test.ts`:
- Line 26: Remove the unnecessary cast by accessing the typed property directly:
replace instances of "const root = (data as any).abapGit;" with "const root =
data.abapGit;" (data is already typed via SchemaScenario<TablSchema>), and make
the same change for the other occurrence using the same symbols (data, abapGit,
TablSchema, SchemaScenario<TablSchema>, root).
In `@packages/adt-plugin-abapgit/xsd/tabl.xsd`:
- Around line 19-31: The current xs:redefine block must be removed and replaced
by composing a new TablValuesType that extends asx:AbapValuesType: add an
xs:import for asx.xsd, define a new complexType named TablValuesType with
xs:complexContent xs:extension base="asx:AbapValuesType" and include the
TABL-specific elements DD02V and DD03P_TABLE (types asx:Dd02vType and
asx:Dd03pTableType); replace any schema declarations that expected the redefined
AbapValuesType with references to TablValuesType and remove the xs:redefine
block entirely to avoid using xs:redefine/xs:override.
In `@packages/adt-plugin-abapgit/xsd/ttyp.xsd`:
- Around line 18-29: You used xs:redefine to alter AbapValuesType which violates
the guideline; instead remove the xs:redefine block and introduce a new composed
type (e.g., AbapValuesTypeWithDD40V) that references the original
asx:AbapValuesType and adds the DD40V element; specifically create a new
xs:complexType (name AbapValuesTypeWithDD40V) that contains a sequence with an
element/ref to asx:AbapValuesType (or embeds its content via composition) plus
an optional <DD40V type="asx:Dd40vType"/>, then update any places that expected
the redefined AbapValuesType to use the new AbapValuesTypeWithDD40V instead.
---
Nitpick comments:
In `@packages/adk/src/objects/ddic/doma/doma.model.ts`:
- Around line 43-45: The getter get crudContract currently returns the untyped
any; either replace any with the precise contract type exported by your ADT
client (the type of this.ctx.client.adt.ddic.domains) or, if no suitable type
exists, keep any but add a short explanatory comment above get crudContract
documenting why any is required and a TODO to fix once the contract typings are
added; update the signature from "protected override get crudContract(): any" to
use the correct interface name (or add the explanatory comment) so the intention
and limitation are explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e0bbdae-0898-47aa-ae6e-35e85c14b9b7
⛔ Files ignored due to path filters (18)
packages/adt-contracts/src/generated/schemas.tsis excluded by!**/generated/**packages/adt-plugin-abapgit/src/schemas/generated/index.tsis excluded by!**/generated/**packages/adt-plugin-abapgit/src/schemas/generated/schemas/index.tsis excluded by!**/generated/**packages/adt-plugin-abapgit/src/schemas/generated/schemas/tabl.tsis excluded by!**/generated/**packages/adt-plugin-abapgit/src/schemas/generated/schemas/ttyp.tsis excluded by!**/generated/**packages/adt-plugin-abapgit/src/schemas/generated/types/index.tsis excluded by!**/generated/**packages/adt-plugin-abapgit/src/schemas/generated/types/tabl.tsis excluded by!**/generated/**packages/adt-plugin-abapgit/src/schemas/generated/types/ttyp.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/schemas/sap/dataelements.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/schemas/sap/domain.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/schemas/sap/index.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/schemas/sap/tablesettings.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/schemas/sap/tabletype.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/typed.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/types/sap/dataelements.types.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/types/sap/domain.types.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/types/sap/tablesettings.types.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/types/sap/tabletype.types.tsis excluded by!**/generated/**
📒 Files selected for processing (39)
packages/adk/src/base/adt.tspackages/adk/src/base/kinds.tspackages/adk/src/index.tspackages/adk/src/objects/ddic/doma/doma.model.tspackages/adk/src/objects/ddic/doma/index.tspackages/adk/src/objects/ddic/dtel/dtel.model.tspackages/adk/src/objects/ddic/dtel/index.tspackages/adk/src/objects/ddic/index.tspackages/adk/src/objects/ddic/tabl/index.tspackages/adk/src/objects/ddic/tabl/tabl.model.tspackages/adk/src/objects/ddic/ttyp/index.tspackages/adk/src/objects/ddic/ttyp/ttyp.model.tspackages/adt-client/src/index.tspackages/adt-contracts/src/adt/ddic/dataelements.tspackages/adt-contracts/src/adt/ddic/domains.tspackages/adt-contracts/src/adt/ddic/index.tspackages/adt-contracts/src/adt/ddic/structures.tspackages/adt-contracts/src/adt/ddic/tables.tspackages/adt-contracts/src/adt/ddic/tabletypes.tspackages/adt-contracts/src/adt/index.tspackages/adt-plugin-abapgit/src/lib/handlers/adk.tspackages/adt-plugin-abapgit/src/lib/handlers/objects/doma.tspackages/adt-plugin-abapgit/src/lib/handlers/objects/dtel.tspackages/adt-plugin-abapgit/src/lib/handlers/objects/index.tspackages/adt-plugin-abapgit/src/lib/handlers/objects/tabl.tspackages/adt-plugin-abapgit/src/lib/handlers/objects/ttyp.tspackages/adt-plugin-abapgit/tests/fixtures/tabl/zage_structure.tabl.xmlpackages/adt-plugin-abapgit/tests/fixtures/tabl/zage_transparent_table.tabl.xmlpackages/adt-plugin-abapgit/tests/fixtures/ttyp/zage_string_table.ttyp.xmlpackages/adt-plugin-abapgit/tests/fixtures/ttyp/zage_struct_table_type.ttyp.xmlpackages/adt-plugin-abapgit/tests/schemas/tabl.test.tspackages/adt-plugin-abapgit/tests/schemas/ttyp.test.tspackages/adt-plugin-abapgit/ts-xsd.config.tspackages/adt-plugin-abapgit/xsd/tabl.xsdpackages/adt-plugin-abapgit/xsd/ttyp.xsdpackages/adt-plugin-abapgit/xsd/types/dd02v.xsdpackages/adt-plugin-abapgit/xsd/types/dd03p.xsdpackages/adt-plugin-abapgit/xsd/types/dd40v.xsdpackages/adt-schemas/ts-xsd.config.ts
Comment on lines
+46
to
+47
| protected override get crudContract(): any { | ||
| return this.ctx.client.adt.ddic.dataelements; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify explicit any usage in ADK DDIC models
rg -nP --type=ts 'crudContract\(\):\s*any' packages/adk/src/objects/ddicRepository: abapify/adt-cli
Length of output: 553
🏁 Script executed:
#!/bin/bash
# Inspect dtel.model.ts file to see imports and class structure
head -60 packages/adk/src/objects/ddic/dtel/dtel.model.ts | cat -nRepository: abapify/adt-cli
Length of output: 2333
🏁 Script executed:
#!/bin/bash
# Find the base class that defines crudContract
fd -e ts -p "packages/adk/src" | xargs rg -l "class.*Model.*\{" | head -5Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Search for the base class definition of DDIC models (likely in packages/adk/src/objects)
rg -A 5 "class DtelModel" packages/adk/src/objects/ddic/dtel/dtel.model.ts | head -20Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check what type ctx.client.adt.ddic.dataelements actually is
# by looking at client definition and types
find packages/adt-client/src -name "*contract*" | xargs rg -l "dataelements" | head -3Repository: abapify/adt-cli
Length of output: 203
🏁 Script executed:
#!/bin/bash
# Check base class AdkMainObject definition
rg -A 20 "class AdkMainObject" packages/adk/src/base/model.ts | head -40Repository: abapify/adt-cli
Length of output: 761
🏁 Script executed:
#!/bin/bash
# Look for crudContract definition in base class
rg -B 2 -A 5 "crudContract" packages/adk/src/base/model.ts | head -30Repository: abapify/adt-cli
Length of output: 901
🏁 Script executed:
#!/bin/bash
# Check AdkContext type definition to understand ctx.client structure
rg -A 30 "export.*type AdkContext" packages/adk/src/base/context.ts | head -50Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if other non-DDIC models use proper types for crudContract
rg "crudContract.*:" packages/adk/src/objects --type=ts | grep -v "any" | head -10Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Find context.ts and check AdkContext definition more carefully
find packages/adk/src -name "context.ts" -exec cat {} \;Repository: abapify/adt-cli
Length of output: 1069
🏁 Script executed:
#!/bin/bash
# Check the get signature in crudContract to understand expected type
rg -B 5 -A 10 "get.*crudContract" packages/adk/src/base/model.ts | head -50Repository: abapify/adt-cli
Length of output: 569
🏁 Script executed:
#!/bin/bash
# Look at the contract type - check what dataelements contract looks like
head -50 packages/adt-contracts/src/adt/ddic/dataelements.tsRepository: abapify/adt-cli
Length of output: 785
🏁 Script executed:
#!/bin/bash
# Check if any ADK models override crudContract with a specific type (not any)
rg "override get crudContract" packages/adk/src/objects -A 1 | head -40Repository: abapify/adt-cli
Length of output: 2059
🏁 Script executed:
#!/bin/bash
# Check what types are exported from dataelements contract
cat -n packages/adt-contracts/src/adt/ddic/dataelements.tsRepository: abapify/adt-cli
Length of output: 967
🏁 Script executed:
#!/bin/bash
# Check if contracts are imported in ADK or if there's a typing gap
rg "import.*dataelements" packages/adk/src/objects/ddic/dtel/ | head -10Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check the base class to see if the any has justification
cat -n packages/adk/src/base/model.ts | sed -n '200,250p'Repository: abapify/adt-cli
Length of output: 1742
🏁 Script executed:
#!/bin/bash
# Check if AdtClient defines the contract types properly
rg -A 20 "export.*AdtClient\|interface AdtClient" packages/adk/src/base/adt.ts | head -50Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if ADK models can import contract types from adt-contracts
rg "from.*adt-contracts" packages/adk/src/objects --type=ts | head -5Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check how the contracts are organized - see if DataelementsContract is the right type
rg "export.*Contract" packages/adt-contracts/src/adt/ddic/ --type=tsRepository: abapify/adt-cli
Length of output: 1170
🏁 Script executed:
#!/bin/bash
# Check the actual adt.ts or client builder to understand if contracts are properly typed
fd -e ts "adt\\.ts|client\\.ts" packages/adk/src/base/ | xargs cat | head -100Repository: abapify/adt-cli
Length of output: 3069
🏁 Script executed:
#!/bin/bash
# Check if there's already a pattern for importing contract types in the codebase
rg "DataelementsContract\|programsContract\|classesContract" packages/adk/src --type=ts | head -5Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if `@abapify/adt-client` exports the contract types
rg "export.*Contract" packages/adt-contracts/src/index.ts | head -20Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check the main adt-contracts/index.ts to see what's exported
cat packages/adt-contracts/src/index.ts | head -100Repository: abapify/adt-cli
Length of output: 932
🏁 Script executed:
#!/bin/bash
# Verify if ADK can import from adt-contracts or adt-client
grep -r "from.*@abapify" packages/adk/src --include="*.ts" | head -5Repository: abapify/adt-cli
Length of output: 401
🏁 Script executed:
#!/bin/bash
# Check if the suggested type signature actually works by testing the path
cat -n packages/adk/src/base/context.tsRepository: abapify/adt-cli
Length of output: 1335
🏁 Script executed:
#!/bin/bash
# Check if AdtClient type definition properly types the contract nested access
rg -A 30 "interface AdtClient\|type AdtClient" packages/adt-client/src --type=ts | head -60Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check how AdtClient.adt.ddic.dataelements is typed - is it strongly typed?
rg "ddic.*dataelements\|dataelements.*:" packages/adt-client/src --type=ts -B 2 -A 2 | head -30Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if the base class comment explains why any is used
rg -B 10 "protected get crudContract.*any" packages/adk/src/base/model.ts | head -20Repository: abapify/adt-cli
Length of output: 370
🏁 Script executed:
#!/bin/bash
# Check the actual AdtClient type structure to understand if the nested property type is accessible
find packages/adt-client/src -name "*.ts" | xargs rg "ddic.*dataelements\|export.*adt:" | head -20Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if adt-contracts properly exports DataelementsContract for import
cat packages/adt-contracts/src/adt/ddic/index.tsRepository: abapify/adt-cli
Length of output: 1361
🏁 Script executed:
#!/bin/bash
# Verify if any ADK model imports contract types from adt-contracts
rg "import.*Contract.*from" packages/adk/src --type=tsRepository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Verify that AdtClient typing doesn't provide inferred contract types for nested access
# Check if adt-client exports AdtClient with proper nested typing
cat packages/adt-client/src/index.tsRepository: abapify/adt-cli
Length of output: 2615
🏁 Script executed:
#!/bin/bash
# Check what package.json says about the adt-client version/exports
cat packages/adt-client/package.json | jq '.exports, .main'Repository: abapify/adt-cli
Length of output: 131
Add TypeScript strict-mode justification or type crudContract properly.
All ADK object models override crudContract with any but lack the // eslint-disable-next-line @typescript-eslint/no-explicit-any`` comment present in the base class. This violates strict-mode requirements.
Either add the disable comment with justification:
// eslint-disable-next-line `@typescript-eslint/no-explicit-any`
protected override get crudContract(): any {
return this.ctx.client.adt.ddic.dataelements;
}Or import and use the contract type directly:
import type { DataelementsContract } from '@abapify/adt-contracts';
protected override get crudContract(): DataelementsContract {
return this.ctx.client.adt.ddic.dataelements;
}This pattern affects 10+ models across the codebase (dtel, doma, tabl, ttyp, clas, intf, prog, devc, fugr, and others).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/adk/src/objects/ddic/dtel/dtel.model.ts` around lines 46 - 47, The
getter protected override get crudContract in dtel.model.ts uses an untyped any
which violates strict-mode; fix by either adding the eslint disable comment
above the getter (// eslint-disable-next-line
`@typescript-eslint/no-explicit-any`) with a short justification, or better import
the proper contract type (e.g., import type { DataelementsContract } from
'@abapify/adt-contracts') and change the signature to protected override get
crudContract(): DataelementsContract { return
this.ctx.client.adt.ddic.dataelements; } so the getter is correctly typed;
update the dtel.model.ts getter accordingly.
Member
Author
There was a problem hiding this comment.
Dismissed — pre-existing base class limitation. ALL ADK models (CLAS, INTF, PROG, FUGR, DEVC + DDIC) use crudContract(): any. The ADK intentionally does not import contract types from adt-contracts (dependency direction constraint). Not a regression in this PR.
There was a problem hiding this comment.
@ThePlenkov, understood — the crudContract(): any pattern is a deliberate architectural constraint across all ADK object models due to the unidirectional dependency (ADK → adt-client only, never → adt-contracts). The existing knowledge base entry already captures this broadly for packages/adk/src/objects/**/*.model.ts, so I won't flag this pattern in future reviews.
( ˘ω˘ ) 🐇
🧠 Learnings used
Learnt from: ThePlenkov
Repo: abapify/adt-cli PR: 84
File: packages/adk/src/objects/ddic/tabl/tabl.model.ts:46-47
Timestamp: 2026-03-16T12:24:05.651Z
Learning: In ADK object models (files matching packages/adk/src/objects/**/*.model.ts), the protected override get crudContract(): any pattern is intentional because ADK has a one-way dependency (ADK → adt-client, not ADK → adt-contracts). Therefore, returning any for crudContract() should not be flagged as a violation. This should apply broadly to all ADK object models, not just the single file.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.877Z
Learning: Applies to packages/adt-plugin-abapgit/src/lib/handlers/**/*.ts : Use ADK types from local re-export module `../adk` instead of directly from `abapify/adk` in handler files
Learnt from: ThePlenkov
Repo: abapify/adt-cli PR: 84
File: packages/adt-contracts/src/adt/ddic/structures.ts:0-0
Timestamp: 2026-03-16T12:24:08.647Z
Learning: In `packages/adt-contracts/src/adt/ddic/structures.ts`, the correct Content-Type for POST requests to the SAP ADT structures endpoint (`/sap/bc/adt/ddic/structures`) is `application/vnd.sap.adt.tablestructures.v2+xml`, which differs from the path-level MIME type `application/vnd.sap.adt.structures.v2+xml` stored in the `contentType` variable.
Learnt from: ThePlenkov
Repo: abapify/adt-cli PR: 84
File: packages/adt-contracts/src/adt/ddic/structures.ts:0-0
Timestamp: 2026-03-16T12:24:08.648Z
Learning: In `packages/adt-contracts/src/adt/ddic/structures.ts`, using a wildcard Accept header (e.g., `application/*`) in DDIC contract POST requests is intentional — it serves as a fallback for SAP ADT endpoints that may return responses in different versions.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.876Z
Learning: Applies to packages/adt-plugin-abapgit/xsd/**/*.xsd : Reuse SAP structures via `xs:complexType`, NOT via global elements
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.877Z
Learning: Applies to packages/adt-plugin-abapgit/src/lib/handlers/**/*.ts : Avoid manual XML string building; use schema `.build()` method instead
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.877Z
Learning: Applies to packages/adt-plugin-abapgit/src/lib/handlers/**/*.ts : Avoid `as any` type assertions; fix types at source in XSD schemas instead
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-client/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:07.245Z
Learning: Applies to packages/adt-client/src/**/*.ts : NEVER use `as any` type assertions without explicit justification. If type inference fails, fix the schema or contract instead of bypassing with casts.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-client/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:07.245Z
Learning: Applies to packages/adt-client/src/adt/**/*-contract.ts : Keep contracts thin with no business logic. Move business logic to services layer.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-client/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:07.245Z
Learning: Applies to packages/adt-client/src/adt/**/*-contract.ts : NEVER add a `metadata` field to contracts. The adapter automatically detects schemas from `responses[200]`. The `metadata: { responseSchema: ... }` field is legacy and should be removed.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T23:46:25.807Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript strict mode — no `any` without a comment explaining why
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-contracts/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:19.204Z
Learning: Applies to packages/adt-contracts/src/adt/**/*.ts : Always import schemas from '../../schemas' in contract definitions, not directly from 'abapify/adt-schemas'
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.877Z
Learning: Applies to packages/adt-plugin-abapgit/src/lib/handlers/objects/**/*.ts : Handler files must NOT contain file system operations, ADT client calls, XML string building, or Promise handling for sources
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-client/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:07.245Z
Learning: Applies to packages/adt-client/src/adt/**/*-contract.ts : The adapter automatically handles response parsing based on content-type (JSON, XML with schema, or text). Do not parse responses manually in contracts.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-cli/AGENTS.md:0-0
Timestamp: 2026-03-02T17:23:46.791Z
Learning: Applies to packages/adt-cli/src/lib/commands/**/*.ts : For new commands needing ADT API access, use V2 client (`abapify/adt-client`) when an endpoint has a contract in v2. Use V1 only when endpoint not yet migrated or v1-specific features are needed.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-client/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:07.245Z
Learning: Applies to packages/adt-client/src/contract.ts : The `fetch()` method is a utility function on the client for debugging and ad-hoc requests, NOT a contract endpoint. Only expose methods as contracts if they map to specific SAP ADT endpoints with known schemas.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-client/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:07.245Z
Learning: File structure for adt-client: organize contracts by SAP endpoint path under src/adt/, create corresponding schema files with *-schema.ts naming, register contracts in src/contract.ts, and create type inference tests in tests/.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T23:46:25.807Z
Learning: Applies to packages/adk/src/** : Object types in ADK must be registered with `handlers.set()` using an `AdkObjectHandler` bridge pattern with a parser function and URL factory
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-contracts/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:19.204Z
Learning: Add fixtures to adt-fixtures package when new contract scenarios require real SAP XML sample data
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.876Z
Learning: Applies to packages/adt-plugin-abapgit/xsd/*.xsd : Create concrete document schema in `xsd/{type}.xsd` with ONE root element (`abapGit`)
Comment on lines
+82
to
+84
| registerObjectType('TABL', TableKind, AdkTable); | ||
| // Note: Structure uses same main type TABL but different ADK kind | ||
| // Registration uses TABL main type - structures will be resolved via subtype logic |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect TABL registrations and structure-kind routing hints
rg -nP --type=ts "registerObjectType\\('TABL'" packages/adk/src
rg -nP --type=ts '\bStructure\b|\bStructureKind\b|TABL/DS|subtype' packages/adk/src/base packages/adk/src/objects
fd registry.ts packages/adk/src -x sed -n '1,300p' {}Repository: abapify/adt-cli
Length of output: 7198
🏁 Script executed:
# Search for subtype-based resolution logic
rg -nP --type=ts "TABL.*DS|subtype.*resolve|Kind.*subtype" packages/adk/src
# Search for AdkStructure usage and registration
rg -nP --type=ts "AdkStructure" packages/adk/src
# Search for factory or builder patterns that might do subtype routing
rg -nP --type=ts "switch|case.*DS|if.*subtype" packages/adk/src/baseRepository: abapify/adt-cli
Length of output: 1315
🏁 Script executed:
# View the context around kinds.ts:105 where AdkStructure is referenced
sed -n '95,115p' packages/adk/src/base/kinds.ts
# Search for how TABL/DS is actually instantiated or resolved at runtime
rg -nP --type=ts "new AdkStructure|AdkStructure\(" packages/adk/src
# Check if there's a factory or resolver that checks subtypes
rg -nP --type=ts "resolveType|resolveKind" packages/adk/src -A 3Repository: abapify/adt-cli
Length of output: 2644
AdkStructure is not reachable via registry resolution for TABL/DS — subtype routing logic is missing.
The resolveType() function extracts only the main type (TABL), returning the AdkTable constructor regardless of subtype. Code comments claim "structures will be resolved via subtype logic," but this logic does not exist in the registry or factory. Either register TABL/DS explicitly alongside the main TABL registration, or implement subtype-aware resolution in resolveType().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/adk/src/objects/ddic/tabl/tabl.model.ts` around lines 82 - 84, The
registry currently registers 'TABL' -> AdkTable via registerObjectType('TABL',
TableKind, AdkTable) but resolveType() only extracts the main type and always
returns AdkTable, so AdkStructure (for 'TABL/DS') is unreachable; fix by either
(A) registering the structure subtype explicitly (registerObjectType('TABL/DS',
TableKind, AdkStructure)) so the factory can return AdkStructure, or (B) make
resolveType() subtype-aware: parse the full type token (e.g. split on '/')
inside resolveType(), check for known subtypes like 'DS' and map them to
AdkStructure (while falling back to AdkTable for plain 'TABL'), and update
registry lookup logic to prefer exact subtype keys before main-type keys.
Member
Author
There was a problem hiding this comment.
Acknowledged for follow-up — same as the Copilot comment about TABL/DS vs TABL/DT. Registry needs subtype-awareness. The AdkStructure class is already defined and working, just not yet registered by subtype. Will address in a follow-up PR.
There was a problem hiding this comment.
@ThePlenkov, understood — thanks for the context. Since this is deferred, would you like me to open a GitHub issue to track the registry subtype-awareness work for TABL/DS / TABL/DT? That way the gap between AdkStructure being defined and it being resolvable via the registry doesn't get lost.
🧠 Learnings used
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-client/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:07.245Z
Learning: File structure for adt-client: organize contracts by SAP endpoint path under src/adt/, create corresponding schema files with *-schema.ts naming, register contracts in src/contract.ts, and create type inference tests in tests/.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T23:46:25.807Z
Learning: Applies to packages/adk/src/** : Object types in ADK must be registered with `handlers.set()` using an `AdkObjectHandler` bridge pattern with a parser function and URL factory
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.877Z
Learning: Applies to packages/adt-plugin-abapgit/src/lib/handlers/**/*.ts : Use ADK types from local re-export module `../adk` instead of directly from `abapify/adk` in handler files
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.876Z
Learning: Applies to packages/adt-plugin-abapgit/xsd/**/*.xsd : Reuse SAP structures via `xs:complexType`, NOT via global elements
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-client/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:07.245Z
Learning: Applies to packages/adt-client/src/adt/**/*-schema.ts : For XML Schemas, ALWAYS use the `createSchema()` helper to enable speci type inference with the `_infer` property
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T23:46:25.807Z
Learning: Applies to packages/adt-schemas/** : Schema literals should be exported as TypeScript exports from `adt-schemas` package, derived from XSD via ts-xsd parsing
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-contracts/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:19.204Z
Learning: Applies to packages/adt-contracts/tests/contracts/**/*.ts : Use fixtures from 'adt-fixtures' for testing real SAP XML samples in contract operations
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.876Z
Learning: Applies to packages/adt-plugin-abapgit/src/lib/handlers/objects/**/*.ts : Handler `toAbapGit()` must map to data structure with return type inferred from schema._values
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.877Z
Learning: Applies to packages/adt-plugin-abapgit/src/lib/handlers/**/*.ts : Avoid `as any` type assertions; fix types at source in XSD schemas instead
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-client/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:07.245Z
Learning: Applies to packages/adt-client/src/**/*.ts : NEVER use `as any` type assertions without explicit justification. If type inference fails, fix the schema or contract instead of bypassing with casts.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-contracts/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:19.204Z
Learning: Add fixtures to adt-fixtures package when new contract scenarios require real SAP XML sample data
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.877Z
Learning: Applies to packages/adt-plugin-abapgit/src/lib/handlers/**/*.ts : Avoid manual XML string building; use schema `.build()` method instead
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-cli/AGENTS.md:0-0
Timestamp: 2026-03-02T17:23:46.791Z
Learning: Applies to packages/adt-cli/src/lib/commands/**/*.ts : When migrating commands from v1 to v2, check if v2 contract exists, update imports to use `getAdtClientV2()`, replace client initialization, update API calls to v2 endpoints, test the command, and update AGENTS.md with migration status.
Learnt from: CR
Repo: abapify/adt-cli PR: 0
File: packages/adt-plugin-abapgit/AGENTS.md:0-0
Timestamp: 2026-03-02T17:24:40.876Z
Learning: Applies to packages/adt-plugin-abapgit/xsd/*.xsd : Create concrete document schema in `xsd/{type}.xsd` with ONE root element (`abapGit`)
Comment on lines
+46
to
+47
| protected override get crudContract(): any { | ||
| return this.ctx.client.adt.ddic.tabletypes; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify explicit any usage in ADK DDIC models
rg -nP --type=ts 'crudContract\(\):\s*any' packages/adk/src/objects/ddicRepository: abapify/adt-cli
Length of output: 553
🏁 Script executed:
#!/bin/bash
# Find the parent class of ttyp.model.ts
head -30 packages/adk/src/objects/ddic/ttyp/ttyp.model.tsRepository: abapify/adt-cli
Length of output: 1029
🏁 Script executed:
#!/bin/bash
# Search for crudContract definition in base classes
find packages/adk/src -name "*.ts" -type f | xargs grep -l "crudContract" | head -10Repository: abapify/adt-cli
Length of output: 557
🏁 Script executed:
#!/bin/bash
# Look for AdkContext type definition to understand the chain
rg -nA 5 "interface AdkContext|type AdkContext" packages/adk/srcRepository: abapify/adt-cli
Length of output: 402
🏁 Script executed:
#!/bin/bash
# Check if there are any inline comments explaining the any type
rg -B 2 "crudContract\(\):\s*any" packages/adk/src/objects/ddicRepository: abapify/adt-cli
Length of output: 1175
🏁 Script executed:
#!/bin/bash
# Find crudContract getter definition in base class
rg -A 3 "protected.*crudContract|abstract.*crudContract" packages/adk/src/base/model.tsRepository: abapify/adt-cli
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Check the full AdkContext and client structure
cat packages/adk/src/base/context.tsRepository: abapify/adt-cli
Length of output: 1069
🏁 Script executed:
#!/bin/bash
# Check what ctx.client.adt.ddic actually returns
rg -nA 5 "ddic" packages/adk/src/base/context.tsRepository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Find AdtClient type definition
rg -n "export.*interface AdtClient|export.*type AdtClient" packages/adk/src/base/adt.tsRepository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Get the full AdtClient structure
head -100 packages/adk/src/base/adt.tsRepository: abapify/adt-cli
Length of output: 3069
🏁 Script executed:
#!/bin/bash
# Look for where ddic.tabletypes is defined
find packages/adk/src -type f -name "*.ts" | xargs grep -l "ddic" | xargs grep -l "tabletypes"Repository: abapify/adt-cli
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Check what AdtClient exports from adt-client package
grep -n "ddic\|tabletypes" node_modules/@abapify/adt-client/dist/*/index.d.ts 2>/dev/null | head -20Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Look at imports in ttyp.model.ts to see what types are available
head -50 packages/adk/src/objects/ddic/ttyp/ttyp.model.tsRepository: abapify/adt-cli
Length of output: 1569
🏁 Script executed:
#!/bin/bash
# Check the base model to see how crudContract is used
grep -n "crudContract" packages/adk/src/base/model.ts | head -20Repository: abapify/adt-cli
Length of output: 864
🏁 Script executed:
#!/bin/bash
# See full context of crudContract in base/model.ts
rg -B 5 -A 10 "protected get crudContract" packages/adk/src/base/model.tsRepository: abapify/adt-cli
Length of output: 569
🏁 Script executed:
#!/bin/bash
# Check other DDIC models to see how they override crudContract
cat packages/adk/src/objects/ddic/tabl/tabl.model.ts | grep -A 5 "crudContract"Repository: abapify/adt-cli
Length of output: 499
🏁 Script executed:
#!/bin/bash
# Check if there are other object types with typed crudContract (non-any)
rg -A 1 "protected override get crudContract\(\):" packages/adk/src/objects --type=ts | head -40Repository: abapify/adt-cli
Length of output: 2059
🏁 Script executed:
#!/bin/bash
# Look at adt-client to understand what type tabletypes contract is
find packages -name "*.ts" -type f | xargs grep -l "\.tabletypes" | head -5Repository: abapify/adt-cli
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Check the adt-client package index to see if ddic is exposed
cat packages/adt-client/src/index.ts | grep -i ddicRepository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check adt-client package structure
ls -la packages/adt-client/srcRepository: abapify/adt-cli
Length of output: 644
🏁 Script executed:
#!/bin/bash
# Look at adt-client main index
head -80 packages/adt-client/src/index.tsRepository: abapify/adt-cli
Length of output: 2412
🏁 Script executed:
#!/bin/bash
# Find where ddic contracts are defined in adt-client
find packages/adt-client/src -name "*ddic*" -o -name "*tabl*" | head -10Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if there are type definitions in adt-client for the contracts
rg -n "class|interface" packages/adt-client/src/index.ts | head -20Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check the base class comment more carefully
sed -n '535,550p' packages/adk/src/base/model.tsRepository: abapify/adt-cli
Length of output: 518
🏁 Script executed:
#!/bin/bash
# Look for CrudContract usage in the codebase
rg "CrudContract" packages/adk/src --type=ts -A 2 | head -30Repository: abapify/adt-cli
Length of output: 244
🏁 Script executed:
#!/bin/bash
# Check if AdtClient type definition is accessible and what it looks like
rg -n "AdtClient" packages/adt-client/src/client.tsRepository: abapify/adt-cli
Length of output: 498
🏁 Script executed:
#!/bin/bash
# Look at the client.ts file to understand the structure
cat packages/adt-client/src/client.tsRepository: abapify/adt-cli
Length of output: 3644
🏁 Script executed:
#!/bin/bash
# Check if CrudContract is the right base type for all these contracts
rg -B 3 "CrudContract" packages/adt-client/src/index.tsRepository: abapify/adt-cli
Length of output: 250
🏁 Script executed:
#!/bin/bash
# Check what type tabletypes, tables, etc. actually are
# Look for contract definitions in adt-contracts
find packages -name "*.d.ts" 2>/dev/null | xargs grep -l "tabletypes\|ddic" | head -5Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Alternative: check if there's a contracts package
ls -la packages/ | grep contractRepository: abapify/adt-cli
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Look at the actual return value - is it ever used in a way that requires the concrete type?
rg -A 5 "this\.crudContract" packages/adk/src/base/model.ts | head -40Repository: abapify/adt-cli
Length of output: 850
🏁 Script executed:
#!/bin/bash
# Check if the base class comment justifies the any usage for subclasses
sed -n '535,550p' packages/adk/src/base/model.tsRepository: abapify/adt-cli
Length of output: 518
🏁 Script executed:
#!/bin/bash
# Look for adt-contracts to find the actual contract types
ls -la packages/adt-contracts/src/Repository: abapify/adt-cli
Length of output: 522
🏁 Script executed:
#!/bin/bash
# Check what ddic contract types are available in adt-contracts
find packages/adt-contracts/src -name "*ddic*" -o -name "*tabl*" -o -name "*ttyp*" | head -10Repository: abapify/adt-cli
Length of output: 190
🏁 Script executed:
#!/bin/bash
# Look at adt-contracts index
head -150 packages/adt-contracts/src/index.ts | tail -100Repository: abapify/adt-cli
Length of output: 932
🏁 Script executed:
#!/bin/bash
# Check the actual tabletypes contract definition
cat packages/adt-contracts/src/adt/ddic/tabletypes.tsRepository: abapify/adt-cli
Length of output: 751
🏁 Script executed:
#!/bin/bash
# Check if there's a CrudContract type that wraps these
rg -n "export.*CrudContract|interface.*CrudContract" packages/adt-contracts/src --type=tsRepository: abapify/adt-cli
Length of output: 247
🏁 Script executed:
#!/bin/bash
# Look at base.ts to understand CrudContract
cat packages/adt-contracts/src/base.tsRepository: abapify/adt-cli
Length of output: 2739
🏁 Script executed:
#!/bin/bash
# Check if TabletypesContract is exported and accessible in ADK
grep -r "TabletypesContract\|TableTypeResponse" packages/adk/src --type=tsRepository: abapify/adt-cli
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Check the pattern - is there any override with a concrete type (not any)?
rg -A 1 "protected override get crudContract" packages/adk/src/objects --type=ts -NRepository: abapify/adt-cli
Length of output: 2059
🏁 Script executed:
#!/bin/bash
# Verify what the actual return type should be - check adt-contracts exports
rg "export.*Contract" packages/adt-contracts/src/adt/ddic/tabletypes.tsRepository: abapify/adt-cli
Length of output: 159
🏁 Script executed:
#!/bin/bash
# Check if the suggested type annotation syntax is even valid by looking for similar patterns
rg "AdkContext\[" packages/adk/src --type=tsRepository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if there's a way to properly type crudContract using typeof
cat packages/adt-contracts/src/adt/ddic/tabletypes.ts | grep -A 5 "export"Repository: abapify/adt-cli
Length of output: 437
🏁 Script executed:
#!/bin/bash
# Check how other files in ADK import contract types
rg "import.*Contract" packages/adk/src/objects --type=ts | head -10Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Look at the base class eslint comment more carefully
sed -n '540,548p' packages/adk/src/base/model.tsRepository: abapify/adt-cli
Length of output: 251
🏁 Script executed:
#!/bin/bash
# Check what contract types are exported from adt-contracts
rg "export type.*Contract" packages/adt-contracts/src/adt/ddic --type=tsRepository: abapify/adt-cli
Length of output: 385
🏁 Script executed:
#!/bin/bash
# Check if these contract types can be imported into ADK
rg "from '@abapify/adt-contracts" packages/adk/src --type=ts | head -10Repository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Verify the base/adt.ts reexports to understand what's available to import
cat packages/adk/src/base/adt.ts | grep -A 50 "Re-export types from adt-client"Repository: abapify/adt-cli
Length of output: 1674
🏁 Script executed:
#!/bin/bash
# Check if there's any pattern of using typeof with imported contracts
rg "typeof.*Contract" packages/adk/src --type=tsRepository: abapify/adt-cli
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check if contract types from adt-contracts are reexported anywhere accessible to ADK
find packages/adk -name "*.ts" -type f | xargs grep "TabletypesContract\|DomainsContract" 2>/dev/nullRepository: abapify/adt-cli
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check what adt-contracts actually exports publicly
cat packages/adt-contracts/src/adt/index.tsRepository: abapify/adt-cli
Length of output: 2138
🏁 Script executed:
#!/bin/bash
# Verify the eslint-disable comment in base class applies to overrides
cat -n packages/adk/src/base/model.ts | sed -n '538,550p'Repository: abapify/adt-cli
Length of output: 562
🏁 Script executed:
#!/bin/bash
# Check the TypeScript compiler output or tsconfig to see how strict mode is configured
cat packages/adk/tsconfig.json | grep -A 20 "compilerOptions"Repository: abapify/adt-cli
Length of output: 406
🏁 Script executed:
#!/bin/bash
# Check the full context of the base class crudContract with surrounding comments
sed -n '530,550p' packages/adk/src/base/model.tsRepository: abapify/adt-cli
Length of output: 650
🏁 Script executed:
#!/bin/bash
# Check if the eslint-disable comment in base class applies to overrides or just that one getter
grep -B 10 "eslint-disable.*any" packages/adk/src/base/model.ts | head -20Repository: abapify/adt-cli
Length of output: 734
🏁 Script executed:
#!/bin/bash
# Check if there are any comments in the subclass overrides explaining the any
sed -n '44,48p' packages/adk/src/objects/ddic/ttyp/ttyp.model.tsRepository: abapify/adt-cli
Length of output: 185
🏁 Script executed:
#!/bin/bash
# Verify that TabletypesContract type is not accessible in ADK (not in base/adt.ts exports)
cat packages/adk/src/base/adt.ts | grep -i "Contract"Repository: abapify/adt-cli
Length of output: 1437
🏁 Script executed:
#!/bin/bash
# Check if TabletypesContract and other Contract types are exported from adt-contracts/index
cat packages/adt-contracts/src/index.tsRepository: abapify/adt-cli
Length of output: 932
🏁 Script executed:
#!/bin/bash
# Final check - see if the crudContract in subclasses needs the comment too
cat -n packages/adk/src/objects/ddic/ttyp/ttyp.model.ts | sed -n '42,50p'Repository: abapify/adt-cli
Length of output: 366
Add eslint-disable comment to crudContract override, or properly type the getter.
The override uses any without explanation, violating strict mode. All 10 overrides across DDIC and repository objects have the same issue.
The suggested type AdkContext['client']['adt']['ddic']['tabletypes'] won't compile. Either add a comment justifying the any:
protected override get crudContract(): any {
+ // Contract type not yet exported from base/adt.ts
return this.ctx.client.adt.ddic.tabletypes;
}Or export TabletypesContract and similar types from packages/adk/src/base/adt.ts and import them in the models. This same pattern applies to all 10 overrides: ttyp, tabl (2x), dtel, doma, prog, intf, fugr, devc, clas.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/adk/src/objects/ddic/ttyp/ttyp.model.ts` around lines 46 - 47, The
getter override crudContract currently returns any (protected override get
crudContract(): any) which breaks strict linting; fix by either (A) adding a
one-line eslint disable with a brief justification above the getter (e.g.,
eslint-disable-next-line `@typescript-eslint/no-explicit-any` // wrapper for ADT
client contract) or (B) properly typing it: export the concrete
TabletypesContract type from packages/adk/src/base/adt.ts, import that type into
this model, and change the getter signature to use that type and return
this.ctx.client.adt.ddic.tabletypes; apply the same pattern for the other
DDIC/repository overrides (tabl, dtel, doma, prog, intf, fugr, devc, clas).
Member
Author
There was a problem hiding this comment.
Same as dtel/tabl — crudContract(): any is the established pattern. Not a regression.
| { | ||
| path: 'tabl/zage_structure.tabl.xml', | ||
| validate: (data) => { | ||
| const root = (data as any).abapGit; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for untyped any-casts in TABL tests and inspect scenario typing contract
rg -n "as any" packages/adt-plugin-abapgit/tests/schemas/tabl.test.ts
rg -n -C3 "type SchemaScenario|interface SchemaScenario|validate" packages/adt-plugin-abapgit/tests/schemas/base/scenario.tsRepository: abapify/adt-cli
Length of output: 1532
🏁 Script executed:
head -80 packages/adt-plugin-abapgit/tests/schemas/tabl.test.tsRepository: abapify/adt-cli
Length of output: 2796
🏁 Script executed:
# Find schema type definitions for TABL
rg -n "TablSchema|export.*Tabl" packages/adt-plugin-abapgit/tests/schemas/tabl.test.ts
rg -n "as TablSchema" packages/adt-plugin-abapgit/tests/schemas/Repository: abapify/adt-cli
Length of output: 249
🏁 Script executed:
head -50 packages/adt-plugin-abapgit/src/schemas/generated/types/tabl.tsRepository: abapify/adt-cli
Length of output: 1535
🏁 Script executed:
# Search for abapGit property in TablSchema
rg -n "abapGit" packages/adt-plugin-abapgit/src/schemas/generated/types/tabl.ts | head -5Repository: abapify/adt-cli
Length of output: 78
Remove unnecessary as any cast in TABL schema tests.
The data parameter is already properly typed as TablSchema from the generic SchemaScenario<TablSchema> instantiation. The cast serves no purpose and violates strict mode typing requirements.
♻️ Proposed fix
- const root = (data as any).abapGit;
+ const root = data.abapGit;Also applies to: 62-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/adt-plugin-abapgit/tests/schemas/tabl.test.ts` at line 26, Remove
the unnecessary cast by accessing the typed property directly: replace instances
of "const root = (data as any).abapGit;" with "const root = data.abapGit;" (data
is already typed via SchemaScenario<TablSchema>), and make the same change for
the other occurrence using the same symbols (data, abapGit, TablSchema,
SchemaScenario<TablSchema>, root).
Comment on lines
+19
to
+31
| <!-- Redefine AbapValuesType to include TABL-specific elements --> | ||
| <xs:redefine schemaLocation="asx.xsd"> | ||
| <xs:complexType name="AbapValuesType"> | ||
| <xs:complexContent> | ||
| <xs:extension base="asx:AbapValuesType"> | ||
| <xs:sequence> | ||
| <xs:element name="DD02V" type="asx:Dd02vType" minOccurs="0"/> | ||
| <xs:element name="DD03P_TABLE" type="asx:Dd03pTableType" minOccurs="0"/> | ||
| </xs:sequence> | ||
| </xs:extension> | ||
| </xs:complexContent> | ||
| </xs:complexType> | ||
| </xs:redefine> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for how other document schemas extend or use AbapValuesType
rg -n "AbapValuesType" --type=xml packages/adt-plugin-abapgit/xsd/
# Check if other schemas avoid xs:redefine
rg -n "xs:redefine" --type=xml packages/adt-plugin-abapgit/xsd/Repository: abapify/adt-cli
Length of output: 4942
Remove xs:redefine and use composition instead.
The schema violates the coding guideline: "Do NOT use xs:redefine or xs:override in XSD schemas." This pattern appears across all document schemas (doma.xsd, ttyp.xsd, intf.xsd, dtel.xsd, devc.xsd, clas.xsd).
Refactor by:
- Creating a new
TablValuesTypethat extendsasx:AbapValuesTypewith TABL-specific elements - Replacing the
xs:redefineblock with anxs:importofasx.xsdand defining the new type directly
This avoids redefinition and aligns with XSD best practices.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/adt-plugin-abapgit/xsd/tabl.xsd` around lines 19 - 31, The current
xs:redefine block must be removed and replaced by composing a new TablValuesType
that extends asx:AbapValuesType: add an xs:import for asx.xsd, define a new
complexType named TablValuesType with xs:complexContent xs:extension
base="asx:AbapValuesType" and include the TABL-specific elements DD02V and
DD03P_TABLE (types asx:Dd02vType and asx:Dd03pTableType); replace any schema
declarations that expected the redefined AbapValuesType with references to
TablValuesType and remove the xs:redefine block entirely to avoid using
xs:redefine/xs:override.
Comment on lines
+18
to
+29
| <!-- Redefine AbapValuesType to include TTYP-specific elements --> | ||
| <xs:redefine schemaLocation="asx.xsd"> | ||
| <xs:complexType name="AbapValuesType"> | ||
| <xs:complexContent> | ||
| <xs:extension base="asx:AbapValuesType"> | ||
| <xs:sequence> | ||
| <xs:element name="DD40V" type="asx:Dd40vType" minOccurs="0"/> | ||
| </xs:sequence> | ||
| </xs:extension> | ||
| </xs:complexContent> | ||
| </xs:complexType> | ||
| </xs:redefine> |
There was a problem hiding this comment.
Guideline violation: xs:redefine is explicitly forbidden.
The coding guidelines state: "Do NOT use xs:redefine or xs:override in XSD schemas". The comment on line 8 acknowledges using xs:redefine, which directly violates this rule.
Consider an alternative approach such as:
- Defining a new standalone type that composes the base elements plus DD40V
- Using a separate schema structure that avoids redefining the base type
🔧 Alternative approach sketch
Instead of redefining AbapValuesType, create a composed type:
- <!-- Redefine AbapValuesType to include TTYP-specific elements -->
- <xs:redefine schemaLocation="asx.xsd">
- <xs:complexType name="AbapValuesType">
- <xs:complexContent>
- <xs:extension base="asx:AbapValuesType">
- <xs:sequence>
- <xs:element name="DD40V" type="asx:Dd40vType" minOccurs="0"/>
- </xs:sequence>
- </xs:extension>
- </xs:complexContent>
- </xs:complexType>
- </xs:redefine>
+ <!-- Include base schema -->
+ <xs:include schemaLocation="asx.xsd"/>
+
+ <!-- TTYP-specific values type extending base with DD40V -->
+ <xs:complexType name="TtypValuesType">
+ <xs:all>
+ <xs:element name="DD40V" type="asx:Dd40vType" minOccurs="0"/>
+ </xs:all>
+ </xs:complexType>As per coding guidelines: "Do NOT use xs:redefine or xs:override in XSD schemas".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/adt-plugin-abapgit/xsd/ttyp.xsd` around lines 18 - 29, You used
xs:redefine to alter AbapValuesType which violates the guideline; instead remove
the xs:redefine block and introduce a new composed type (e.g.,
AbapValuesTypeWithDD40V) that references the original asx:AbapValuesType and
adds the DD40V element; specifically create a new xs:complexType (name
AbapValuesTypeWithDD40V) that contains a sequence with an element/ref to
asx:AbapValuesType (or embeds its content via composition) plus an optional
<DD40V type="asx:Dd40vType"/>, then update any places that expected the
redefined AbapValuesType to use the new AbapValuesTypeWithDD40V instead.
- Handle HTTP 405 'Method Not Allowed' as upsert fallback trigger (DDIC endpoints like TABL/TTYP return 405 instead of 404) - Handle HTTP 422 'already exists' after create fallback by marking object as unchanged instead of failing the deploy - Apply isAlreadyExistsError check in both lock catch and saveViaContract catch paths for complete coverage - Make unlockAll() resilient to individual unlock failures - Fix crud() POST Content-Type: use specific content type instead of generic 'application/*' - Update contract tests to match new POST Content-Type behavior
Add optional 'accept' field to CrudOptions for multi-version Accept headers on GET/POST/PUT requests, following SAP ADT best practices: - GET Accept: multi-version, newest first (server picks best match) - POST/PUT Accept: same multi-version list - POST/PUT Content-Type: single specific version (payload format) Updated all 10 crud() call sites with proper version fallback lists based on fr0ster/mcp-abap-adt-clients reference implementation. The 'accept' field is optional and falls back to 'contentType' for backward compatibility.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/adt-contracts/src/adt/ddic/tables.ts (1)
33-43:⚠️ Potential issue | 🟠 MajorURL-encode DDIC table names in all path builders.
Line 41 (and CRUD-generated paths) lowercases names but does not URL-encode them. Namespaced object names can produce invalid ADT URLs.
🔧 Proposed fix
export const tablesContract = { ...crud({ basePath, schema: blueSourceSchema, contentType, accept, + nameTransform: (n: string) => encodeURIComponent(n.toLowerCase()), }), objectstructure: (name: string, options?: { version?: string }) => - http.get(`${basePath}/${name.toLowerCase()}/objectstructure`, { + http.get( + `${basePath}/${encodeURIComponent(name.toLowerCase())}/objectstructure`, + { responses: { 200: undefined }, query: options?.version ? { version: options.version } : undefined, - }), + } + ), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adt-contracts/src/adt/ddic/tables.ts` around lines 33 - 43, The path builders currently use name.toLowerCase() (e.g., in objectstructure) and CRUD-generated paths without URL-encoding, which can break for namespaced DDIC table names; update all places that build paths from a table/object name (for example the objectstructure function and the CRUD helpers that use basePath and name) to apply encodeURIComponent to the lowercased name (e.g., encodeURIComponent(name.toLowerCase())) before interpolating into the URL so all path segments are safely URL-encoded.packages/adk/src/objects/ddic/tabl/tabl.model.ts (1)
46-47:⚠️ Potential issue | 🟠 MajorReplace
crudContract(): anywith precise indexed client types.Both getters erase contract types and bypass strict typing.
🔧 Proposed fix
- protected override get crudContract(): any { + protected override get crudContract(): AdkContext['client']['adt']['ddic']['tables'] { return this.ctx.client.adt.ddic.tables; } @@ - protected override get crudContract(): any { + protected override get crudContract(): AdkContext['client']['adt']['ddic']['structures'] { return this.ctx.client.adt.ddic.structures; }#!/bin/bash # Verify no explicit `crudContract(): any` remains in DDIC object models rg -nP --type=ts 'protected\s+override\s+get\s+crudContract\(\)\s*:\s*any' packages/adk/src/objects/ddicAs per coding guidelines "Use TypeScript strict mode — no
anywithout a comment explaining why".Also applies to: 73-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adk/src/objects/ddic/tabl/tabl.model.ts` around lines 46 - 47, The getter crudContract currently returns type any; change its signature to a precise indexed client type instead of any by using the actual client member type—for example declare the getter as returning typeof this.ctx.client.adt.ddic.tables (or the exact interface/type exported for the ADT tables client) and keep the implementation return this.ctx.client.adt.ddic.tables; update the same pattern for the other DDIC object model getters referenced (e.g., the occurrences around the later lines 73-74) so none of them use : any and they all use the concrete typeof this.ctx.client.adt.ddic.<...> client types.packages/adt-contracts/src/adt/ddic/tabletypes.ts (1)
6-6:⚠️ Potential issue | 🟡 MinorFix object-type label in module docs (
TTYP/DA→TTYP/TT).Line 6 is inconsistent with the table type object-kind naming used in this PR.
📝 Proposed fix
- * Object type: TTYP/DA (ttypda) + * Object type: TTYP/TT (ttyptt)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adt-contracts/src/adt/ddic/tabletypes.ts` at line 6, Update the module documentation comment that currently reads "Object type: TTYP/DA (ttypda)" to the correct table-type label "TTYP/TT" (replace the full string so it reads "Object type: TTYP/TT"); locate this in the header comment of the tabletypes.ts module where the object-type label is declared and modify the label text accordingly so the docs match the table type naming used elsewhere in this PR.
🧹 Nitpick comments (2)
packages/adt-contracts/src/helpers/crud.ts (1)
527-534: Verify backward compatibility of therepo()helper signature change.The new
acceptparameter was added as the 5th positional argument. Existing callers passing only 4 arguments remain compatible since it's optional, but this positional API could become unwieldy if more options are needed.Consider whether future callers would benefit from an options object pattern instead. However, given the current usage is limited, this is acceptable for now.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adt-contracts/src/helpers/crud.ts` around lines 527 - 534, The repo() helper added a new positional accept parameter which is optional but could break ergonomics long-term; to preserve backward compatibility while allowing future options, change repo() to accept a single options object as the last argument (e.g., repo(basePath, schema, contentType, nameTransform?, options?)) or add an overloaded signature that accepts either the old positional args or a final options object, then adapt the call to crud(...) to unpack options.accept and nameTransform appropriately; update the function declaration for repo and its call-site to support both forms and ensure crud(...) keeps accepting accept via the options you pass through.packages/adk/src/base/model.ts (1)
478-493: Deduplicate the upsert→create fallback block.The same fallback/createErr handling is repeated in two catch paths. Extracting it into one helper will prevent drift and make future fixes safer.
Also applies to: 525-539
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adk/src/base/model.ts` around lines 478 - 493, The upsert→create fallback and its createErr handling is duplicated; extract that logic into a single private helper (e.g., fallbackToCreateOrHandleError or handleUpsertCreateFallback) inside the same class and have both catch paths call it; the helper should call this.save({ ...options, mode: 'create' }), catch createErr, set this._unchanged = true and return this when this.isAlreadyExistsError(createErr), or rethrow otherwise, and replace the duplicated blocks that currently reference shouldFallbackToCreate, save, isAlreadyExistsError, and _unchanged with a single call to that helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/adk/src/base/model.ts`:
- Around line 478-493: The issue: _unchanged is set to true in multiple save()
branches but not reset, causing stale state on subsequent save() calls; to fix,
reset this._unchanged = false at the start of the save method so each invocation
starts fresh. Update the save() function (referencing the save method in
model.ts and the _unchanged property) to clear _unchanged before any
early-return checks or branch logic (affects the upsert fallback block where
isAlreadyExistsError sets _unchanged and the similar branch around lines
525-544); ensure no other code relies on persistent _unchanged across separate
save() calls.
- Around line 633-644: In isAlreadyExistsError, make the matching
case-insensitive and robust by normalizing the error text before checking; e.g.,
obtain a safe string from the error (use e instanceof Error ? e.message :
String(e)), call .toLowerCase() on it, and then check .includes('422') ||
.includes('unprocessable') && .includes('already exists') (all lowercased) so
the function (isAlreadyExistsError) correctly detects variations in casing and
non-Error inputs.
---
Duplicate comments:
In `@packages/adk/src/objects/ddic/tabl/tabl.model.ts`:
- Around line 46-47: The getter crudContract currently returns type any; change
its signature to a precise indexed client type instead of any by using the
actual client member type—for example declare the getter as returning typeof
this.ctx.client.adt.ddic.tables (or the exact interface/type exported for the
ADT tables client) and keep the implementation return
this.ctx.client.adt.ddic.tables; update the same pattern for the other DDIC
object model getters referenced (e.g., the occurrences around the later lines
73-74) so none of them use : any and they all use the concrete typeof
this.ctx.client.adt.ddic.<...> client types.
In `@packages/adt-contracts/src/adt/ddic/tables.ts`:
- Around line 33-43: The path builders currently use name.toLowerCase() (e.g.,
in objectstructure) and CRUD-generated paths without URL-encoding, which can
break for namespaced DDIC table names; update all places that build paths from a
table/object name (for example the objectstructure function and the CRUD helpers
that use basePath and name) to apply encodeURIComponent to the lowercased name
(e.g., encodeURIComponent(name.toLowerCase())) before interpolating into the URL
so all path segments are safely URL-encoded.
In `@packages/adt-contracts/src/adt/ddic/tabletypes.ts`:
- Line 6: Update the module documentation comment that currently reads "Object
type: TTYP/DA (ttypda)" to the correct table-type label "TTYP/TT" (replace the
full string so it reads "Object type: TTYP/TT"); locate this in the header
comment of the tabletypes.ts module where the object-type label is declared and
modify the label text accordingly so the docs match the table type naming used
elsewhere in this PR.
---
Nitpick comments:
In `@packages/adk/src/base/model.ts`:
- Around line 478-493: The upsert→create fallback and its createErr handling is
duplicated; extract that logic into a single private helper (e.g.,
fallbackToCreateOrHandleError or handleUpsertCreateFallback) inside the same
class and have both catch paths call it; the helper should call this.save({
...options, mode: 'create' }), catch createErr, set this._unchanged = true and
return this when this.isAlreadyExistsError(createErr), or rethrow otherwise, and
replace the duplicated blocks that currently reference shouldFallbackToCreate,
save, isAlreadyExistsError, and _unchanged with a single call to that helper.
In `@packages/adt-contracts/src/helpers/crud.ts`:
- Around line 527-534: The repo() helper added a new positional accept parameter
which is optional but could break ergonomics long-term; to preserve backward
compatibility while allowing future options, change repo() to accept a single
options object as the last argument (e.g., repo(basePath, schema, contentType,
nameTransform?, options?)) or add an overloaded signature that accepts either
the old positional args or a final options object, then adapt the call to
crud(...) to unpack options.accept and nameTransform appropriately; update the
function declaration for repo and its call-site to support both forms and ensure
crud(...) keeps accepting accept via the options you pass through.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 55178b00-f97c-4823-a2ca-ea44bdd7f944
⛔ Files ignored due to path filters (7)
packages/adt-contracts/src/generated/schemas.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/schemas/custom/blueSource.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/schemas/custom/dataelementWrapper.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/schemas/custom/index.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/typed.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/types/custom/blueSource.types.tsis excluded by!**/generated/**packages/adt-schemas/src/schemas/generated/types/custom/dataelementWrapper.types.tsis excluded by!**/generated/**
📒 Files selected for processing (23)
packages/adk/src/base/adt.tspackages/adk/src/base/model.tspackages/adk/src/base/object-set.tspackages/adk/src/objects/ddic/dtel/dtel.model.tspackages/adk/src/objects/ddic/tabl/tabl.model.tspackages/adt-client/src/index.tspackages/adt-contracts/src/adt/ddic/dataelements.tspackages/adt-contracts/src/adt/ddic/domains.tspackages/adt-contracts/src/adt/ddic/index.tspackages/adt-contracts/src/adt/ddic/structures.tspackages/adt-contracts/src/adt/ddic/tables.tspackages/adt-contracts/src/adt/ddic/tabletypes.tspackages/adt-contracts/src/adt/functions/groups.tspackages/adt-contracts/src/adt/oo/classes.tspackages/adt-contracts/src/adt/oo/interfaces.tspackages/adt-contracts/src/adt/packages/index.tspackages/adt-contracts/src/adt/programs/programs.tspackages/adt-contracts/src/helpers/crud.tspackages/adt-contracts/tests/contracts/oo.test.tspackages/adt-contracts/tests/contracts/packages.test.tspackages/adt-schemas/.xsd/custom/blueSource.xsdpackages/adt-schemas/.xsd/custom/dataelementWrapper.xsdpackages/adt-schemas/ts-xsd.config.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/adt-contracts/src/adt/ddic/domains.ts
- packages/adt-contracts/src/adt/ddic/index.ts
- packages/adt-client/src/index.ts
- packages/adk/src/objects/ddic/dtel/dtel.model.ts
- packages/adt-contracts/src/adt/ddic/structures.ts
…fallback, case-insensitive error matching - Reset _unchanged at start of save() to prevent stale state across calls - Extract duplicated upsert-create fallback into fallbackToCreate() helper - Make isAlreadyExistsError() case-insensitive for robustness - Fix TTYP/DA typo to TTYP/TT in tabletypes contract comment
|
Member
Author
Addressing review findings (CodeRabbit, Copilot, SonarCloud)✅ Fixed in 2816dd4
⏭️ Acknowledged — tracked for follow-up
🚫 Dismissed — not applicable
SonarCloudNo new issues found on the PR (0 issues reported). |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/adk/src/base/model.ts`:
- Around line 481-485: The current upsert flow incorrectly treats a 405 from
lock() and a subsequent POST returning "already exists" (422) as a successful
upsert; update the logic in shouldFallbackToCreate and the upsert path so that
fallbackToCreate is only triggered for definitive not-found responses (404) and
NOT for lock 405, and change the POST error handling so that a 422 "already
exists" does not set _unchanged = true but instead retries an update or performs
a load+compare (e.g., call update() or loadAndCompare()) before declaring
success; adjust related handlers used by fallbackToCreate,
shouldFallbackToCreate, lock(), and the code that collapses 422 responses so
existing DDIC objects aren’t misreported as written.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d30d4aa-a61a-415e-93af-519af528d2a3
📒 Files selected for processing (2)
packages/adk/src/base/model.tspackages/adt-contracts/src/adt/ddic/tabletypes.ts
Comment on lines
+481
to
485
| // For upsert, fallback to create if object doesn't exist (404) | ||
| // or endpoint doesn't support the operation (405 - common for DDIC types) | ||
| if (mode === 'upsert' && this.shouldFallbackToCreate(e)) { | ||
| return this.fallbackToCreate(options); | ||
| } |
There was a problem hiding this comment.
Don't treat lock() 405s and create already exists as a successful upsert.
A 405 from lock() only proves the lock endpoint is unusable, not that the object is missing. If an existing DDIC object hits that path, we jump to POST and then collapse 422 already exists into _unchanged = true, so the caller gets a false success even though this instance's payload was never written. Please only fall back to create on a definitive not-found, and retry update or load+compare before treating already exists as unchanged.
🔧 Minimal safe direction
} catch (e) {
- if (mode === 'upsert' && this.shouldFallbackToCreate(e)) {
+ if (mode === 'upsert' && this.isNotFoundError(e)) {
return this.fallbackToCreate(options);
}
throw e;
}
...
} catch (e: unknown) {
if (mode === 'upsert' && this.shouldFallbackToCreate(e)) {
return this.fallbackToCreate(options);
}
- // If PUT returns 422 "already exists" directly in upsert mode
- if (mode === 'upsert' && this.isAlreadyExistsError(e)) {
- this._unchanged = true;
- return this;
- }
throw e;
}
...
} catch (createErr) {
if (this.isAlreadyExistsError(createErr)) {
- this._unchanged = true;
- return this;
+ throw createErr;
}
throw createErr;
}Also applies to: 517-524, 602-608
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/adk/src/base/model.ts` around lines 481 - 485, The current upsert
flow incorrectly treats a 405 from lock() and a subsequent POST returning
"already exists" (422) as a successful upsert; update the logic in
shouldFallbackToCreate and the upsert path so that fallbackToCreate is only
triggered for definitive not-found responses (404) and NOT for lock 405, and
change the POST error handling so that a 422 "already exists" does not set
_unchanged = true but instead retries an update or performs a load+compare
(e.g., call update() or loadAndCompare()) before declaring success; adjust
related handlers used by fallbackToCreate, shouldFallbackToCreate, lock(), and
the code that collapses 422 responses so existing DDIC objects aren’t
misreported as written.
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
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.
DDIC Object Type Support
Adds full support for ABAP Dictionary object types across the ADK/ADT stack:
What's included
ADT Schemas (adt-schemas)
ADT Contracts (adt-contracts)
ADK Object Models (adk)
abapGit Handlers (adt-plugin-abapgit)
Test results
Packages modified
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores