# Response: repoman v0.1 Feedback

**Source feedback:** `bugs/reef-feedback.md`
**Reef version evaluated:** 0.5.9
**Response date:** 2026-04-28
**Status:** All 8 items accepted — implementation plan in `bugs/reef-feedback-implementation-plan.md`

---

## Summary

Thank you for the thorough, well-prioritized feedback. Each item is a real friction point and the proposed API shapes are largely on the mark. We accepted all 8 items; no rejections.

Two corrections worth flagging up front:
- **Item 6 is partially out of date.** `assert_true`, `assert_false`, `assert_not_eq_int`, `assert_not_eq_bool`, and `assert_not_eq_string` already exist on `TestRunner` in v0.5.9. Genuinely missing: `assert_contains` and Result-aware assertions (`assert_ok_*` / `assert_err_*`). We'll add those.
- **Item 4 sketch references `str.substring_from` — that function does not exist.** The correct call in v0.5.9 is `str.substring(s, start, len)`. The implementation will use the real function; just flagging in case repoman has the workaround coded against a placeholder name.

The rest is accurate as filed.

---

## Item-by-item disposition

### 1. TOML/JSON encoders missing — **HIGH** — accepted

Going with the **builder API** (your option b), not the flat `toml_serialize(keys, values, count)`. Reason: the parser's parallel-array representation cannot losslessly round-trip `[[project]]` table-arrays — table-array indices are encoded into key names as a parser-internal detail, not a public contract. A flat serialize would be ambiguous on round-trip.

The builder will be a plain `struct` (not an Active Object — no message-passing overhead needed for single-threaded doc building), modeled on `text.stringbuilder`. JSON gets a parallel `JsonBuilder` with the same surface plus `begin/end_object` / `begin/end_array` for JSON's mandatory structural nesting.

YAML and INI extensions are out of scope for this batch — repoman doesn't need them, and YAML's nested-table indentation is non-trivial.

**Scope:** medium (2-3 days). Sprint 4.

### 2. `io.file` missing rename and fsync — **HIGH** — accepted

Both will be added to `io.file`. `rename` becomes a thin Reef wrapper delegating to the existing `runtime.reef_fs_rename` (same C function `fs.ops.rename` already calls — keeping `fs.ops.rename` in place because the modules have different audiences; doc cross-references on both will resolve the discoverability concern).

`fsync` requires a small C runtime addition (`reef_fs_fsync(path)` in `reef_fs.c`) plus the FFI bridge plus the user-facing wrapper. Path-based, not handle-based — `io.file` has no handle type today and adding one would be a much larger redesign. The path-based fsync does an extra `open`/`close`, but for the atomic-write recipe that's nanoseconds; the actual disk flush dominates.

**Scope:** small. Sprint 2.

### 3. No generic `Result[T, E]` — **MEDIUM** — accepted

Will create `core/result_generic.reef` mirroring `core/option_generic.reef` exactly. Five functions: `is_ok[T,E]`, `is_err[T,E]`, `unwrap_ok[T,E]`, `unwrap_err[T,E]`, `unwrap_or[T,E]`. The existing `core.result` (type-specific monomorphizations) stays untouched for one release; deprecation notice and stdlib internal migration follow in a later release.

**One thing to validate first:** that two-parameter generic enums (`enum Ok(T); Err(E)`) compile cleanly. `option_generic.reef` proves single-param works; we'll smoke-test the two-param case before writing function bodies. If we hit a compiler limit, fallback is `Result[T]` with `E` fixed to `string` — still satisfies the use case you described.

The syntax reference's "Future stdlib" note about generic Result will be updated to "Available in `core.result_generic`" once shipped.

**Scope:** small. Sprint 3.

### 4. `io.path.expand_home` missing — **MEDIUM** — accepted

Pure-Reef addition to `io.path`. Behavior: `~` → `$HOME`, `~/foo` → `$HOME/foo`, anything else → passthrough. If `HOME` is unset, returns the path unchanged (substituting empty would silently corrupt `~/foo` to `/foo`).

