feat(m1): skill export + cloud sync fixes#144
Merged
Conversation
New CLI: gradata skill export <name> [--output-dir DIR] [--description STR]
[--category CAT] [--no-meta]
The bet: Claude Skills' "gotchas" section is exactly what graduated
RULE-tier lessons are -- but generated from real corrections instead of
hand-written. This turns a brain into a portable, shippable Skill folder
with valid YAML frontmatter, category-grouped gotchas, and (when
available) injectable meta-principles.
- new module enhancements/skill_export.py reuses _parse_rules from
rule_export so the RULE-only filter and [hooked] marker stripping
stay consistent across exporters
- auto-generated frontmatter description lists rule categories with
defensive 900-char clip (Anthropic 1024 ceiling)
- name slugified for safe folder name + frontmatter alignment
- description quote-escapes preserve YAML validity
- meta-rule loader degrades gracefully on missing system.db / table
24 new tests; full suite 3969 pass (+24, 0 regressions).
Unblocks M4 items 7 and 9 (self-dev Skill, composition Skill) per
plans/swift-toasting-origami.md.
Co-Authored-By: Gradata <noreply@gradata.ai>
…metry Three bugs kept last_sync_at frozen: - cloud/client.py POSTed /brains/sync (path doesn't exist) -> /sync - cloud/sync.py POSTed /v1/telemetry/metrics -> /api/v1/telemetry/metrics - Stop hook never fired cloud sync because Claude Code doesn't call brain.end_session(). Added cloud_sync_tick() helper in _core.py and new _run_cloud_sync step in session_close.py waterfall. Also elevated silent DEBUG failures to WARNING with HTTP status + exc_info so the next failure mode surfaces in run.log. 3945 tests pass. Co-Authored-By: Gradata <noreply@gradata.ai>
- cloud/sync: contribute_corpus posts to /api/v1/corpus/contribute (was /v1/corpus/contribute, would 404 since backend mounts router under /api/v1) - _core: clarifying comment on _BrainStub explaining db_path may not exist for fresh brains and that downstream compute_metrics tolerates that - skill_export: hoist `import re` to module-level (deferred-import pattern is reserved for heavy optional extras per CLAUDE.md) - test_cloud_sync: update assertion to match corrected corpus path Co-Authored-By: Gradata <noreply@gradata.ai>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 Walkthrough
WalkthroughThis PR introduces cloud sync improvements, a new skill export feature for generating Anthropic Claude Skills from graduated rules, and CLI integration. Changes include enhanced telemetry logging, a hook-safe cloud sync entrypoint, skill markdown generation with filesystem output, and updated API endpoint paths for backend alignment. Changes
Sequence DiagramsequenceDiagram
actor User
participant Session as Session<br/>(hooks)
participant Core as cloud_sync_tick<br/>(_core.py)
participant Disk as lessons.md +<br/>system.db
participant Client as CloudClient
participant API as Gradata Cloud<br/>API
User->>Session: Session ends
Session->>Core: cloud_sync_tick(brain_dir, session_num)
Core->>Disk: Read lessons.md
Core->>Disk: Read corrections from system.db
Disk-->>Core: Rules + telemetry data
Core->>Client: _cloud_sync_session via _BrainStub
Client->>API: POST /api/v1/telemetry/metrics
Client->>API: POST /api/v1/corpus/contribute
API-->>Client: Response (or error)
Client-->>Core: Result (or error logged as WARNING)
Core-->>Session: Complete (swallows errors)
Session-->>User: Continue execution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Gradata/src/gradata/cli.py`:
- Around line 1034-1040: Resolve and reuse the same brain_root when constructing
the Brain so lessons/meta don't come from a different directory: call
_resolve_brain_root(args) once (brain_root) and pass that resolved path into the
brain creation/lookup instead of letting _get_brain() default to cwd; update the
call around brain = _get_brain(args) to use the resolved brain_root (or add a
brain_root argument to _get_brain) and then call lessons_path =
brain._find_lessons_path() so precedence follows GRADATA_BRAIN > --brain-dir >
cwd consistently.
In `@Gradata/src/gradata/cloud/sync.py`:
- Around line 223-226: The POST call in sync.py is constructing a double
"/api/v1" by calling self._post("/api/v1/telemetry/metrics", asdict(payload))
even though api_base already includes "/api/v1"; update the call to use the path
"/telemetry/metrics" (or otherwise join api_base and path without duplicating
the version segment) so the request URL becomes {api_base}/telemetry/metrics;
change the invocation in the method where result = self._post(...,
asdict(payload)) accordingly and keep using the same _post method and payload
variable.
- Around line 242-244: The call to self._post is using a duplicated API prefix
("/api/v1/corpus/contribute") even though the backend already mounts the router
under /api/v1; change the endpoint passed to self._post in the code that assigns
result (the line calling self._post("/api/v1/corpus/contribute", {"patterns":
anonymized_patterns})) to use the route without the "/api/v1" prefix (e.g.
"/corpus/contribute") so the request path is correct.
In `@Gradata/src/gradata/enhancements/skill_export.py`:
- Line 199: The current assignment treats whitespace-only description as
explicit and yields an empty desc; change the logic so you first compute the
stripped value and if it's non-empty use it, otherwise call
_auto_description(rules, slug). In other words, update the desc determination
around the variables description, desc, and the call to _auto_description so
that whitespace-only strings fall back to _auto_description (e.g., check
description and description.strip() before choosing the explicit value).
- Around line 102-103: The YAML description is only escaping double quotes
(safe_desc = description.replace('"', '\\"')) which leaves backslashes unescaped
and can produce invalid YAML; update the logic in skill_export.py to first
escape backslashes and then escape quotes (e.g., transform description by
replacing "\" with "\\" before replacing '"' with '\"') and use that result in
lines.append(f'description: "{safe_desc}"') so backslashes in Windows paths or
other content are preserved and do not create invalid YAML.
In `@Gradata/tests/test_cloud_sync.py`:
- Line 132: The test in test_cloud_sync.py asserts call_path ==
"/api/v1/telemetry/metrics" which mirrors the current double-prefix bug; after
fixing sync_metrics in sync.py to use "/telemetry/metrics", update the assertion
to expect "/telemetry/metrics" instead—locate the assertion comparing call_path
in test_cloud_sync.py and change the expected string to "/telemetry/metrics" so
it matches the corrected sync_metrics behavior.
- Line 171: The corpus contribution path assertion uses the wrong positional
index; update the assertion in tests/test_cloud_sync.py to compare the correct
argument from mock_post.call_args (use the same index used for other path
assertions after fixing sync.py, e.g., mock_post.call_args[0][1] ==
"/api/v1/corpus/contribute") so it checks the actual request path sent by the
code under test.
In `@Gradata/tests/test_skill_export.py`:
- Around line 121-125: Extend the test coverage for YAML frontmatter escaping by
adding cases that exercise backslashes and newlines in the description string
produced by export_skill: call export_skill(tmp_path, name="demo",
description='contains \\ backslash') and assert the returned text contains the
backslash escaped (i.e. the frontmatter shows a double backslash sequence), and
call export_skill(tmp_path, name="demo", description="line1\nline2") and assert
the returned text contains a YAML-safe escaped newline (e.g. "\n" or the
serializer's chosen escaped representation) so both backslash and newline
edge-cases are covered; place these assertions alongside or as additional tests
near test_double_quotes_in_description_are_escaped and reference the
export_skill helper in your assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 604d9b3f-7855-4ae2-88ef-04d79e141c76
📒 Files selected for processing (8)
Gradata/src/gradata/_core.pyGradata/src/gradata/cli.pyGradata/src/gradata/cloud/client.pyGradata/src/gradata/cloud/sync.pyGradata/src/gradata/enhancements/skill_export.pyGradata/src/gradata/hooks/session_close.pyGradata/tests/test_cloud_sync.pyGradata/tests/test_skill_export.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest (py3.11)
- GitHub Check: pytest (py3.12)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.439Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
🔇 Additional comments (7)
Gradata/src/gradata/cloud/client.py (1)
132-135: LGTM!The endpoint path change from
/brains/syncto/syncis correctly implemented. The inline comment clearly documents the backend route mapping, and the URL construction (DEFAULT_ENDPOINTalready includes/api/v1) produces the expected full pathPOST /api/v1/sync.Gradata/src/gradata/cloud/sync.py (1)
204-214: LGTM on error logging elevation.Elevating HTTP/network errors from DEBUG to WARNING with status codes and descriptive markers improves observability. This aligns with the PR objective to surface silent failures that previously hid issues like the "last_sync never updates" bug.
Gradata/src/gradata/_core.py (2)
1371-1372: Good improvement: Failure logging now includes stack trace.Elevating to WARNING with
exc_info=Trueensures cloud sync failures are visible in logs with full traceback context, which aids debugging.
1375-1446: LGTM: Well-designed hook-safe cloud sync entrypoint.The implementation correctly:
- Gracefully handles missing
lessons.mdandsystem.db- Uses a minimal
_BrainStubto avoid expensive Brain initialization (migrations, FTS)- Wraps everything in try/except to never block the hook
- Documents the
db_pathedge case for fresh brainsThe comment on lines 1425-1431 clearly explains why the stub passes
db_patheven when the file doesn't exist.Gradata/src/gradata/hooks/session_close.py (2)
340-356: LGTM: Clean integration of cloud sync into session close hook.The implementation correctly:
- Gates on
GRADATA_API_KEYto avoid unnecessary network attempts- Uses the hook-safe
cloud_sync_tickthat doesn't require Brain instantiation- Catches exceptions at WARNING level to never block the hook
- Defaults
session_numberto 0, whichcloud_sync_tickhandles gracefully by skipping DB queries
391-391: Correct placement in the hook workflow.
_run_cloud_syncis called after graduation, pipeline, tree consolidation, pending applications, and brain prompt refresh — ensuring telemetry reflects the fully-processed session state before syncing to cloud.Gradata/src/gradata/cli.py (1)
1297-1319: Skill CLI parser + dispatch wiring looks good
skill exportis fully wired (required subcommand, clear flags, and command map registration), so invocation flow is consistent with other top-level commands.Also applies to: 1360-1360
Comment on lines
+1034
to
+1040
| brain_root = _resolve_brain_root(args) | ||
| lessons_path: Path | None = None | ||
| try: | ||
| brain = _get_brain(args) | ||
| lessons_path = brain._find_lessons_path() | ||
| except Exception: | ||
| lessons_path = None |
There was a problem hiding this comment.
Resolve brain root once to avoid cross-brain exports
Line 1034 resolves brain_root via _resolve_brain_root() (default ./brain), but Lines 1037-1038 resolve lessons_path via _get_brain() (default cwd). That can export rules from one brain while loading meta-principles from another.
Suggested fix
def cmd_skill_export(args):
@@
- brain_root = _resolve_brain_root(args)
- lessons_path: Path | None = None
- try:
- brain = _get_brain(args)
- lessons_path = brain._find_lessons_path()
- except Exception:
- lessons_path = None
+ # Keep one canonical resolution path so rules + meta read from same brain.
+ lessons_path: Path | None = None
+ try:
+ brain = _get_brain(args)
+ brain_root = Path(brain.dir)
+ lessons_path = brain._find_lessons_path()
+ except Exception:
+ brain_root = Path(
+ env_str("GRADATA_BRAIN") or getattr(args, "brain_dir", None) or Path.cwd()
+ )
+ lessons_path = NoneBased on learnings: cli.py brain resolution precedence should be GRADATA_BRAIN > --brain-dir > cwd.
📝 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
| brain_root = _resolve_brain_root(args) | |
| lessons_path: Path | None = None | |
| try: | |
| brain = _get_brain(args) | |
| lessons_path = brain._find_lessons_path() | |
| except Exception: | |
| lessons_path = None | |
| # Keep one canonical resolution path so rules + meta read from same brain. | |
| lessons_path: Path | None = None | |
| try: | |
| brain = _get_brain(args) | |
| brain_root = Path(brain.dir) | |
| lessons_path = brain._find_lessons_path() | |
| except Exception: | |
| brain_root = Path( | |
| env_str("GRADATA_BRAIN") or getattr(args, "brain_dir", None) or Path.cwd() | |
| ) | |
| lessons_path = None |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/cli.py` around lines 1034 - 1040, Resolve and reuse the
same brain_root when constructing the Brain so lessons/meta don't come from a
different directory: call _resolve_brain_root(args) once (brain_root) and pass
that resolved path into the brain creation/lookup instead of letting
_get_brain() default to cwd; update the call around brain = _get_brain(args) to
use the resolved brain_root (or add a brain_root argument to _get_brain) and
then call lessons_path = brain._find_lessons_path() so precedence follows
GRADATA_BRAIN > --brain-dir > cwd consistently.
Comment on lines
+223
to
+226
| # Backend mounts the metrics router under /api/v1 (see | ||
| # cloud/app/main.py → app.include_router(router, prefix="/api/v1") | ||
| # and cloud/app/routes/metrics.py → @router.post("/telemetry/metrics")). | ||
| result = self._post("/api/v1/telemetry/metrics", asdict(payload)) |
There was a problem hiding this comment.
Critical: Double /api/v1 prefix causes incorrect URL construction.
The api_base (default: https://api.gradata.ai/api/v1) already includes /api/v1, but the path /api/v1/telemetry/metrics also includes it. This results in:
https://api.gradata.ai/api/v1/api/v1/telemetry/metrics
The path should be /telemetry/metrics (without the /api/v1 prefix) since api_base already contains the version segment.
🐛 Proposed fix
- # Backend mounts the metrics router under /api/v1 (see
- # cloud/app/main.py → app.include_router(router, prefix="/api/v1")
- # and cloud/app/routes/metrics.py → `@router.post`("/telemetry/metrics")).
- result = self._post("/api/v1/telemetry/metrics", asdict(payload))
+ # Backend mounts the metrics router under /api/v1 (see
+ # cloud/app/main.py → app.include_router(router, prefix="/api/v1")
+ # and cloud/app/routes/metrics.py → `@router.post`("/telemetry/metrics")).
+ # api_base already includes /api/v1, so we only append the route path.
+ result = self._post("/telemetry/metrics", asdict(payload))📝 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
| # Backend mounts the metrics router under /api/v1 (see | |
| # cloud/app/main.py → app.include_router(router, prefix="/api/v1") | |
| # and cloud/app/routes/metrics.py → @router.post("/telemetry/metrics")). | |
| result = self._post("/api/v1/telemetry/metrics", asdict(payload)) | |
| # Backend mounts the metrics router under /api/v1 (see | |
| # cloud/app/main.py → app.include_router(router, prefix="/api/v1") | |
| # and cloud/app/routes/metrics.py → `@router.post`("/telemetry/metrics")). | |
| # api_base already includes /api/v1, so we only append the route path. | |
| result = self._post("/telemetry/metrics", asdict(payload)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/cloud/sync.py` around lines 223 - 226, The POST call in
sync.py is constructing a double "/api/v1" by calling
self._post("/api/v1/telemetry/metrics", asdict(payload)) even though api_base
already includes "/api/v1"; update the call to use the path "/telemetry/metrics"
(or otherwise join api_base and path without duplicating the version segment) so
the request URL becomes {api_base}/telemetry/metrics; change the invocation in
the method where result = self._post(..., asdict(payload)) accordingly and keep
using the same _post method and payload variable.
Comment on lines
+242
to
+244
| # Backend mounts the corpus router under /api/v1 (same prefix as | ||
| # telemetry — see cloud/app/main.py). | ||
| result = self._post("/api/v1/corpus/contribute", {"patterns": anonymized_patterns}) |
There was a problem hiding this comment.
Same double-prefix issue for corpus contribution path.
🐛 Proposed fix
- # Backend mounts the corpus router under /api/v1 (same prefix as
- # telemetry — see cloud/app/main.py).
- result = self._post("/api/v1/corpus/contribute", {"patterns": anonymized_patterns})
+ # Backend mounts the corpus router under /api/v1 (same prefix as
+ # telemetry — see cloud/app/main.py).
+ # api_base already includes /api/v1, so we only append the route path.
+ result = self._post("/corpus/contribute", {"patterns": anonymized_patterns})📝 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
| # Backend mounts the corpus router under /api/v1 (same prefix as | |
| # telemetry — see cloud/app/main.py). | |
| result = self._post("/api/v1/corpus/contribute", {"patterns": anonymized_patterns}) | |
| # Backend mounts the corpus router under /api/v1 (same prefix as | |
| # telemetry — see cloud/app/main.py). | |
| # api_base already includes /api/v1, so we only append the route path. | |
| result = self._post("/corpus/contribute", {"patterns": anonymized_patterns}) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/cloud/sync.py` around lines 242 - 244, The call to
self._post is using a duplicated API prefix ("/api/v1/corpus/contribute") even
though the backend already mounts the router under /api/v1; change the endpoint
passed to self._post in the code that assigns result (the line calling
self._post("/api/v1/corpus/contribute", {"patterns": anonymized_patterns})) to
use the route without the "/api/v1" prefix (e.g. "/corpus/contribute") so the
request path is correct.
Comment on lines
+102
to
+103
| safe_desc = description.replace('"', '\\"') | ||
| lines.append(f'description: "{safe_desc}"') |
There was a problem hiding this comment.
Escape backslashes in YAML description
Line 102 only escapes ". In double-quoted YAML, unescaped \ can create invalid escapes or mutate content (e.g., Windows-style paths). This can break frontmatter parsing.
Suggested fix
- safe_desc = description.replace('"', '\\"')
+ safe_desc = (
+ description.replace("\\", "\\\\")
+ .replace('"', '\\"')
+ .replace("\n", "\\n")
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/enhancements/skill_export.py` around lines 102 - 103, The
YAML description is only escaping double quotes (safe_desc =
description.replace('"', '\\"')) which leaves backslashes unescaped and can
produce invalid YAML; update the logic in skill_export.py to first escape
backslashes and then escape quotes (e.g., transform description by replacing "\"
with "\\" before replacing '"' with '\"') and use that result in
lines.append(f'description: "{safe_desc}"') so backslashes in Windows paths or
other content are preserved and do not create invalid YAML.
| rules = _parse_rules(Path(brain_root), lessons_path=lessons_path) | ||
| rules = _filter_rules(rules, category) | ||
| metas = _load_meta_principles(Path(brain_root)) if include_meta else [] | ||
| desc = description.strip() if description else _auto_description(rules, slug) |
There was a problem hiding this comment.
Whitespace-only explicit descriptions should fall back to auto description
Line 199 treats " " as explicit input and emits an empty frontmatter description. That should fall back to _auto_description(...) to keep a usable skill descriptor.
Suggested fix
- desc = description.strip() if description else _auto_description(rules, slug)
+ desc = (
+ description.strip()
+ if (description is not None and description.strip())
+ else _auto_description(rules, slug)
+ )📝 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
| desc = description.strip() if description else _auto_description(rules, slug) | |
| desc = ( | |
| description.strip() | |
| if (description is not None and description.strip()) | |
| else _auto_description(rules, slug) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/src/gradata/enhancements/skill_export.py` at line 199, The current
assignment treats whitespace-only description as explicit and yields an empty
desc; change the logic so you first compute the stripped value and if it's
non-empty use it, otherwise call _auto_description(rules, slug). In other words,
update the desc determination around the variables description, desc, and the
call to _auto_description so that whitespace-only strings fall back to
_auto_description (e.g., check description and description.strip() before
choosing the explicit value).
| mock_post.assert_called_once() | ||
| call_path = mock_post.call_args[0][0] | ||
| assert call_path == "/telemetry/metrics" | ||
| assert call_path == "/api/v1/telemetry/metrics" |
There was a problem hiding this comment.
Test assertions match the double-prefix bug in sync.py.
When the path in sync_metrics() is corrected to /telemetry/metrics, this test assertion will need to be updated accordingly.
🛠️ Fix after sync.py is corrected
- assert call_path == "/api/v1/telemetry/metrics"
+ assert call_path == "/telemetry/metrics"📝 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
| assert call_path == "/api/v1/telemetry/metrics" | |
| assert call_path == "/telemetry/metrics" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/tests/test_cloud_sync.py` at line 132, The test in test_cloud_sync.py
asserts call_path == "/api/v1/telemetry/metrics" which mirrors the current
double-prefix bug; after fixing sync_metrics in sync.py to use
"/telemetry/metrics", update the assertion to expect "/telemetry/metrics"
instead—locate the assertion comparing call_path in test_cloud_sync.py and
change the expected string to "/telemetry/metrics" so it matches the corrected
sync_metrics behavior.
| assert result is True | ||
| mock_post.assert_called_once() | ||
| assert mock_post.call_args[0][0] == "/corpus/contribute" | ||
| assert mock_post.call_args[0][0] == "/api/v1/corpus/contribute" |
There was a problem hiding this comment.
Same adjustment needed for corpus contribution path assertion.
🛠️ Fix after sync.py is corrected
- assert mock_post.call_args[0][0] == "/api/v1/corpus/contribute"
+ assert mock_post.call_args[0][0] == "/corpus/contribute"📝 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
| assert mock_post.call_args[0][0] == "/api/v1/corpus/contribute" | |
| assert mock_post.call_args[0][0] == "/corpus/contribute" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/tests/test_cloud_sync.py` at line 171, The corpus contribution path
assertion uses the wrong positional index; update the assertion in
tests/test_cloud_sync.py to compare the correct argument from
mock_post.call_args (use the same index used for other path assertions after
fixing sync.py, e.g., mock_post.call_args[0][1] == "/api/v1/corpus/contribute")
so it checks the actual request path sent by the code under test.
Comment on lines
+121
to
+125
| def test_double_quotes_in_description_are_escaped(self, tmp_path: Path) -> None: | ||
| text = export_skill(tmp_path, name="demo", description='He said "hi" loudly') | ||
| # Ensure the quote is backslash-escaped so YAML stays valid | ||
| assert r'description: "He said \"hi\" loudly"' in text | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add regression coverage for YAML escape edge cases
This test currently verifies quotes only. Please also cover backslashes/newlines in descriptions so frontmatter serialization regressions are caught.
Suggested test additions
class TestExportSkill:
@@
def test_double_quotes_in_description_are_escaped(self, tmp_path: Path) -> None:
text = export_skill(tmp_path, name="demo", description='He said "hi" loudly')
# Ensure the quote is backslash-escaped so YAML stays valid
assert r'description: "He said \"hi\" loudly"' in text
+
+ def test_backslashes_and_newlines_in_description_are_escaped(self, tmp_path: Path) -> None:
+ text = export_skill(tmp_path, name="demo", description="Path C:\\new\\skill\nnext line")
+ assert r'description: "Path C:\\new\\skill\nnext line"' in text🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Gradata/tests/test_skill_export.py` around lines 121 - 125, Extend the test
coverage for YAML frontmatter escaping by adding cases that exercise backslashes
and newlines in the description string produced by export_skill: call
export_skill(tmp_path, name="demo", description='contains \\ backslash') and
assert the returned text contains the backslash escaped (i.e. the frontmatter
shows a double backslash sequence), and call export_skill(tmp_path, name="demo",
description="line1\nline2") and assert the returned text contains a YAML-safe
escaped newline (e.g. "\n" or the serializer's chosen escaped representation) so
both backslash and newline edge-cases are covered; place these assertions
alongside or as additional tests near
test_double_quotes_in_description_are_escaped and reference the export_skill
helper in your assertions.
This was referenced May 1, 2026
Gradata
added a commit
that referenced
this pull request
May 1, 2026
Critical: - cloud/sync.py: fix double /api/v1 prefix on telemetry + corpus paths Major: - cli.py: resolve brain_root once for skill export consistency - skill_export.py: escape backslashes in YAML descriptions - skill_export.py: whitespace-only desc falls back to auto - implicit_feedback.py: negative signals win over approval on conflict - inject_brain_rules.py: harden MAX_RULES int parse against malformed env Tests: - update assertions for corrected /telemetry + /corpus paths - add regression coverage for YAML backslash/newline/whitespace - tighten loose assertions in hooks_intelligence Co-authored-by: Oliver <oliver@spritesai.com>
This was referenced May 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Clean 3-commit rebase of the M1 work onto current
main. Supersedes #143 which had 38 stale commits conflicting with #142 (cloud Phase 2).enhancements/skill_export.py+ CLIgradata skill export <name>— turns graduated RULE-tier lessons into a portable Anthropic Claude Skill folder with valid YAML frontmatter, category-grouped gotchas, and (when available) injectable meta-principles. Reuses_parse_rulesfromrule_exportfor marker stripping consistency./api/v1/telemetry/metrics,/api/v1/corpus/contribute); wires_run_cloud_syncinto the session_close waterfall so Stop hook actually fires telemetry (Claude Code never callsbrain.end_session()so_cloud_sync_sessionwas never executing); elevates silent DEBUG → WARNING with HTTP status + exc_info so the next failure mode surfaces._BrainStubclarifying comment, hoistimport reto module-level (deferred-import pattern reserved for heavy optional extras per CLAUDE.md).Test plan
pytest tests/test_skill_export.py tests/test_cloud_sync.py -q→ 45 passpytest -q -m "not integration"→ 4062 pass, 0 regressionsorigin/main(5635a66), conflicts resolved against feat(cloud): Phase 2 — push/pull, materializer, conflict resolution, telemetry + legacy bloat cleanup #142 (cloud Phase 2)Generated with Gradata