docs: expand FUNCTIONAL_REQUIREMENTS with protocol and FFI FRs#40
Conversation
Adds FR-PROTO section covering apps/runtime TypeScript protocol layer (topic registry, method registry, EventEnvelope, ordered delivery) and clarifies FR-FFI coverage for Go and Python bindings. Total FRs: 28 -> 34.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 20 minutes and 41 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Note
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the functional requirements for the phenotype-config project, specifically adding specifications for the Local Bus Protocol and FFI bridges. The review feedback highlights several discrepancies between the new documentation and the existing implementation, such as incomplete topic lists and mismatched function or module names in the FFI sections.
| ## FR-PROTO: Local Bus Protocol (apps/runtime) | ||
|
|
||
| ### FR-PROTO-001: Topic Registry | ||
| **SHALL** define a canonical topic registry (`TOPICS` constant in `apps/runtime/src/protocol/topics.ts`) listing all pub/sub event topic strings: `workspace.opened`, `project.ready`, `session.created`, `session.attached`, `session.attach.started`, `session.attach.failed`, `session.restore.started`, `session.restore.completed`, `session.terminated`, `terminal.spawn.started`, `terminal.spawned`, `terminal.spawn.failed`, `terminal.output`, `terminal.state.changed`, `renderer.switch.started`, `renderer.switch.succeeded`. |
There was a problem hiding this comment.
The list of pub/sub event topics in FR-PROTO-001 is not exhaustive. The TOPICS constant in apps/runtime/src/protocol/topics.ts contains additional topics such as renderer.switch.failed, agent.run.started, approval.requested, etc. Please update the functional requirement to reflect the complete list of topics for accuracy.
| **Code:** `apps/runtime/src/protocol/topics.ts` | ||
|
|
||
| ### FR-PROTO-005: Go FFI Protocol Bridge | ||
| **SHALL** expose the `pheno-ffi-go` crate (`crates/pheno-ffi-go`) as a C ABI that the Go runtime layer can link against; the generated header SHALL include at minimum `pheno_config_get`, `pheno_config_set`, `pheno_flag_get`, `pheno_flag_enable`. |
There was a problem hiding this comment.
FR-PROTO-005 states that pheno_flag_get should be included in the generated header for the Go FFI. However, the crates/pheno-ffi-go/src/lib.rs file does not appear to export a function named pheno_flag_get. It exposes pheno_flag_list, pheno_flag_enable, and pheno_flag_disable. Please clarify if this function is intended to be exposed, or if the documentation should be updated to reflect the actual exposed flag-related functions.
| **Code:** `crates/pheno-ffi-go/` | ||
|
|
||
| ### FR-PROTO-006: Python FFI Protocol Bridge | ||
| **SHALL** expose the `pheno-ffi-python` crate (`crates/pheno-ffi-python`) as a PyO3 extension module importable as `import pheno`; all functions SHALL release the GIL during SQLite I/O. |
There was a problem hiding this comment.
FR-PROTO-006 specifies that the Python FFI module should be importable as import pheno. However, the #[pymodule] attribute in crates/pheno-ffi-python/src/lib.rs defines the module name as phenotype_config. This discrepancy should be resolved to ensure consistency between the documentation and the implementation.
2 participants
Adds FR-PROTO section covering apps/runtime TypeScript protocol layer (topic registry, EventEnvelope, ordered subscriber delivery) which was missing from the existing FR. Also adds FR-PROTO-005/006 clarifying Go and Python FFI bindings. Total FRs: 28 → 34.