`expand_user("~user/...")` is **deferred** to a separate item — it requires a new C runtime helper (`getpwnam` resolution) and adds a `getpwnam_r` thread-safety footnote. Your feedback rated it as a "maybe" and we'd rather ship `expand_home` quickly than couple them. The current `expand_home` returns unchanged on `~user` form so adding `expand_user` later won't break anything.

**Scope:** small. Sprint 1.

### 5. `sys.flag` no subcommand dispatch — **MEDIUM** — accepted in two phases

**Phase 1 (Sprint 2):** `flag_parser_from(args: [string]): FlagParser` — caller slices argv and feeds it in. `parse()` reads from the override array when set, otherwise calls `sys.args` as today. Backward-compatible.

**Phase 2 (Sprint 4):** Full `subcommand_parser` API matching cobra/clap conventions. Has open design questions worth resolving with you before implementation:
- Should subcommand flags be defined before `dispatch` (each subcommand pre-registered with its own parser) or after (cobra-style, parser handed back from dispatch)?
- How do global flags (e.g., `--verbose` before the subcommand token) coexist with per-subcommand flags?
- Does `dispatch` intercept `--help` and route to subcommand-specific usage, or is that the caller's job?

We'll spec Phase 2 against repoman's actual subcommand structure once you reach that part of the build. Phase 1 unblocks you immediately.

**Scope:** Phase 1 small (~1 day), Phase 2 medium (2-3 days after design pass).

### 6. `test.framework` thin assertions — **MEDIUM** — accepted (with correction)

**Already in v0.5.9** (you may have been reading older docs):
- `assert_true(condition, message)` — line 130
- `assert_false(condition, message)` — line 135
- `assert_not_eq_int/bool/string` — lines 178-201

**Genuinely missing, will add (Sprint 1):**
- `assert_contains_string(haystack, needle, message)` — substring; uses existing `str.contains`
- `assert_ok_int/str/bool` — pass when `Result*_Ok(_)`, fail with diagnostic showing the wrapped error
- `assert_err_int/str/bool` — pass when `Result*_Err(_)`

Skipping `ResultFloat` variants for now (rare in error paths; add on demand). Skipping array-membership `assert_contains` until generic Result lands — a typed-array version per element type would be 4-5 procs and isn't worth it before generics. The string substring form covers your actual use case.

