Skip to content

feat: workspace context CLI and API-first data model docs#52

Merged
eddietejeda merged 6 commits intomainfrom
feat/context-cli
Apr 23, 2026
Merged

feat: workspace context CLI and API-first data model docs#52
eddietejeda merged 6 commits intomainfrom
feat/context-cli

Conversation

@eddietejeda
Copy link
Copy Markdown
Contributor

Summary

Adds hotdata context commands to sync workspace Markdown with the context API, and updates skill + README so the API is the source of truth for the data model (DATAMODEL) and related docs.

CLI

  • context list — list named contexts; optional --prefix
  • context show <name> — print Markdown to stdout (no local file)
  • context pull <name> — write ./<name>.md from API (--force, --dry-run)
  • context push <name> — upsert from ./<name>.md (--dry-run)

Name validation matches SQL table identifier rules (aligned with runtimedb validate_table_name).

Documentation

  • README: workspace context section and commands table
  • skills/hotdata: SKILL.md, WORKFLOWS.md, MODEL_BUILD.md, DATA_MODEL.template.md — context API only for model storage; ./DATAMODEL.md described as push/pull transport / agent edit surface

Branch

feat/context-cli

Wire workspace /v1/context to local ./<NAME>.md in the current directory.
Validate names with the same rules as SQL table identifiers (aligned with
runtimedb validate_table_name). Pull refuses to overwrite without --force;
push and pull support --dry-run.
Document context list/show/pull/push, DATAMODEL convention, agent workflow
(show before work, push after edits), optional pull sync, and sandbox
promotion to workspace context.
Remove legacy local-path guidance. SKILL, WORKFLOWS, MODEL_BUILD, and
DATA_MODEL.template now treat DATAMODEL workspace context as authoritative;
./DATAMODEL.md is only a CLI push/pull surface.
@sentry
Copy link
Copy Markdown

sentry Bot commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 30.69767% with 149 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/context.rs 32.03% 140 Missing ⚠️
src/main.rs 0.00% 9 Missing ⚠️

📢 Thoughts on this report? Let us know!

claude[bot]
claude Bot previously approved these changes Apr 22, 2026
Comment thread src/context.rs Outdated
Comment thread src/context.rs
);
}

#[cfg(test)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the test suite only covers three validate_context_stem paths. A few cheap additions would meaningfully raise confidence: a name starting with a digit (should fail), a name starting with underscore (should succeed), the exact 128-char boundary (should succeed), a 129-char name (should fail), and a reserved word in uppercase e.g. SELECT (should fail via the lowercase check). (not blocking)

Address PR review: skip GET when non-dry-run pull would refuse overwrite.
Add validate_context_stem tests for leading digit, underscore, length
boundary, and uppercase reserved word.
Comment thread src/context.rs
);
eprintln!(
"{}",
format!("Create ./{name}.md locally, then run: hotdata context push {name}")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Err(_) => unreachable!() silently depends on the invariant that fetch_context only ever returns Err(NOT_FOUND) — all other non-OK statuses currently call process::exit before returning. That invariant is not encoded in the type, so a future change to fetch_context (e.g. propagating additional status codes as Err) would cause a panic with no useful diagnostic here and in the equivalent arm in pull. A named panic message like panic!("unexpected error status from fetch_context") would at least surface a clear message. Same applies to the analogous arm in pull. (not blocking)

Comment thread src/context.rs
if dry_run {
eprintln!(
"{}",
format!("would write {} chars to {}", n_chars, path.display()).dark_grey()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: path.parent() here is always current_dir() (since local_md_path returns <cwd>/<name>.md), so create_dir_all is a guaranteed no-op — the current directory always exists. The block can be removed. (not blocking)

claude[bot]
claude Bot previously approved these changes Apr 22, 2026
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues. The implementation is clean and consistent with the rest of the codebase. Validation logic is correct, error handling follows existing patterns, and test coverage is solid (all boundary cases covered).

…kdir

Replace unreachable Err arms with explicit panic message if fetch_context
ever returns a non-404 error. Remove create_dir_all on pull (path is always
cwd + single filename segment).
@eddietejeda eddietejeda merged commit d50982f into main Apr 23, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant