You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
description: Full code review — SDK patterns, naming, test coverage, code smells, and security. Reads code-smell.md and code-security.md inline.
3
+
description: Full code review — SDK patterns, naming, test coverage, then runs /code-smell and /code-security.
4
4
paths:
5
5
- src/main/java/**/*.java
6
6
- src/test/java/**/*.java
7
7
---
8
8
9
9
You are a senior engineer performing a thorough code review on the Skyflow Java SDK.
10
10
11
-
## Review Mode
11
+
## Scope
12
12
13
13
Use `$ARGUMENTS` to determine scope:
14
14
-`full review` — scan all files under `src/main/java/com/skyflow/` recursively (exclude `generated/`)
@@ -22,100 +22,51 @@ Use `$ARGUMENTS` to determine scope:
22
22
23
23
---
24
24
25
-
## 1. Request / Response / Options patterns
25
+
## Step 1 — SDK Pattern Review
26
+
27
+
Check the files in scope against the rules below.
28
+
29
+
### 1. Request / Response / Options patterns
26
30
27
31
- Request builders are plain data holders — validation happens in `Validations.validateXxxRequest()` inside the controller, not in `build()`. Flag if validation logic is duplicated outside `Validations`.
28
32
- Response getters returning `ArrayList<HashMap<String, Object>>` is the established SDK pattern — do not flag these as violations.
29
33
- All response classes must have `getErrors()` returning `null` (not absent) when no errors.
30
34
- No separate `*Options` classes exist — options are fields on the request builder itself.
31
35
- SDK must not add field-level null/empty validation on top of what the backend enforces. Only structural checks (`table == null`, `values == null`) are permitted.
32
36
33
-
---
34
-
35
-
## 2. Error handling
37
+
### 2. Error handling
36
38
37
39
- All public methods must declare `throws SkyflowException`
38
40
-`SkyflowException` must be thrown (not swallowed) on invalid input
39
41
- No `System.out.println` or bare `e.printStackTrace()` — use `LogUtil`
40
42
- Catch blocks must not silently drop exceptions
41
43
-`catch (Exception e)` without re-throw or explicit handling is a critical issue
42
44
43
-
---
44
-
45
-
## 3. Naming conventions
46
-
47
-
- Classes: `PascalCase`
48
-
- Methods / fields: `camelCase` — acronyms as words: `skyflowId` not `skyflowID`, `tokenUri` not `tokenURI`, `downloadUrl` not `downloadURL`
49
-
- Constants: `UPPER_SNAKE_CASE`
50
-
- Builder setter methods: `setFooId()` not `setFooID()`
51
-
- Deprecated methods must use `@Deprecated(since = "x.x", forRemoval = true)` + `@deprecated` Javadoc with `{@link}` to the replacement
45
+
### 3. Naming conventions and response field normalisation
52
46
53
-
---
47
+
Follow the conventions in CLAUDE.md under "Naming Conventions". Key enforcement points:
48
+
- Acronyms as words: `skyflowId`, `tokenUri`, `clientId` — never uppercase abbreviations
49
+
- Builder setters: `setFooId()` not `setFooID()`; constants: `UPPER_SNAKE_CASE`; classes: `PascalCase`
50
+
- Response maps: `skyflowId` (camelCase) only — never `skyflow_id`; `getErrors()` must be present on every response class
51
+
- Deprecated methods: `@Deprecated(since = "x.x", forRemoval = true)` + `@deprecated` Javadoc with `{@link}` to replacement
54
52
55
-
## 4. Response field normalisation
56
-
57
-
- All response maps must use `skyflowId` (camelCase), never `skyflow_id` (snake_case)
58
-
-`getErrors()` must be present on every response class
59
-
60
-
---
61
-
62
-
## 5. Test coverage
53
+
### 5. Test coverage
63
54
64
55
- Every public method must have at least one positive and one negative test
65
56
- Tests must use `Assert.assertEquals` / `Assert.assertNull` — not just `Assert.fail` guards
66
57
- No mocking of the production class under test
67
58
- Reflection-based tests on private methods are acceptable only when no public API exercises the method
68
59
69
-
---
70
-
71
-
## 6. Code quality
60
+
### 6. Code quality
72
61
73
62
- No magic strings for API field names — use `Constants` or `ErrorMessage` enums
74
63
- No duplicate validation logic across request classes — belongs in `Validations`
75
64
- No `@SuppressWarnings` without a comment explaining why
76
65
-`LogUtil.printWarningLog` must be used for deprecation warnings, not `System.err`
77
66
78
-
---
79
-
80
-
## 7. Code smells
81
-
82
-
Code smells are structural signals — they may not need immediate fixes but must be flagged. Report them at **Smell** severity.
83
-
84
-
### Method & class size
85
-
-**Long method** — any method over 40 lines. Candidate for decomposition into private helpers.
86
-
-**Long class** — any class over 300 lines. May be taking on too many responsibilities.
87
-
-**Large parameter list** — more than 4 parameters on a method. Consider a config/options object.
88
-
89
-
### Responsibility violations
90
-
-**Business logic in Request/Response classes** — these are data holders. If a Request/Response class contains conditional logic beyond null-safe getters, flag it.
91
-
-**toString() with business logic** — `toString()` should only serialise state. Logic like field renaming, manual JSON construction, or conditional field injection belongs in the controller or formatter methods.
92
-
-**Validation outside Validations.java** — any `if (x == null) throw new SkyflowException(...)` outside `src/main/java/com/skyflow/utils/validations/` is misplaced.
67
+
### Output for Step 1
93
68
94
-
### Control flow
95
-
-**Deep nesting** — more than 3 levels of `if`/`for`/`try` nesting. Extract inner blocks to named methods.
96
-
-**Long if-else chains** — more than 4 branches. Consider a map, switch, or polymorphism.
97
-
-**Null checks scattered** — multiple consecutive null guards that could be replaced with `Optional` or early return.
98
-
99
-
### Data
100
-
-**Magic numbers** — literal integers or sizes (e.g. `25`, `3600`, `100`) without a named constant. Use `Constants`.
101
-
-**Raw HashMap chains** — `HashMap<String, Object>` passed through more than 2 method boundaries without a typed wrapper or comment explaining why. Flag for awareness; don't require a fix.
102
-
-**Temporary field** — a class field that is only set in certain code paths and `null` the rest of the time. Should be a local variable or method parameter instead.
103
-
104
-
### Dead code
105
-
-**Unused private methods** — private methods with no callers.
106
-
-**Unused imports** — any `import` not referenced in the file.
107
-
-**Unreachable code** — code after `return`/`throw` in the same branch.
108
-
-**Commented-out code** — blocks of commented code without explanation. Remove or add a TODO with a ticket reference.
109
-
110
-
### Comments
111
-
-**Explains what, not why** — a comment that restates what the code does (`// get the vault ID`) is noise. Only flag comments that explain the *what* without adding *why*.
112
-
-**Stale comment** — a comment that contradicts the current code (e.g. references a removed parameter or old method name).
113
-
114
-
---
115
-
116
-
## Output Format
117
-
118
-
Group findings by file. For each file:
69
+
Group findings by file:
119
70
120
71
```
121
72
### path/to/File.java
@@ -125,8 +76,6 @@ Group findings by file. For each file:
|**Smell**| Structural signal, technical debt — flag and track, fix when in the area |
140
88
141
-
End with:
142
-
1. A tech-debt summary table grouped by category (Error handling / Naming / Smells / Tests)
89
+
---
90
+
91
+
## Step 2 — Code Smell Analysis
92
+
93
+
Read the file `.claude/commands/code-smell.md` and follow all of its instructions for the same files in scope. Produce its full output (per-file smell table + smell summary + recommendation).
94
+
95
+
---
96
+
97
+
## Step 3 — Security Audit
98
+
99
+
Read the file `.claude/commands/code-security.md` and follow all of its instructions for the same files in scope. Produce its full output (per-finding blocks + summary table + overall risk rating).
description: Stage check + Jira-aware commit — extracts ticket ID from branch name and validates against pr.yml commit-message check.
4
+
---
5
+
6
+
Create a git commit for staged changes on the current branch.
7
+
8
+
Use `$ARGUMENTS` as the commit message description. If empty, ask the user for a description before proceeding.
9
+
10
+
## Step 1 — Extract ticket ID from branch name
11
+
12
+
```bash
13
+
git rev-parse --abbrev-ref HEAD
14
+
```
15
+
16
+
Extract the Jira ticket ID using the pattern `[A-Z]{1,10}-[0-9]+`:
17
+
-`devesh/SK-1234-fix-foo` → `SK-1234`
18
+
-`karthik/GV-770-ext-auth-json-error` → `GV-770`
19
+
-`username/SDK-2814-some-fix` → `SDK-2814`
20
+
21
+
If no ticket ID is found, **stop** and ask the user to provide one before continuing.
22
+
23
+
## Step 2 — Check what is staged
24
+
25
+
```bash
26
+
git status --short
27
+
git diff --cached --stat
28
+
```
29
+
30
+
If nothing is staged, list the unstaged files and ask the user which files to stage. Do not run `git add .` — ask for explicit paths (`.env`, `credentials.json`, and `generated/` must never be staged).
31
+
32
+
## Step 3 — Assemble and validate the commit message
33
+
34
+
Build the message as:
35
+
```
36
+
<ticket-id> <description>
37
+
```
38
+
39
+
If the user provided a Conventional Commits prefix (`feat`, `fix`, `chore`, `docs`, `refactor`, `test`), prepend it:
40
+
```
41
+
feat: SK-1234 add bulk insert support
42
+
fix: GV-770 handle null bearer token on refresh
43
+
```
44
+
45
+
Validate against the `pr.yml` enforced pattern: `(\[?[A-Z]{1,10}-[1-9][0-9]*)|(\[AUTOMATED\])|(Merge)|(Release)`
46
+
- Must contain a Jira ID — a bare description without a ticket ID will fail CI.
47
+
- If validation fails, report the exact requirement and stop.
48
+
49
+
## Step 4 — Commit
50
+
51
+
```bash
52
+
git commit -m "<assembled message>"
53
+
```
54
+
55
+
Report the resulting commit SHA and the commit message first line.
0 commit comments