# Contributing

Thanks for looking at `chroma-client`. This document is the short version of
how changes get made here: what the workflow is, what a change has to prove,
and what the code is expected to look like.

## Getting set up

You need the [MoonBit toolchain](https://www.moonbitlang.com/download/) and
nothing else — no Chroma server, no Docker. Install it with the official
script for your system:

```bash
# Linux & macOS, from the repository root
sh scripts/install-moonbit.sh
export PATH="$HOME/.moon/bin:$PATH"
```

```powershell
# Windows, in PowerShell
Set-ExecutionPolicy RemoteSigned -Scope CurrentUser; irm https://cli.moonbitlang.com/install/powershell.ps1 | iex
```

Then check what you got and refresh the dependency index:

```bash
moon version --all      # must match scripts/moon-version.sh
moon update
```

`scripts/install-moonbit.sh` installs the toolchain and then checks it against
`scripts/moon-version.sh`; CI runs the same script (`.github/workflows/ci.yml`,
and `.cnb/Dockerfile` for CNB).

The module declares `preferred_target = "wasm"`, but `native` is the target CI
uses for tests (the async transport needs a runtime that supports
`moonbitlang/async`; `wasm-gc` does not exist yet).

## Before you open a PR

Run the same script CI runs:

```bash
sh scripts/ci.sh
```

That is, in order: `moon fmt --check`, the README code-block check,
`moon check --target native --deny-warn`, `moon info` (plus a check that the
generated `.mbti` files did not drift), `moon test --target native --deny-warn
--enable-coverage`, the coverage reports, the coverage gate, and debug +
release builds. A single step on its own:

```bash
sh scripts/ci.sh fmt check     # just formatting and type-checking
sh scripts/ci.sh test coverage # just tests and coverage
sh scripts/ci.sh gate          # just the coverage threshold
sh scripts/ci.sh docs          # just the README code-block check
```

Every step has to be green. `moon check --deny-warn` means warnings are
errors: if you introduce one, fix it rather than suppressing it.

## Tests

Two layers, and a change usually needs at least the first.

**Unit tests** live next to the code, in `*_test.mbt` (blackbox) and
`*_wbtest.mbt` (whitebox) files, and run without any server. They drive the
client through `MemoryTransport`, which replays scripted responses. Prefer
plain assertions (`assert_eq`) for results that are stable; for structured
debugging output, derive `Debug` and use `debug_inspect` snapshot tests, and
refresh them with `moon test --update`.

**Async unit tests** live in `scripted_async_wbtest.mbt`, next to the
`ScriptedAsyncTransport` double in `scripted_transport_wbtest.mbt`. They exist
because `MemoryTransport` cannot drive the `_async` methods: those take an
`AsyncTransport`, and an `async` call cannot be made from a synchronous
implementation. The doubles are private to the package (whitebox), so they do
not widen the public API — use them for anything that has to exercise the
request the `_async` methods build. `FailingTransport` (the synchronous double
that raises, which `MemoryTransport` cannot do) is declared in
`scripted_transport_wbtest.mbt` as well; `transport_doubles_wbtest.mbt` only
tests it.

**A `_test.mbt` file is only a test package if some `///|` block in it opens
with something the package parser recognises** (`test "..."`, `async test`,
`pkgtype`, `import`, ...). `guard_test.mbt` still starts with a bare doc
comment block, and when that file gains a second one — a `///` block that
carries no title — the package falls back to "the whole thing is a program,
main is missing" and type-checks every test against the *package* instead of
against `Result`. The error text is a wall of `Constr Type Mismatch(constructor
Ok)` pointing at every `match` in the file, which reads like a type error in
the tests and is actually a linkage complaint. If it does not reproduce
locally, drop the incremental build (`rm -rf _build`) before believing the
green run: the cached package graph is what the CI run starts from too.

**Assert on error variants through `try_catch`, not `is Err(...)`.**
`try_catch` (in `client_test.mbt`) and `try_async` (in `scripted_async_wbtest.mbt`)
turn a raised `ChromaError` into `Result[Unit, ChromaError]`; the test then
`match`es on it. `error is Err(InvalidArgument(_))` does not work: the
`Err(...)` is read as a constructor pattern for `ChromaError` itself, and
`InvalidArgument` is not in scope at that position.

**Live tests** are `chroma_wbtest.mbt` (whitebox: they share the root package
with the client, so they drive its real `AsyncHttpTransport`). They run
against a real Chroma server and skip entirely when `CHROMA_ENDPOINT` is
unset — that is why `scripts/ci.sh` works with no Docker. To run them:

```bash
sh scripts/integration.sh      # starts chromadb/chroma, runs, tears down
```

or against a server you already have:

```bash
CHROMA_ENDPOINT=http://localhost:8000 \
  moon test --target native -p SundayT1998/chroma-client
```

They matter because scripted responses only prove the client agrees with
itself: the reshape of the record body to Chroma's columnar format, the bare
integer the count endpoints return, and the `new_metadata` field on
`modify_collection` were all found by a live server, not by the unit suite.
If your change touches the wire format, add or adjust a live test.

Test names are sentences, not paths: a `*_test.mbt` name starts with the method
it covers (`peek posts a bounded limit ...`), and a `*_wbtest.mbt` name says
what the wire sees or what the live server answered (`peek returns at most the
limit it was given`). The double's name never appears in the test name.

The API shape follows chroma-go's `pkg/api/v2`; when adding a method, mirror
the Go name and options so a reader can map one to the other, and update the
mapping table in the README and [`chroma-api-coverage.md`](chroma-api-coverage.md).

Set `CHROMA_TEST_RUN_ID` when several jobs share one server; each run
namespaces its collection names with it so parallel runs cannot collide.

MoonBit's test runner executes tests concurrently, which rules out asserting on
a database-wide number in `default_database`: `collections_count` and the
listing see every other test creating and dropping collections mid-assertion
(this is what made `count_collections counts ...` fail intermittently in CI).
Such a test uses `isolated_client`, which creates a tenant/database of its own
through `create_database_async` and works there. Chroma does not create a
database on demand — writing into a missing one is a 400 — so isolation needs
that call, not just a different `Config::database`.

## Coverage

`scripts/coverage_gate.sh` enforces the core-path target (>= 80% by default)
and runs as the `gate` step of `scripts/ci.sh`, after the coverage report.
"Core path" means every instrumented file except two:

- `cmd/main` — the demo executable, exercised by running it against a live
  server, not by the unit suite;
- `async_http_transport.mbt` — the real socket transport, which only the
  integration job can cover, because it needs a server to talk to.

Everything else — request building, decoding, error mapping, retries, URL
handling — counts. The gate reads the cobertura report, so run
`moon test --enable-coverage` and `moon coverage report -f cobertura` first if
you invoke it on its own:

```bash
sh scripts/ci.sh test coverage gate
```

A red gate means a new function has no test. Add one in the file next to the
code (`*_test.mbt` for the synchronous API, `*_wbtest.mbt` when it needs the
async doubles), rather than lowering the threshold.

## Performance

`bench_test.mbt` measures the cost of serializing a large record batch; it is
skipped unless `CHROMA_BENCH=1` is set, so ordinary runs do not pay for it.

```bash
CHROMA_BENCH=1 sh scripts/ci.sh test
```

The numbers and what they mean for the write path are in
[`docs/serialization-perf.md`](serialization-perf.md). Re-run it before
changing `records_to_columns`, and change that page rather than adding a
second table.

## Code style

- **Blocks.** MoonBit code is organised in blocks separated by `///|`. The
  order of blocks does not matter, so a block can usually be written, moved or
  reviewed on its own. Keep it that way.
- **Doc comments on public API.** Every `pub` item gets a doc comment — the
  first line is a one-sentence summary, then a blank `///` line, then the rest.
  Two names for one value are not two doc comments: if an item only exists to
  spell a call differently, fold it into the original.
  The library's API reference is generated from these with `moon doc`. Write
  down *why* a thing behaves the way it does (a protocol quirk, a rejected
  alternative) rather than restating the signature.
- **Deprecated blocks** live in `deprecated.mbt` in each package.
- **Packages are named by content**, not by Go-style visibility. A package is
  `http`, never `internal/http`. Do not introduce `internal/` or `pkg/`
  directories.
- **README code blocks are backed by `readme_test.mbt`.** Every block in
  `README.mbt.md` has a namespace-qualified copy in `readme_test.mbt`, and
  `moon check --deny-warn` type-checks that copy, so a README that drifts from
  the API fails CI instead of failing a reader. `readme_test.mbt` is listed in
  `moon.pkg`'s `options(targets:)` like the other backend-specific files, so a
  `wasm-gc` build of the README examples does not pull in the sockets and
  timers of `moonbitlang/async`. When a block changes in the README, change it
  there too.

  The README's fences still say `moonbit nocheck`, and that is not a loophole:
  `readme_test.mbt` is what is compiled, so the marker only says "this block is
  compiled *elsewhere*". It is also what `moon fmt` does — the formatter adds
  the marker to every block that is not a self-contained program, which is all
  of them (they are snippets that use the package's own names). `scripts/ci.sh`
  now enforces the half that can be checked mechanically: a block added to the
  README without a copy in `readme_test.mbt` has nothing compiling it, so the
  `docs` step says how many blocks the README publishes and the review asks for
  the same number of new examples in the test file.
- **Format before committing**: `moon fmt`.
- **Interfaces are checked in.** After a change, run `moon info && moon fmt`
  and review the `.mbti` diff. If the diff is empty, the change was not visible
  to users of the package — usually a safe refactor. If it is not empty, make
  sure every line of it is intended, and say so in the PR description.
- **The toolchain build is pinned in `scripts/moon-version.sh`.** `moon info`
  renders the same builtin integer type as `Int` on `0.1.20261012` and as
  `Int32` on `0.1.20261016`. The installer only publishes `latest` and
  `nightly`, so a release number passed to it 403s; `scripts/install-moonbit.sh`
  installs `latest` and then fails when the build is not the pinned one. Install
  through it rather than with a bare `curl | bash`, or `moon info` will disagree
  with CI.
- **Never hand-edit a `.mbti` to silence a diff.** CI does not compare the
  committed file against the runner's renderer for exactly that `Int`/`Int32`
  reason. It runs `moon info` twice and fails only when the second run moves the
  file, which means the file was edited by hand or predates the source — the
  fix is `moon info` and a commit, not a mass rename of `Int32`.
- **Do not hide the builtin integer type behind a type alias.** `moon info`
  inlines aliases, so a `.mbti` generated from a signature spelled with one
  still names the builtin — the alias buys nothing for the diff above, and it
  adds a name that means nothing outside this module. Spell it out.

## Commits and PRs

Commit subjects follow [Conventional Commits](https://www.conventionalcommits.org):

```
feat: add peek to the collection API
fix: decode bare-integer counts
refactor: flatten the integration package into the root package
docs: move contributor docs into docs/
```

The body of a commit and of a PR description should say **what problem the
change solves**, not only what moved. This repository's PRs are read as design
notes, so the useful shape is:

- **现状问题** — what is wrong today, with evidence (a failing test, a live
  server response, a bug the current code cannot express);
- **改动点** — what changed, and why this shape rather than another;
- **放弃方案** — the alternatives considered and why they were dropped;
- **验证** — the commands run and their result.

Link the issue the PR closes (`Closes #10`, `Ref: #3`).

Try to keep a PR to one concern. If you notice unrelated problems while
working, open an issue rather than folding the fix into the same PR — or, if it
is small and clearly related, say so explicitly in the description.

## Reporting a bug

Include the Chroma version, the target backend (`native` / `wasm` / `js`), the
smallest call that reproduces it, and what you expected instead. If the client
rejected something locally, the `InvalidArgument` message names the record or
the argument; paste it. If the server rejected it, paste the `ApiError`
(`status`, `error_code`, `message`) and the request method and path.

## Licensing

Contributions are accepted under Apache-2.0, the license the module already
carries.