feat(skills): add harper-best-practices skill and rules#640
Open
Ethan-Arrowood wants to merge 1 commit intoaffaan-m:mainfrom
Open
feat(skills): add harper-best-practices skill and rules#640Ethan-Arrowood wants to merge 1 commit intoaffaan-m:mainfrom
Ethan-Arrowood wants to merge 1 commit intoaffaan-m:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds 18 new Harper framework documentation files under Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
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
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
Contributor
Greptile SummaryThis PR adds a Issues found:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Developer starts Harper task] --> B{Task type?}
B -->|New data structure| C[schema- rules\nadding-tables-with-schemas\ndefining-relationships\nvector-indexing\nusing-blob-datatype\nhandling-binary-data]
B -->|API / Communication| D[api- rules\nautomatic-apis\nquerying-rest-apis\nreal-time-apps\nchecking-authentication]
B -->|Custom logic| E[logic- rules\ncustom-resources\nextending-tables\nprogrammatic-table-requests\ntypescript-type-stripping\ncaching]
B -->|Infra / Deploy| F[ops- rules\ncreating-harper-apps\ncreating-a-fabric-account-and-cluster\ndeploying-to-harper-fabric\nserving-web-content]
C --> G[harper-best-practices SKILL.md\norchestrates all rules]
D --> G
E --> G
F --> G
G --> H[Validated Harper application]
Last reviewed commit: "feat(skills): add ha..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
rules/harper/real-time-apps.md (1)
22-33: Consider improving code formatting in the SSE handling example.The SSE handling logic (Lines 25-26) is correct, but the formatting could be clearer. The inline comment and the subsequent closing brace with leading whitespace make the control flow slightly harder to parse.
♻️ Optional formatting improvement
async *connect(target, incomingMessages) { // Subscribe to table changes const subscription = await tables.MyTable.subscribe(target); - if (!incomingMessages) { return subscription; // SSE mode - } + + // SSE mode: return subscription for server-sent events + if (!incomingMessages) { + return subscription; + } // Handle incoming client messages for await (let message of incomingMessages) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/harper/real-time-apps.md` around lines 22 - 33, The SSE branch in the async generator connect function is formatted inline and hard to read; refactor the conditional that checks incomingMessages in connect (and where you create subscription via tables.MyTable.subscribe) to use a clear block on its own lines: place the comment above or beside a standalone if (!incomingMessages) { return subscription; } block so the closing brace is aligned with the surrounding code and the control flow is obvious; keep the behavior unchanged (return subscription for SSE mode) and only adjust formatting around incomingMessages, subscription, and the comment.rules/harper/vector-indexing.md (2)
113-116: Consider using map for cleaner array transformation.The traditional forEach-with-push pattern can be replaced with a more idiomatic map operation.
♻️ Proposed refactor
- let embeddings = []; - embedding.data.forEach((embeddingData) => { - embeddings.push(embeddingData.embedding); - }); - - return embeddings; + return embedding.data.map((embeddingData) => embeddingData.embedding);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/harper/vector-indexing.md` around lines 113 - 116, Replace the forEach+push pattern that builds embeddings with a map to make the transformation concise: instead of using embedding.data.forEach((embeddingData) => { embeddings.push(embeddingData.embedding); }), produce embeddings by mapping embedding.data to embeddingData.embedding (i.e., use embedding.data.map(...)) and assign the result to the embeddings variable to preserve the same value and semantics in the surrounding code.
80-80: Consider simplifying the embedding generator API.The code correctly accesses
embedding[0]to retrieve the single embedding vector from thenumber[][]return type. Both_generateOllamaEmbeddingand_generateOpenAIEmbeddingreturn arrays of embeddings—one per input—and accessing[0]extracts the first (and only) embedding when a single prompt is provided.For improved clarity, the generator methods could return
number[]directly for single-prompt use cases instead ofnumber[][], eliminating the need for index access and simplifying the API contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/harper/vector-indexing.md` at line 80, The embedding access uses embedding[0] because _generateOllamaEmbedding and _generateOpenAIEmbedding currently return number[][]; change those generator methods to return number[] for the single-prompt case (or provide a dedicated method like _generateOllamaEmbeddingSingle/_generateOpenAIEmbeddingSingle) and update all call sites that use embedding[0] to accept the direct number[] result; ensure signatures and any tests reflect the new return type and handle multi-prompt callers separately if needed.rules/harper/using-blob-datatype.md (1)
29-29: Optional: Consider rephrasing "very large files".The static analysis tool suggests that "very large" is an overused intensifier. While this is stylistic rather than substantive, you could consider alternatives like "extremely large files" or "files exceeding X MB" for added precision.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/harper/using-blob-datatype.md` at line 29, Update the guidance text that currently says "very large files" to a more precise phrase; for example, change it to "extremely large files" or better yet "files larger than X MB" (or specify a concrete threshold) in the sentence that mentions passing a stream to createBlob() so readers understand when to prefer streaming over loading the entire file into memory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 292: Replace the non-canonical product name "Harper DB" with the correct
"HarperDB" wherever it appears in the README, specifically update the
directory/heading text "harper-best-practices/" that currently reads "Harper DB"
to "HarperDB" so it matches the platform naming used elsewhere in the repo.
In `@rules/harper/caching.md`:
- Line 45: Change the link label "Real Time Apps" to the hyphenated form
"Real-Time Apps" in the sentence that references the link (the text shown as
Real Time Apps in the `subscribe()` active caching note) so the compound
adjective is consistently hyphenated.
In `@rules/harper/checking-authentication.md`:
- Line 54: Resolve the inconsistency between the /me handler and the
documentation by choosing one behavior: either update the documentation entry
for GET /me to state "401: No current session" to match the handler that returns
401 when no user is present, or modify the /me handler to return new
Response(null, { status: 200 }) when unauthenticated so the docs' "200 with
empty body means 'not signed in'" is correct; locate the behavior in the /me
handler (where it currently returns 401) and the markdown rule for GET /me in
rules/harper/checking-authentication.md and apply the corresponding change.
- Around line 122-139: The code block fence is malformed: change the opening
fence from a separate backticks line plus "js" to a single line using "```js" so
the language identifier is on the same line as the opening fence; update the
block around the RefreshJWT class (including the post method) to use a proper
```js ... ``` fenced block so syntax highlighting and markdown parsing work
correctly.
- Around line 80-108: The code fence in the markdown is malformed (the language
identifier is on the next line) around the IssueTokens class example; fix it by
moving the language identifier onto the opening backticks so the fence reads
"```js" (and ensure the corresponding closing "```" exists), which will
correctly highlight the export class IssueTokens definition and the get/post
methods.
In `@rules/harper/deploying-to-harper-fabric.md`:
- Around line 90-92: The Deploy step running "npm run deploy" doesn't map
repository secrets into environment variables for dotenv-cli; update the GitHub
Actions job step named "Deploy" (the step that runs npm run deploy) to add an
env block that assigns each required secret (the same secrets referenced earlier
in the workflow) to environment variables expected by dotenv-cli so they are
available during npm run deploy; ensure variable names match what dotenv-cli/npm
scripts expect (e.g., DB_*, API_KEY, etc.) and use the workflow secret
expressions to populate them.
In `@rules/harper/vector-indexing.md`:
- Around line 127-132: The curl example in the ProductSearch POST snippet
contains a malformed header flag (-H "accept: \) that is missing a value and a
closing quote; update that header to a valid header (for example -H "Accept:
application/json") or remove the broken -H line entirely so the curl command is
well-formed, and ensure the surrounding Content-Type and Authorization headers
remain unchanged in the ProductSearch POST example.
- Around line 93-97: The file references ollama and OLLAMA_EMBEDDING_MODEL but
doesn't import/define them; add an import for the Ollama embedding client (e.g.,
import ollama from the Ollama SDK or the project's wrapper) and define the
constant OLLAMA_EMBEDDING_MODEL with the intended model name, then ensure the
embedding call in the function that uses ollama.embed(...) remains unchanged so
the returned embedding?.embeddings works as expected; locate usages of ollama
and OLLAMA_EMBEDDING_MODEL in the file to insert the import and constant near
the other imports/constants (referencing symbols: ollama,
OLLAMA_EMBEDDING_MODEL, embed/embeddings).
In `@skills/harper-best-practices/SKILL.md`:
- Around line 26-34: The document currently has a "## Steps" section but lacks
the required "How It Works" and "Examples" sections for skill-format compliance;
rename or move the content under "## Steps" into a new "## How It Works" section
(preserve the numbered steps and any references to `rules/harper/`), and add a
separate "## Examples" section with at least one concrete usage example (input,
expected output or sample commands) to illustrate application of the steps;
ensure a top-level "When to Use" remains if present and that the new headers are
exactly "How It Works" and "Examples" to satisfy the skill format.
---
Nitpick comments:
In `@rules/harper/real-time-apps.md`:
- Around line 22-33: The SSE branch in the async generator connect function is
formatted inline and hard to read; refactor the conditional that checks
incomingMessages in connect (and where you create subscription via
tables.MyTable.subscribe) to use a clear block on its own lines: place the
comment above or beside a standalone if (!incomingMessages) { return
subscription; } block so the closing brace is aligned with the surrounding code
and the control flow is obvious; keep the behavior unchanged (return
subscription for SSE mode) and only adjust formatting around incomingMessages,
subscription, and the comment.
In `@rules/harper/using-blob-datatype.md`:
- Line 29: Update the guidance text that currently says "very large files" to a
more precise phrase; for example, change it to "extremely large files" or better
yet "files larger than X MB" (or specify a concrete threshold) in the sentence
that mentions passing a stream to createBlob() so readers understand when to
prefer streaming over loading the entire file into memory.
In `@rules/harper/vector-indexing.md`:
- Around line 113-116: Replace the forEach+push pattern that builds embeddings
with a map to make the transformation concise: instead of using
embedding.data.forEach((embeddingData) => {
embeddings.push(embeddingData.embedding); }), produce embeddings by mapping
embedding.data to embeddingData.embedding (i.e., use embedding.data.map(...))
and assign the result to the embeddings variable to preserve the same value and
semantics in the surrounding code.
- Line 80: The embedding access uses embedding[0] because
_generateOllamaEmbedding and _generateOpenAIEmbedding currently return
number[][]; change those generator methods to return number[] for the
single-prompt case (or provide a dedicated method like
_generateOllamaEmbeddingSingle/_generateOpenAIEmbeddingSingle) and update all
call sites that use embedding[0] to accept the direct number[] result; ensure
signatures and any tests reflect the new return type and handle multi-prompt
callers separately if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42589e56-f44e-457e-8939-7ab8aa8db3d8
📒 Files selected for processing (21)
AGENTS.mdREADME.mdrules/harper/adding-tables-with-schemas.mdrules/harper/automatic-apis.mdrules/harper/caching.mdrules/harper/checking-authentication.mdrules/harper/creating-a-fabric-account-and-cluster.mdrules/harper/creating-harper-apps.mdrules/harper/custom-resources.mdrules/harper/defining-relationships.mdrules/harper/deploying-to-harper-fabric.mdrules/harper/extending-tables.mdrules/harper/handling-binary-data.mdrules/harper/programmatic-table-requests.mdrules/harper/querying-rest-apis.mdrules/harper/real-time-apps.mdrules/harper/serving-web-content.mdrules/harper/typescript-type-stripping.mdrules/harper/using-blob-datatype.mdrules/harper/vector-indexing.mdskills/harper-best-practices/SKILL.md
README.md
Outdated
| | |-- videodb/ # Video and audio: ingest, search, edit, generate, stream (NEW) | ||
| | |-- golang-patterns/ # Go idioms and best practices | ||
| | |-- golang-testing/ # Go testing patterns, TDD, benchmarks | ||
| | |-- harper-best-practices/ # Harper DB schema, APIs, authentication, deployment |
Contributor
There was a problem hiding this comment.
Use canonical product name for consistency.
Line 292 says “Harper DB”; use “HarperDB” to match the platform naming used elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 292, Replace the non-canonical product name "Harper DB"
with the correct "HarperDB" wherever it appears in the README, specifically
update the directory/heading text "harper-best-practices/" that currently reads
"Harper DB" to "HarperDB" so it matches the platform naming used elsewhere in
the repo.
| const { MyCache } = tables; | ||
| MyCache.sourcedFrom(ThirdPartyAPI); | ||
| ``` | ||
| 4. **Implement Active Caching (Optional)**: Use `subscribe()` for proactive updates. See [Real Time Apps](real-time-apps.md). |
Contributor
There was a problem hiding this comment.
Hyphenate compound adjective in link label.
At Line 45, “Real Time Apps” should be “Real-Time Apps” for consistent phrasing.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ubscribe()` for proactive updates. See Real Time Apps. 5. **Implemen...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/harper/caching.md` at line 45, Change the link label "Real Time Apps"
to the hyphenated form "Real-Time Apps" in the sentence that references the link
(the text shown as Real Time Apps in the `subscribe()` active caching note) so
the compound adjective is consistently hyphenated.
Comment on lines
+122
to
+139
| ``` | ||
| js | ||
| export class RefreshJWT extends Resource { | ||
| static loadAsInstance = false; | ||
|
|
||
| async post(target, data) { | ||
| if (!data.refreshToken) { | ||
| throw new Error('refreshToken is required'); | ||
| } | ||
|
|
||
| const { operation_token: jwt } = await databases.system.hdb_user.operation({ | ||
| operation: 'refresh_operation_token', | ||
| refresh_token: data.refreshToken, | ||
| }); | ||
| return { jwt }; | ||
| } | ||
| } | ||
| ``` |
Contributor
There was a problem hiding this comment.
Fix malformed code block syntax.
The code block delimiter is improperly formatted. The language identifier should be on the same line as the opening backticks.
📝 Proposed fix
-```
-js
+```js
export class RefreshJWT extends Resource {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` | |
| js | |
| export class RefreshJWT extends Resource { | |
| static loadAsInstance = false; | |
| async post(target, data) { | |
| if (!data.refreshToken) { | |
| throw new Error('refreshToken is required'); | |
| } | |
| const { operation_token: jwt } = await databases.system.hdb_user.operation({ | |
| operation: 'refresh_operation_token', | |
| refresh_token: data.refreshToken, | |
| }); | |
| return { jwt }; | |
| } | |
| } | |
| ``` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/harper/checking-authentication.md` around lines 122 - 139, The code
block fence is malformed: change the opening fence from a separate backticks
line plus "js" to a single line using "```js" so the language identifier is on
the same line as the opening fence; update the block around the RefreshJWT class
(including the post method) to use a proper ```js ... ``` fenced block so syntax
highlighting and markdown parsing work correctly.
Comment on lines
+90
to
+92
| - name: Deploy | ||
| run: npm run deploy | ||
| ``` |
Contributor
There was a problem hiding this comment.
Missing environment variable mapping in deploy step.
The GitHub Actions workflow references repository secrets at Lines 94-97, but the deploy step (Lines 90-92) doesn't explicitly map these secrets to environment variables. The dotenv-cli expects environment variables, but GitHub Actions doesn't automatically expose repository secrets to the workflow environment.
🔧 Proposed fix to add environment variables
- name: Deploy
run: npm run deploy
+ env:
+ CLI_TARGET: ${{ secrets.CLI_TARGET }}
+ CLI_TARGET_USERNAME: ${{ secrets.CLI_TARGET_USERNAME }}
+ CLI_TARGET_PASSWORD: ${{ secrets.CLI_TARGET_PASSWORD }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/harper/deploying-to-harper-fabric.md` around lines 90 - 92, The Deploy
step running "npm run deploy" doesn't map repository secrets into environment
variables for dotenv-cli; update the GitHub Actions job step named "Deploy" (the
step that runs npm run deploy) to add an env block that assigns each required
secret (the same secrets referenced earlier in the workflow) to environment
variables expected by dotenv-cli so they are available during npm run deploy;
ensure variable names match what dotenv-cli/npm scripts expect (e.g., DB_*,
API_KEY, etc.) and use the workflow secret expressions to populate them.
Comment on lines
+93
to
+97
| const embedding = await ollama.embed({ | ||
| model: OLLAMA_EMBEDDING_MODEL, | ||
| input: promptData, | ||
| }); | ||
| return embedding?.embeddings; |
Contributor
There was a problem hiding this comment.
Missing import and constant for Ollama embedding.
The code references ollama (line 93) and OLLAMA_EMBEDDING_MODEL (line 94), but neither is imported nor defined in this file. This will cause a runtime error when the Ollama embedding generator is selected.
🔧 Proposed fix
Add the missing import and constant at the top of the file after the OpenAI imports:
import OpenAI from 'openai';
const openai = new OpenAI();
+
+import { Ollama } from 'ollama';
+const ollama = new Ollama();
+
// the name of the OpenAI embedding model
const OPENAI_EMBEDDING_MODEL = 'text-embedding-3-small';
+// the name of the Ollama embedding model
+const OLLAMA_EMBEDDING_MODEL = 'nomic-embed-text';
const SIMILARITY_THRESHOLD = 0.5;Note: Replace 'nomic-embed-text' with the actual Ollama model name you intend to use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/harper/vector-indexing.md` around lines 93 - 97, The file references
ollama and OLLAMA_EMBEDDING_MODEL but doesn't import/define them; add an import
for the Ollama embedding client (e.g., import ollama from the Ollama SDK or the
project's wrapper) and define the constant OLLAMA_EMBEDDING_MODEL with the
intended model name, then ensure the embedding call in the function that uses
ollama.embed(...) remains unchanged so the returned embedding?.embeddings works
as expected; locate usages of ollama and OLLAMA_EMBEDDING_MODEL in the file to
insert the import and constant near the other imports/constants (referencing
symbols: ollama, OLLAMA_EMBEDDING_MODEL, embed/embeddings).
rules/harper/vector-indexing.md
Outdated
Comment on lines
+127
to
+132
| curl -X POST "http://localhost:9926/ProductSearch/" \ | ||
| -H "accept: \ | ||
| -H "Content-Type: application/json" \ | ||
| -H "Authorization: Basic <YOUR_AUTH>" \ | ||
| -d '{"prompt": "shorts for the gym"}' | ||
| ```` |
Contributor
There was a problem hiding this comment.
Fix malformed curl command.
Line 128 has an incomplete header flag: -H "accept: \ with no value and no closing quote. This will cause the curl command to fail.
🔧 Proposed fix
curl -X POST "http://localhost:9926/ProductSearch/" \
--H "accept: \
+-H "Accept: application/json" \
-H "Content-Type: application/json" \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| curl -X POST "http://localhost:9926/ProductSearch/" \ | |
| -H "accept: \ | |
| -H "Content-Type: application/json" \ | |
| -H "Authorization: Basic <YOUR_AUTH>" \ | |
| -d '{"prompt": "shorts for the gym"}' | |
| ```` | |
| curl -X POST "http://localhost:9926/ProductSearch/" \ | |
| -H "Accept: application/json" \ | |
| -H "Content-Type: application/json" \ | |
| -H "Authorization: Basic <YOUR_AUTH>" \ | |
| -d '{"prompt": "shorts for the gym"}' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/harper/vector-indexing.md` around lines 127 - 132, The curl example in
the ProductSearch POST snippet contains a malformed header flag (-H "accept: \)
that is missing a value and a closing quote; update that header to a valid
header (for example -H "Accept: application/json") or remove the broken -H line
entirely so the curl command is well-formed, and ensure the surrounding
Content-Type and Authorization headers remain unchanged in the ProductSearch
POST example.
Comment on lines
+26
to
+34
| ## Steps | ||
|
|
||
| 1. Review the requirements for the task (schema design, API needs, or infrastructure setup). | ||
| 2. Consult the relevant category under "Rule Categories by Priority" to understand the impact of your decisions. | ||
| 3. Apply specific rules from the "Rule Details" section below by reading the corresponding rule files in `rules/harper/`. | ||
| 4. If you're building a new table, prioritize the `schema-` rules. | ||
| 5. If you're extending functionality, consult the `logic-` and `api-` rules. | ||
| 6. Validate your implementation against the `ops-` rules before deployment. | ||
|
|
Contributor
There was a problem hiding this comment.
Add explicit “How It Works” and “Examples” sections for skill-format compliance.
Line 26 currently starts a ## Steps section, but the skill format expects clear How It Works and Examples sections.
✏️ Suggested doc-structure patch
## When to Use
@@
- Deploying applications to Harper Fabric
-## Steps
+## How It Works
1. Review the requirements for the task (schema design, API needs, or infrastructure setup).
2. Consult the relevant category under "Rule Categories by Priority" to understand the impact of your decisions.
3. Apply specific rules from the "Rule Details" section below by reading the corresponding rule files in `rules/harper/`.
4. If you're building a new table, prioritize the `schema-` rules.
5. If you're extending functionality, consult the `logic-` and `api-` rules.
6. Validate your implementation against the `ops-` rules before deployment.
+
+## Examples
+
+See the concrete examples embedded in each rule subsection below (GraphQL schemas, REST query patterns, and deployment workflow snippets).As per coding guidelines, “Skills should be formatted as Markdown with clear sections for When to Use, How It Works, and Examples.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Steps | |
| 1. Review the requirements for the task (schema design, API needs, or infrastructure setup). | |
| 2. Consult the relevant category under "Rule Categories by Priority" to understand the impact of your decisions. | |
| 3. Apply specific rules from the "Rule Details" section below by reading the corresponding rule files in `rules/harper/`. | |
| 4. If you're building a new table, prioritize the `schema-` rules. | |
| 5. If you're extending functionality, consult the `logic-` and `api-` rules. | |
| 6. Validate your implementation against the `ops-` rules before deployment. | |
| ## Steps | |
| 1. Review the requirements for the task (schema design, API needs, or infrastructure setup). | |
| 2. Consult the relevant category under "Rule Categories by Priority" to understand the impact of your decisions. | |
| 3. Apply specific rules from the "Rule Details" section below by reading the corresponding rule files in `rules/harper/`. | |
| 4. If you're building a new table, prioritize the `schema-` rules. | |
| 5. If you're extending functionality, consult the `logic-` and `api-` rules. | |
| 6. Validate your implementation against the `ops-` rules before deployment. | |
| ## How It Works | |
| 1. Review the requirements for the task (schema design, API needs, or infrastructure setup). | |
| 2. Consult the relevant category under "Rule Categories by Priority" to understand the impact of your decisions. | |
| 3. Apply specific rules from the "Rule Details" section below by reading the corresponding rule files in `rules/harper/`. | |
| 4. If you're building a new table, prioritize the `schema-` rules. | |
| 5. If you're extending functionality, consult the `logic-` and `api-` rules. | |
| 6. Validate your implementation against the `ops-` rules before deployment. | |
| ## Examples | |
| See the concrete examples embedded in each rule subsection below (GraphQL schemas, REST query patterns, and deployment workflow snippets). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/harper-best-practices/SKILL.md` around lines 26 - 34, The document
currently has a "## Steps" section but lacks the required "How It Works" and
"Examples" sections for skill-format compliance; rename or move the content
under "## Steps" into a new "## How It Works" section (preserve the numbered
steps and any references to `rules/harper/`), and add a separate "## Examples"
section with at least one concrete usage example (input, expected output or
sample commands) to illustrate application of the steps; ensure a top-level
"When to Use" remains if present and that the new headers are exactly "How It
Works" and "Examples" to satisfy the skill format.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
21 issues found across 21 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="rules/harper/automatic-apis.md">
<violation number="1" location="rules/harper/automatic-apis.md:32">
P2: Documentation hardcodes insecure `ws://` WebSocket URL, which breaks on HTTPS origins (mixed content) and promotes unencrypted transport.</violation>
</file>
<file name="rules/harper/serving-web-content.md">
<violation number="1" location="rules/harper/serving-web-content.md:33">
P2: Vite plugin dev workflow requires running Harper (e.g., `harper run .`) so the plugin loads; `npm run dev` alone is ambiguous and can bypass Harper entirely.</violation>
</file>
<file name="rules/harper/deploying-to-harper-fabric.md">
<violation number="1" location="rules/harper/deploying-to-harper-fabric.md:91">
P1: The deployment workflow example tells users to create secrets but never injects them into the deploy step, so `npm run deploy` won’t receive required CLI credentials.</violation>
</file>
<file name="skills/harper-best-practices/SKILL.md">
<violation number="1" location="skills/harper-best-practices/SKILL.md:384">
P2: The GitHub Actions deploy example documents required secrets but never injects them into the deploy step, making the example likely nonfunctional for authenticated deployment.</violation>
</file>
<file name="rules/harper/creating-harper-apps.md">
<violation number="1" location="rules/harper/creating-harper-apps.md:45">
P2: Documentation mixes package managers: users can scaffold with PNPM/Bun but follow-up commands hardcode `npm`, which may cause lockfile conflicts or failures in environments without npm.</violation>
</file>
<file name="rules/harper/querying-rest-apis.md">
<violation number="1" location="rules/harper/querying-rest-apis.md:20">
P2: Pagination example includes a raw space in a literal URL (`limit(20, 10)`), which is not a valid unencoded URI and can lead to malformed or altered requests when copied verbatim.</violation>
</file>
<file name="rules/harper/vector-indexing.md">
<violation number="1" location="rules/harper/vector-indexing.md:48">
P2: Missing opening code fence before the main JavaScript example breaks markdown block structure and rendering.</violation>
<violation number="2" location="rules/harper/vector-indexing.md:93">
P2: The Ollama example calls `ollama.embed` with `OLLAMA_EMBEDDING_MODEL`, but neither `ollama` nor `OLLAMA_EMBEDDING_MODEL` is defined/imported in the snippet, so the Ollama path will throw a ReferenceError if used.</violation>
<violation number="3" location="rules/harper/vector-indexing.md:128">
P2: The documented curl example has a malformed `accept` header, making the command invalid when copied and run.</violation>
</file>
<file name="rules/harper/custom-resources.md">
<violation number="1" location="rules/harper/custom-resources.md:35">
P2: The custom-resources guide says resources may be `.js` or `.ts`, but the recommended `jsResource.files` example only matches `.ts`, which can prevent `.js` resources from loading.</violation>
</file>
<file name="rules/harper/checking-authentication.md">
<violation number="1" location="rules/harper/checking-authentication.md:27">
P2: Sign-in example contradicts the file’s own status-code contract by not returning 400 for missing credentials and mapping all login failures to 403.</violation>
<violation number="2" location="rules/harper/checking-authentication.md:38">
P2: `GET /me` example returns 401 for anonymous users, but the status-code conventions section specifies a 200 empty-body response for unauthenticated users, creating inconsistent guidance.</violation>
<violation number="3" location="rules/harper/checking-authentication.md:46">
P2: Sign-out example returns 200 even when no session exists, contradicting documented 401 behavior and session-verification checklist.</violation>
</file>
<file name="rules/harper/typescript-type-stripping.md">
<violation number="1" location="rules/harper/typescript-type-stripping.md:16">
P2: The documented minimum Node version is too low for unflagged direct `.ts` execution, so the instructions are incorrect for Node 22.6–22.17.x.</violation>
<violation number="2" location="rules/harper/typescript-type-stripping.md:18">
P2: Documentation overstates direct TypeScript support; Node strip-types only supports erasable syntax, so “standard TypeScript” guidance can produce runtime failures.</violation>
<violation number="3" location="rules/harper/typescript-type-stripping.md:27">
P2: Instruction to use `.ts` extensions omits required TypeScript compiler configuration (`allowImportingTsExtensions` + `noEmit`/`emitDeclarationOnly`), so following the rule can cause TS5097 errors in `tsc`/editor diagnostics.</violation>
</file>
<file name="rules/harper/handling-binary-data.md">
<violation number="1" location="rules/harper/handling-binary-data.md:23">
P2: The binary-data guidance hard-codes `image/jpeg` for blob metadata and response headers even though it’s presented as generic (images, audio, etc.). This will mislabel non‑JPEG uploads. The example should demonstrate using the actual MIME type per upload instead of a fixed JPEG type.</violation>
</file>
<file name="rules/harper/real-time-apps.md">
<violation number="1" location="rules/harper/real-time-apps.md:25">
P2: In an `async *connect` generator, `return subscription` ends the generator and does not stream subscription events. This SSE branch will terminate immediately instead of yielding updates; use `yield*`/`for await` to forward the subscription or make `connect` non-generator.</violation>
<violation number="2" location="rules/harper/real-time-apps.md:37">
P1: Documentation hardcodes `ws://` WebSocket scheme causing mixed-content failures under HTTPS and promotes insecure transport. Should use `wss://` (secure) or provide guidance on selecting the appropriate scheme based on the page protocol.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:292">
P2: README adds Harper skill but doesn't document how to install the corresponding rules/harper/ ruleset. The install examples only mention typescript/python/golang/swift/php targets, leaving no supported installation path for the new Harper rules.</violation>
</file>
<file name="rules/harper/defining-relationships.md">
<violation number="1" location="rules/harper/defining-relationships.md:33">
P2: Nested `select()` syntax should use curly braces for sub-attributes (e.g., `books{title}`), not parentheses, per HarperDB REST docs.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ``` | ||
| 3. **Use Pub/Sub**: Use `tables.TableName.subscribe(query)` to listen for specific data changes and stream them to the client. | ||
| 4. **Handle SSE**: Ensure your `connect` method gracefully handles cases where `incomingMessages` is null (Server-Sent Events). | ||
| 5. **Connect from Client**: Use standard WebSockets (`new WebSocket('ws://...')`) to connect to your resource endpoint. |
Contributor
There was a problem hiding this comment.
P1: Documentation hardcodes ws:// WebSocket scheme causing mixed-content failures under HTTPS and promotes insecure transport. Should use wss:// (secure) or provide guidance on selecting the appropriate scheme based on the page protocol.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rules/harper/real-time-apps.md, line 37:
<comment>Documentation hardcodes `ws://` WebSocket scheme causing mixed-content failures under HTTPS and promotes insecure transport. Should use `wss://` (secure) or provide guidance on selecting the appropriate scheme based on the page protocol.</comment>
<file context>
@@ -0,0 +1,37 @@
+ ```
+3. **Use Pub/Sub**: Use `tables.TableName.subscribe(query)` to listen for specific data changes and stream them to the client.
+4. **Handle SSE**: Ensure your `connect` method gracefully handles cases where `incomingMessages` is null (Server-Sent Events).
+5. **Connect from Client**: Use standard WebSockets (`new WebSocket('ws://...')`) to connect to your resource endpoint.
</file context>
Suggested change
| 5. **Connect from Client**: Use standard WebSockets (`new WebSocket('ws://...')`) to connect to your resource endpoint. | |
| 5. **Connect from Client**: Use standard WebSockets (`new WebSocket('wss://...')`) to connect to your resource endpoint. Use `wss://` for HTTPS pages and `ws://` for local development. |
| - **Update Record (Partial)**: `PATCH /{TableName}/{id}` | ||
| - **Delete All/Filtered Records**: `DELETE /{TableName}/` | ||
| - **Delete Single Record**: `DELETE /{TableName}/{id}` | ||
| 3. **Use Automatic WebSockets**: Connect to `ws://your-harper-instance/{TableName}` to receive events whenever updates are made to that table. This is the easiest way to add real-time capabilities. For more complex needs, see [Real-time Applications](real-time-apps.md). |
Contributor
There was a problem hiding this comment.
P2: Documentation hardcodes insecure ws:// WebSocket URL, which breaks on HTTPS origins (mixed content) and promotes unencrypted transport.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rules/harper/automatic-apis.md, line 32:
<comment>Documentation hardcodes insecure `ws://` WebSocket URL, which breaks on HTTPS origins (mixed content) and promotes unencrypted transport.</comment>
<file context>
@@ -0,0 +1,34 @@
+ - **Update Record (Partial)**: `PATCH /{TableName}/{id}`
+ - **Delete All/Filtered Records**: `DELETE /{TableName}/`
+ - **Delete Single Record**: `DELETE /{TableName}/{id}`
+3. **Use Automatic WebSockets**: Connect to `ws://your-harper-instance/{TableName}` to receive events whenever updates are made to that table. This is the easiest way to add real-time capabilities. For more complex needs, see [Real-time Applications](real-time-apps.md).
+4. **Apply Filtering and Querying**: Use query parameters with `GET /{TableName}/` and `DELETE /{TableName}/`. See the [Querying REST APIs](querying-rest-apis.md) skill for advanced details.
+5. **Customize if Needed**: If the automatic APIs don't meet your requirements, [customize the resources](./custom-resources.md).
</file context>
| ``` | ||
| - Ensure `vite.config.ts` and `index.html` are in the project root. | ||
| - Install dependencies: `npm install --save-dev vite @harperfast/vite-plugin`. | ||
| - Use `npm run dev` for development with HMR. |
Contributor
There was a problem hiding this comment.
P2: Vite plugin dev workflow requires running Harper (e.g., harper run .) so the plugin loads; npm run dev alone is ambiguous and can bypass Harper entirely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rules/harper/serving-web-content.md, line 33:
<comment>Vite plugin dev workflow requires running Harper (e.g., `harper run .`) so the plugin loads; `npm run dev` alone is ambiguous and can bypass Harper entirely.</comment>
<file context>
@@ -0,0 +1,34 @@
+ ```
+ - Ensure `vite.config.ts` and `index.html` are in the project root.
+ - Install dependencies: `npm install --save-dev vite @harperfast/vite-plugin`.
+ - Use `npm run dev` for development with HMR.
+4. **Deploy for Production**: For Vite apps, use a build script to generate static files into a `web/` folder and deploy them using the static handler pattern.
</file context>
| - name: Run lint | ||
| run: npm run lint | ||
| - name: Deploy | ||
| run: npm run deploy |
Contributor
There was a problem hiding this comment.
P2: The GitHub Actions deploy example documents required secrets but never injects them into the deploy step, making the example likely nonfunctional for authenticated deployment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/harper-best-practices/SKILL.md, line 384:
<comment>The GitHub Actions deploy example documents required secrets but never injects them into the deploy step, making the example likely nonfunctional for authenticated deployment.</comment>
<file context>
@@ -0,0 +1,398 @@
+ - name: Run lint
+ run: npm run lint
+ - name: Deploy
+ run: npm run deploy
+```
+
</file context>
| @@ -0,0 +1,43 @@ | |||
| --- | |||
Contributor
There was a problem hiding this comment.
P2: The binary-data guidance hard-codes image/jpeg for blob metadata and response headers even though it’s presented as generic (images, audio, etc.). This will mislabel non‑JPEG uploads. The example should demonstrate using the actual MIME type per upload instead of a fixed JPEG type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rules/harper/handling-binary-data.md, line 23:
<comment>The binary-data guidance hard-codes `image/jpeg` for blob metadata and response headers even though it’s presented as generic (images, audio, etc.). This will mislabel non‑JPEG uploads. The example should demonstrate using the actual MIME type per upload instead of a fixed JPEG type.</comment>
<file context>
@@ -0,0 +1,43 @@
+ async post(target, record) {
+ if (record.data) {
+ record.data = createBlob(Buffer.from(record.data, 'base64'), {
+ type: 'image/jpeg',
+ });
+ }
</file context>
README.md
Outdated
| | |-- videodb/ # Video and audio: ingest, search, edit, generate, stream (NEW) | ||
| | |-- golang-patterns/ # Go idioms and best practices | ||
| | |-- golang-testing/ # Go testing patterns, TDD, benchmarks | ||
| | |-- harper-best-practices/ # Harper DB schema, APIs, authentication, deployment |
Contributor
There was a problem hiding this comment.
P2: README adds Harper skill but doesn't document how to install the corresponding rules/harper/ ruleset. The install examples only mention typescript/python/golang/swift/php targets, leaving no supported installation path for the new Harper rules.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 292:
<comment>README adds Harper skill but doesn't document how to install the corresponding rules/harper/ ruleset. The install examples only mention typescript/python/golang/swift/php targets, leaving no supported installation path for the new Harper rules.</comment>
<file context>
@@ -289,6 +289,7 @@ everything-claude-code/
| |-- videodb/ # Video and audio: ingest, search, edit, generate, stream (NEW)
| |-- golang-patterns/ # Go idioms and best practices
| |-- golang-testing/ # Go testing patterns, TDD, benchmarks
+| |-- harper-best-practices/ # Harper DB schema, APIs, authentication, deployment
| |-- cpp-coding-standards/ # C++ coding standards from C++ Core Guidelines (NEW)
| |-- cpp-testing/ # C++ testing with GoogleTest, CMake/CTest (NEW)
</file context>
- Adds 1 skill and 18 rule files for Harper covering schema design,
automatic APIs, authentication, custom resources, and deployment to
Harper Fabric.
Ethan-Arrowood
force-pushed
the
feat/add-harper-skills
branch
from
6c8da71 to
285878f
Compare
Contributor
|
Contributor
Analysis Failed
Troubleshooting
Retry: |
Comment on lines
+93
to
+96
| async _generateOllamaEmbedding(promptData) { | ||
| const embedding = await ollama.embed({ | ||
| model: OLLAMA_EMBEDDING_MODEL, | ||
| input: promptData, |
Contributor
There was a problem hiding this comment.
ollama import and OLLAMA_EMBEDDING_MODEL constant
The _generateOllamaEmbedding method references both ollama (the client) and OLLAMA_EMBEDDING_MODEL (the model name constant), but neither is imported or defined anywhere in the code block. openai and OPENAI_EMBEDDING_MODEL are defined at the module level (lines 51–55), but the Ollama equivalents are completely absent. Any developer who copies this example will get a ReferenceError at runtime when EMBEDDING_GENERATOR === 'ollama'.
Suggested change
| async _generateOllamaEmbedding(promptData) { | |
| const embedding = await ollama.embed({ | |
| model: OLLAMA_EMBEDDING_MODEL, | |
| input: promptData, | |
| async _generateOllamaEmbedding(promptData) { | |
| const embedding = await ollama.embed({ | |
| model: OLLAMA_EMBEDDING_MODEL, | |
| input: promptData, | |
| }); | |
| return embedding?.embeddings; | |
| } |
The snippet at the top of the code block should also include the missing imports/constants alongside the openai setup, for example:
import ollama from 'ollama';
const OLLAMA_EMBEDDING_MODEL = 'nomic-embed-text'; // or whichever model is recommended
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
skills/harper-best-practices/SKILL.md (1)
26-34:⚠️ Potential issue | 🟡 MinorUse required top-level section names for skill-format compliance.
## Stepsshould be renamed to## How It Works, and add a top-level## Examplessection so this skill follows the required structure.✏️ Suggested patch
-## Steps +## How It Works 1. Review the requirements for the task (schema design, API needs, or infrastructure setup). 2. Consult the relevant category under "Rule Categories by Priority" to understand the impact of your decisions. 3. Apply specific rules from the "Rule Details" section below by reading the corresponding rule files in `rules/harper/`. 4. If you're building a new table, prioritize the `schema-` rules. 5. If you're extending functionality, consult the `logic-` and `api-` rules. 6. Validate your implementation against the `ops-` rules before deployment. + +## Examples + +- **Task:** “I need to add a new table with relationships and expose APIs.” +- **How to apply this skill:** Follow “How It Works” Step 1-3, then prioritize `schema-` and `api-` rules in `rules/harper/`.As per coding guidelines, “Skills should be formatted as Markdown with clear sections for When to Use, How It Works, and Examples.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/harper-best-practices/SKILL.md` around lines 26 - 34, Rename the top-level heading "## Steps" to "## How It Works" and add a new top-level "## Examples" section below it (populate with at least one short example or placeholder) so the skill follows the required top-level sections ("When to Use", "How It Works", "Examples"); update any internal links or references that pointed to "## Steps" to use "## How It Works" and ensure the new "## Examples" header is present and not empty.rules/harper/deploying-to-harper-fabric.md (1)
90-92:⚠️ Potential issue | 🔴 CriticalMap repository secrets into the Deploy step environment.
Line 90-92 invokes
npm run deploybut does not exposeCLI_TARGET,CLI_TARGET_USERNAME, andCLI_TARGET_PASSWORDto that step.🛠️ Suggested fix
- name: Deploy run: npm run deploy + env: + CLI_TARGET: ${{ secrets.CLI_TARGET }} + CLI_TARGET_USERNAME: ${{ secrets.CLI_TARGET_USERNAME }} + CLI_TARGET_PASSWORD: ${{ secrets.CLI_TARGET_PASSWORD }}#!/bin/bash # Verify the workflow snippet includes env mapping on the Deploy step. rg -n -C3 'name:\s*Deploy|run:\s*npm run deploy|CLI_TARGET|CLI_TARGET_USERNAME|CLI_TARGET_PASSWORD' rules/harper/deploying-to-harper-fabric.md🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/harper/deploying-to-harper-fabric.md` around lines 90 - 92, The Deploy step currently runs "npm run deploy" but does not pass the required secrets; update the GitHub Actions step named "Deploy" (the step that runs npm run deploy) to map the repository secrets into its environment by adding env entries for CLI_TARGET, CLI_TARGET_USERNAME, and CLI_TARGET_PASSWORD that reference the corresponding secrets (e.g., ${{ secrets.CLI_TARGET }}, ${{ secrets.CLI_TARGET_USERNAME }}, ${{ secrets.CLI_TARGET_PASSWORD }}), so the deploy script can access them at runtime.rules/harper/vector-indexing.md (2)
93-99:⚠️ Potential issue | 🔴 CriticalDefine/import Ollama client symbols used by the snippet.
Line 93-99 uses
ollamaandOLLAMA_EMBEDDING_MODELbut this example block does not define them, so copied code will fail.🧩 Suggested fix
import OpenAI from 'openai'; const openai = new OpenAI(); // the name of the OpenAI embedding model const OPENAI_EMBEDDING_MODEL = 'text-embedding-3-small'; +import { Ollama } from 'ollama'; +const ollama = new Ollama(); +const OLLAMA_EMBEDDING_MODEL = 'nomic-embed-text';#!/bin/bash # Verify references exist and declarations are present in the doc snippet. rg -n -C2 'ollama|OLLAMA_EMBEDDING_MODEL|OpenAI_EMBEDDING_MODEL|import \{ Ollama \}' rules/harper/vector-indexing.md🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/harper/vector-indexing.md` around lines 93 - 99, The snippet's _generateOllamaEmbedding function references undefined symbols (ollama and OLLAMA_EMBEDDING_MODEL); add/import the Ollama client and define the embedding model constant referenced by that function (e.g., create or import an ollama client instance named "ollama" and export/declare "OLLAMA_EMBEDDING_MODEL") so the example is self-contained and copy-paste runnable; update the top of the example to show the import/initialization for the Ollama client and the OLLAMA_EMBEDDING_MODEL constant used by _generateOllamaEmbedding.
129-133:⚠️ Potential issue | 🟠 MajorFix malformed
curlheader quoting.Line 130 has a broken header string and will fail as written.
🧪 Suggested fix
curl -X POST "http://localhost:9926/ProductSearch/" \ --H "accept: application/json \ +-H "Accept: application/json" \ -H "Content-Type: application/json" \ -H "Authorization: Basic <YOUR_AUTH>" \ -d '{"prompt": "shorts for the gym"}'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/harper/vector-indexing.md` around lines 129 - 133, The curl command has malformed header quoting: the -H "accept: application/json line is missing a closing quote (and proper line continuation), causing the command to fail; edit the curl block so each -H uses a complete quoted string (e.g., -H "Accept: application/json") and ensure each continued line ends with a backslash, keeping the POST target "http://localhost:9926/ProductSearch/" and the JSON body '-d '{"prompt": "shorts for the gym"}'' unchanged.rules/harper/caching.md (1)
45-45:⚠️ Potential issue | 🟡 MinorUse “Real-Time Apps” for consistent terminology.
Line 45 should use the hyphenated form in the link label to match the target doc naming and prior phrasing consistency.
✏️ Suggested edit
-4. **Implement Active Caching (Optional)**: Use `subscribe()` for proactive updates. See [Real Time Apps](real-time-apps.md). +4. **Implement Active Caching (Optional)**: Use `subscribe()` for proactive updates. See [Real-Time Apps](real-time-apps.md).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/harper/caching.md` at line 45, Update the link label to use the hyphenated, consistent title "Real-Time Apps" in the sentence that currently reads "Use `subscribe()` for proactive updates. See [Real Time Apps](real-time-apps.md)." Replace the bracketed text so it becomes [Real-Time Apps](real-time-apps.md) to match the target document name and prior phrasing consistency; keep the rest of the sentence and the `subscribe()` reference unchanged.
🧹 Nitpick comments (2)
rules/harper/using-blob-datatype.md (1)
29-29: Optional wording polish on Line 29.Consider replacing “very large files” with a more specific phrase (e.g., “large files” or “multi-GB files”) to keep phrasing tighter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/harper/using-blob-datatype.md` at line 29, Change the phrasing in the streaming guidance to be more specific: replace "very large files" with a tighter term such as "large files" or "multi-GB files" in the sentence that references createBlob(), e.g., update the line that currently reads "For very large files, pass a stream to `createBlob()` to avoid loading the entire file into memory." to use your chosen phrase so it reads "For large files, pass a stream to `createBlob()`..." or "For multi-GB files, pass a stream to `createBlob()`..." to improve clarity.rules/harper/extending-tables.md (1)
36-36: Clarify super-call notation in docs.Line 36 should use call-style wording (
super.method(...)) to avoid ambiguity with bracket notation.✍️ Suggested edit
-4. **Override Methods**: Override `get`, `post`, `put`, `patch`, or `delete` as needed. Always call `super[method]` to maintain default Harper functionality unless you intend to replace it entirely. +4. **Override Methods**: Override `get`, `post`, `put`, `patch`, or `delete` as needed. Call the corresponding `super.method(...)` to preserve default Harper behavior unless you intentionally replace it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/harper/extending-tables.md` at line 36, Update the documentation line that instructs calling the parent implementation to use call-style notation instead of bracket notation: replace the ambiguous "Always call `super[method]`" phrasing with explicit call-style wording such as "Always call `super.method(...)`" and mention the HTTP handler names (`get`, `post`, `put`, `patch`, `delete`) so readers know to call e.g. `super.get(...)`, `super.post(...)`, etc.; keep the guidance about calling super unless replacing behavior entirely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rules/harper/automatic-apis.md`:
- Line 32: Update the "Use Automatic WebSockets" line to recommend secure
WebSockets by default: replace the production example
`ws://your-harper-instance/{TableName}` with
`wss://your-harper-instance/{TableName}` and add a short note that `ws://`
should only be used for local/dev testing; keep `ws://` examples only in a
local/dev subsection or parenthetical so the guidance clearly distinguishes
production (`wss://`) from development (`ws://`).
---
Duplicate comments:
In `@rules/harper/caching.md`:
- Line 45: Update the link label to use the hyphenated, consistent title
"Real-Time Apps" in the sentence that currently reads "Use `subscribe()` for
proactive updates. See [Real Time Apps](real-time-apps.md)." Replace the
bracketed text so it becomes [Real-Time Apps](real-time-apps.md) to match the
target document name and prior phrasing consistency; keep the rest of the
sentence and the `subscribe()` reference unchanged.
In `@rules/harper/deploying-to-harper-fabric.md`:
- Around line 90-92: The Deploy step currently runs "npm run deploy" but does
not pass the required secrets; update the GitHub Actions step named "Deploy"
(the step that runs npm run deploy) to map the repository secrets into its
environment by adding env entries for CLI_TARGET, CLI_TARGET_USERNAME, and
CLI_TARGET_PASSWORD that reference the corresponding secrets (e.g., ${{
secrets.CLI_TARGET }}, ${{ secrets.CLI_TARGET_USERNAME }}, ${{
secrets.CLI_TARGET_PASSWORD }}), so the deploy script can access them at
runtime.
In `@rules/harper/vector-indexing.md`:
- Around line 93-99: The snippet's _generateOllamaEmbedding function references
undefined symbols (ollama and OLLAMA_EMBEDDING_MODEL); add/import the Ollama
client and define the embedding model constant referenced by that function
(e.g., create or import an ollama client instance named "ollama" and
export/declare "OLLAMA_EMBEDDING_MODEL") so the example is self-contained and
copy-paste runnable; update the top of the example to show the
import/initialization for the Ollama client and the OLLAMA_EMBEDDING_MODEL
constant used by _generateOllamaEmbedding.
- Around line 129-133: The curl command has malformed header quoting: the -H
"accept: application/json line is missing a closing quote (and proper line
continuation), causing the command to fail; edit the curl block so each -H uses
a complete quoted string (e.g., -H "Accept: application/json") and ensure each
continued line ends with a backslash, keeping the POST target
"http://localhost:9926/ProductSearch/" and the JSON body '-d '{"prompt": "shorts
for the gym"}'' unchanged.
In `@skills/harper-best-practices/SKILL.md`:
- Around line 26-34: Rename the top-level heading "## Steps" to "## How It
Works" and add a new top-level "## Examples" section below it (populate with at
least one short example or placeholder) so the skill follows the required
top-level sections ("When to Use", "How It Works", "Examples"); update any
internal links or references that pointed to "## Steps" to use "## How It Works"
and ensure the new "## Examples" header is present and not empty.
---
Nitpick comments:
In `@rules/harper/extending-tables.md`:
- Line 36: Update the documentation line that instructs calling the parent
implementation to use call-style notation instead of bracket notation: replace
the ambiguous "Always call `super[method]`" phrasing with explicit call-style
wording such as "Always call `super.method(...)`" and mention the HTTP handler
names (`get`, `post`, `put`, `patch`, `delete`) so readers know to call e.g.
`super.get(...)`, `super.post(...)`, etc.; keep the guidance about calling super
unless replacing behavior entirely.
In `@rules/harper/using-blob-datatype.md`:
- Line 29: Change the phrasing in the streaming guidance to be more specific:
replace "very large files" with a tighter term such as "large files" or
"multi-GB files" in the sentence that references createBlob(), e.g., update the
line that currently reads "For very large files, pass a stream to `createBlob()`
to avoid loading the entire file into memory." to use your chosen phrase so it
reads "For large files, pass a stream to `createBlob()`..." or "For multi-GB
files, pass a stream to `createBlob()`..." to improve clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 219b7f60-1ee2-4f82-ab5c-eba878dce973
📒 Files selected for processing (21)
AGENTS.mdREADME.mdrules/harper/adding-tables-with-schemas.mdrules/harper/automatic-apis.mdrules/harper/caching.mdrules/harper/checking-authentication.mdrules/harper/creating-a-fabric-account-and-cluster.mdrules/harper/creating-harper-apps.mdrules/harper/custom-resources.mdrules/harper/defining-relationships.mdrules/harper/deploying-to-harper-fabric.mdrules/harper/extending-tables.mdrules/harper/handling-binary-data.mdrules/harper/programmatic-table-requests.mdrules/harper/querying-rest-apis.mdrules/harper/real-time-apps.mdrules/harper/serving-web-content.mdrules/harper/typescript-type-stripping.mdrules/harper/using-blob-datatype.mdrules/harper/vector-indexing.mdskills/harper-best-practices/SKILL.md
✅ Files skipped from review due to trivial changes (2)
- rules/harper/creating-a-fabric-account-and-cluster.md
- rules/harper/custom-resources.md
🚧 Files skipped from review as they are similar to previous changes (10)
- AGENTS.md
- README.md
- rules/harper/defining-relationships.md
- rules/harper/serving-web-content.md
- rules/harper/querying-rest-apis.md
- rules/harper/checking-authentication.md
- rules/harper/typescript-type-stripping.md
- rules/harper/programmatic-table-requests.md
- rules/harper/real-time-apps.md
- rules/harper/creating-harper-apps.md
| - **Update Record (Partial)**: `PATCH /{TableName}/{id}` | ||
| - **Delete All/Filtered Records**: `DELETE /{TableName}/` | ||
| - **Delete Single Record**: `DELETE /{TableName}/{id}` | ||
| 3. **Use Automatic WebSockets**: Connect to `ws://your-harper-instance/{TableName}` to receive events whenever updates are made to that table. This is the easiest way to add real-time capabilities. For more complex needs, see [Real-time Applications](real-time-apps.md). |
Contributor
There was a problem hiding this comment.
Prefer secure WebSocket endpoint guidance.
Line 32 currently promotes ws:// without a security caveat. Please document wss:// as the production default and keep ws:// only for local/dev examples.
🔐 Suggested wording
-3. **Use Automatic WebSockets**: Connect to `ws://your-harper-instance/{TableName}` to receive events whenever updates are made to that table. This is the easiest way to add real-time capabilities. For more complex needs, see [Real-time Applications](real-time-apps.md).
+3. **Use Automatic WebSockets**: Connect to `wss://your-harper-instance/{TableName}` in production (or `ws://localhost:...` in local development) to receive events whenever updates are made to that table. This is the easiest way to add real-time capabilities. For more complex needs, see [Real-time Applications](real-time-apps.md).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/harper/automatic-apis.md` at line 32, Update the "Use Automatic
WebSockets" line to recommend secure WebSockets by default: replace the
production example `ws://your-harper-instance/{TableName}` with
`wss://your-harper-instance/{TableName}` and add a short note that `ws://`
should only be used for local/dev testing; keep `ws://` examples only in a
local/dev subsection or parenthetical so the guidance clearly distinguishes
production (`wss://`) from development (`ws://`).
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
1 participant
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.
What Changed
This PR adds a
harper-best-practicesskill and corresponding ruleset. This is based on the dedicated Harper Skills repository.Why This Change
Harper is an open source, enterprise application platform and many of our users are leveraging Claude for development. Inclusion in this project would greatly benefit the Harper community.
Testing Done
node tests/run-all.js)Type of Change
fix:Bug fixfeat:New featurerefactor:Code refactoringdocs:Documentationtest:Testschore:Maintenance/toolingci:CI/CD changesSecurity & Quality Checklist
Documentation
Summary by cubic
Adds the
harper-best-practicesskill and 18 Harper rules spanning schema design, automatic APIs, authentication, data handling, real-time, and Fabric deployment. Updates docs to reflect 109 skills and lists the new Harper skill.skills/harper-best-practices/SKILL.mdand 18 rules inrules/harper/*covering: schemas/relationships, automatic REST/WebSocket + querying, custom resources/extends, programmatic tables/caching, auth (sessions + JWT/refresh), blobs/binary + vector search, real-time, web serving with@harperfast/vite-plugin, TS type stripping, app creation, and Fabric account/deploy.AGENTS.mdandREADME.mdupdated to 109 skills; skills list includes Harper; examples useharperdb.Written for commit 285878f. Summary will update on new commits.
Summary by CodeRabbit