A `test_framework.reef` test file (which currently doesn't exist) will be created alongside.

**Scope:** small. Sprint 1.

### 7. TOML parser quirks — **LOW** — accepted in two phases

**Phase 1 (Sprint 1) — silent truncation:** new `toml_parse_status(input, keys, values, max_capacity): int` returning `-1` when input remains unconsumed AND the cap was hit. Existing `toml_parse` and `toml_parse_sized` are untouched (zero breakage). Module docstring will be updated to recommend `toml_parse_status` for production callers.

`json_parse` has the **identical silent-truncation flaw** at a 512-entry cap — we'll mirror the fix as `json_parse_status` in the same release, given the symmetry.

**Phase 2 (Sprint 3) — `TomlDoc` struct:** new ergonomic API replacing the parallel-array threading. Adds `TomlDoc` struct + `toml_parse_doc()` + 4 wrapper getters (`toml_get_doc`, `toml_get_int_doc`, `toml_get_bool_doc`, `toml_has_key_doc`). Coexists with old API for ≥1 release.

**Scope:** Phase 1 small, Phase 2 small-medium.

### 8. `join_path` naming — **LOW** — accepted (bundled)

Adding `fn join(a, b)` as the canonical export, leaving `join_path` as a silent synonym. No deprecation ceremony — Reef has no precedent for one in the stdlib (verified by grep through CHANGELOG; all prior changes were additions or fixes, never renames). A docstring line on `join_path` notes that `join` is preferred.

This is the only `<verb>_path` redundancy in `io.path` — `dirname`/`basename`/`extension` etc. don't repeat the module name. No coordinated naming pass needed elsewhere.

**Scope:** ~15 minutes. Bundled with item 4 in Sprint 1.

---

## Sprint plan summary

Detailed plan in `bugs/reef-feedback-implementation-plan.md`. Top-line sequencing:

| Sprint | Items | Est. effort | Outcome |
|---|---|---|---|
| 1 | 4, 6, 7a, 8 (+JSON mirror) | ~1.5 days | Quick wins; unblocks ~half the friction points |
| 2 | 2, 5a | ~2 days | Runtime additions; subcommand-friendly parser |
| 3 | 3, 7b | ~1.5 days | Generic Result + ergonomic TomlDoc |
| 4 | 1, 5b | ~5 days | Encoders + full subcommand API |

**Defer indefinitely:** `expand_user` (item 4 follow-on, requires runtime work and only a "maybe" in the feedback).

---

## What you can do today

While we land Sprint 1, you can unblock these items in repoman with minimal hand-rolled glue:

- **`expand_home`** — your sketched workaround works; just substitute `str.substring(p, 2, str.length(p) - 2)` for the non-existent `substring_from`.
- **`assert_contains`** — `runner.assert_eq_bool(str.contains(haystack, needle), true, message)` works as a stopgap.
- **Atomic write** — without `fsync`, the write→rename sequence still gives crash-consistency in the common case (just not power-loss durability). Acceptable for v0.1.
- **TOML truncation detection** — call `toml_parse_sized(input, keys, values, max_entries)` with your own cap and assert the returned count is less than the cap — if equal, you may be truncated.
- **Subcommand dispatch** — hand-roll using `sys.args.get_args()` to extract argv[1] as the subcommand, then call `flag_parser()` with a different program name per subcommand. Migrate to `flag_parser_from(slice)` once Sprint 2 ships.

---

## Test plan summary

Each sprint will add tests; the matrix below shows what coverage lands per item:

| Item | Test file | Cases |
|---|---|---|
| 4 | `tests/integration/test_path_expand_home.reef` | bare `~`, `~/foo`, `/abs`, `relative`, empty, `~~double`, `HOME` unset |
| 8 | (same as above) | `path.join(a, b)` produces same result as `path.join_path(a, b)` |
| 6 | `reef-stdlib/test/test_framework.reef` (new) | pass + fail paths for every new assertion, verify diagnostic format |
| 7a | `reef-stdlib/encoding/toml_test.reef` (new) | exact-cap parse returns count (not `-1`), over-cap returns `-1`, under-cap returns count |
| 2 | extends `examples/test_file_io.reef` | rename existence check, atomic-write recipe (write→fsync→rename) |
| 5a | new `tests/integration/test_flag_subcommand.reef` | flat parse unchanged, sliced parse, `tool sub --flag` end-to-end |
| 3 | new `tests/result_generic_test.reef` | `Result[int,string]`, `Result[Point,string]` (struct payload — exercises BUG-034 territory) |
| 7b | extends `toml_test.reef` | `TomlDoc` round-trip, `truncated` flag |
| 1 | new `tests/encoding_toml_writer_test.reef`, `encoding_json_writer_test.reef` | round-trip parse→serialize→parse for all supported types incl. `[[project]]` |
| 5b | extends `test_flag_subcommand.reef` | full `tool sub --flag` end-to-end via `subcommand_parser` |

---

## Open questions for repoman team

1. **Subcommand structure (item 5b):** what does repoman's actual subcommand layout look like? Knowing whether you need global flags before the subcommand token, and whether subcommands can have sub-subcommands (`tool group sub --flag`), shapes the Phase 2 design.
2. **Result error type (item 3):** if generic `Result[T, E]` two-param form hits a compiler limit, are you okay with `Result[T]` where `E = string`? Your feedback example used `string` errors throughout, so this should be a no-op.
3. **Encoder priority (item 1):** TOML or JSON first? You're using both per the feedback, but if one unblocks more of repoman's surface, we'll lead with that.

---

*Reef team — Chris Tusal*