(broken) add json parsing for ai
This commit is contained in:
314
docs/json-glib-integration-plan.md
Normal file
314
docs/json-glib-integration-plan.md
Normal file
@@ -0,0 +1,314 @@
|
||||
# JSON-GLib Integration Plan for Issue #15
|
||||
|
||||
## Overview
|
||||
|
||||
This document outlines the complete plan for integrating json-glib into the CProof (profanity) build system and codebase. The integration replaces the `strstr`-based JSON parsing in [`_parse_ai_response()`](src/ai/ai_client.c:587) with proper json-glib parsing, and adds comprehensive unit tests.
|
||||
|
||||
---
|
||||
|
||||
## Current State Analysis
|
||||
|
||||
### What Has Already Been Done
|
||||
|
||||
| Component | Status | Details |
|
||||
|-----------|--------|---------|
|
||||
| `configure.ac` | DONE | `PKG_CHECK_MODULES([json-glib], [json-glib-1.0 >= 1.6.0])` added at line 101-102 |
|
||||
| `src/ai/ai_client.c` | DONE | `#include <json-glib/json-glib.h>` added at line 24; [`_parse_ai_response()`](src/ai/ai_client.c:587) rewritten using json-glib |
|
||||
| `Dockerfile.ubuntu` | DONE | `libjson-glib-dev` added at line 37 |
|
||||
| `Dockerfile.debian` | DONE | `libjson-glib-dev` added at line 38 |
|
||||
|
||||
### What Is Missing (Root Causes of `bootstrap.sh` Failure)
|
||||
|
||||
1. **`Makefile.am` missing json-glib flags:**
|
||||
- `AM_CFLAGS` at line 272 does not include `$(json-glib_CFLAGS)`
|
||||
- `tests_unittests_unittests_LDADD` at line 299 does not include `$(json-glib_LIBS)`
|
||||
- The main `profanity` binary target also needs `$(json-glib_LIBS)` in its link flags
|
||||
|
||||
2. **No unit tests for `_parse_ai_response()`:**
|
||||
- [`test_ai_client.c`](tests/unittests/test_ai_client.c) has 464 lines of tests but none for the JSON parsing function
|
||||
- The function is `static`, so it cannot be tested directly; a test harness or refactoring is needed
|
||||
|
||||
3. **`bootstrap.sh` failure:**
|
||||
- [`bootstrap.sh`](bootstrap.sh) is a simple script: `mkdir -p m4 && autoreconf -i "$@"`
|
||||
- The failure is caused by `autoreconf` failing due to missing json-glib m4 macros in the `m4/` directory, or the `configure.ac` referencing `PKG_CHECK_MODULES` without the proper `ax_pthread.m4` or json-glib pkg-config files being available in the build environment
|
||||
|
||||
---
|
||||
|
||||
## Detailed Plan
|
||||
|
||||
### 1. Build System Changes
|
||||
|
||||
#### 1.1 `configure.ac` — No Changes Needed
|
||||
|
||||
The json-glib check is already present:
|
||||
|
||||
```autoconf
|
||||
PKG_CHECK_MODULES([json-glib], [json-glib-1.0 >= 1.6.0], [],
|
||||
[AC_MSG_ERROR([json-glib 1.6.0 or higher is required])])
|
||||
```
|
||||
|
||||
This generates the variables `json-glib_CFLAGS` and `json-glib_LIBS` for use in `Makefile.am`.
|
||||
|
||||
#### 1.2 `Makefile.am` — Changes Required
|
||||
|
||||
**File:** [`Makefile.am`](Makefile.am)
|
||||
|
||||
**Change 1: Add json-glib_CFLAGS to AM_CFLAGS (line 272)**
|
||||
|
||||
```makefile
|
||||
# Current (line 272):
|
||||
AM_CFLAGS = @AM_CFLAGS@ -I$(srcdir)/src
|
||||
|
||||
# Change to:
|
||||
AM_CFLAGS = @AM_CFLAGS@ $(json-glib_CFLAGS) -I$(srcdir)/src
|
||||
```
|
||||
|
||||
**Change 2: Add json-glib_LIBS to profanity linking (line 275)**
|
||||
|
||||
The `profanity` binary target needs json-glib_LIBS. Since `profanity_SOURCES` is defined at line 275 and no explicit `profanity_LDADD` exists, we need to add one:
|
||||
|
||||
```makefile
|
||||
# Add after line 275:
|
||||
profanity_LDADD = $(json-glib_LIBS) $(LIBS)
|
||||
```
|
||||
|
||||
**Change 3: Add json-glib_LIBS to test linking (line 299)**
|
||||
|
||||
```makefile
|
||||
# Current (line 299):
|
||||
tests_unittests_unittests_LDADD = -lcmocka
|
||||
|
||||
# Change to:
|
||||
tests_unittests_unittests_LDADD = -lcmocka $(json-glib_LIBS)
|
||||
```
|
||||
|
||||
#### 1.3 Why `bootstrap.sh` Fails
|
||||
|
||||
The [`bootstrap.sh`](bootstrap.sh) script runs `autoreconf -i`, which regenerates the `configure` script from `configure.ac`. The failure occurs because:
|
||||
|
||||
1. **pkg-config dependency:** `PKG_CHECK_MODULES` requires `pkg-config` to be available at configure-time (not build-time). The `autoreconf` step itself should succeed, but the resulting `configure` script will fail if `pkg-config --modversion json-glib-1.0 >= 1.6.0` cannot find the json-glib installation.
|
||||
|
||||
2. **Likely root cause:** The build environment (Docker container or host) does not have `json-glib` development files installed, or `pkg-config` cannot locate the `json-glib-1.0.pc` file.
|
||||
|
||||
**Fix:** Ensure `libjson-glib-dev` (Debian/Ubuntu) or `json-glib` (Arch/Fedora) is installed in the build environment before running `bootstrap.sh`.
|
||||
|
||||
---
|
||||
|
||||
### 2. Source Code Changes
|
||||
|
||||
#### 2.1 `src/ai/ai_client.c` — No Changes Needed
|
||||
|
||||
The [`_parse_ai_response()`](src/ai/ai_client.c:587) function has already been rewritten using json-glib:
|
||||
|
||||
```c
|
||||
static gchar*
|
||||
_parse_ai_response(const gchar* response_json)
|
||||
{
|
||||
// ... json-glib parsing with three fallback strategies:
|
||||
// Try 1: Perplexity /v1/agent format
|
||||
// Try 2: OpenAI /v1/chat/completions format
|
||||
// Try 3: Fallback "text" field
|
||||
}
|
||||
```
|
||||
|
||||
#### 2.2 `src/ai/ai_client.h` — No Changes Needed
|
||||
|
||||
The header file already includes `<glib.h>` and defines all necessary types. No json-glib-specific types are exposed (json-glib is an implementation detail).
|
||||
|
||||
---
|
||||
|
||||
### 3. Test Changes
|
||||
|
||||
#### 3.1 `tests/unittests/test_ai_client.c` — New Tests Required
|
||||
|
||||
The current test file has 464 lines covering provider management, session handling, JSON escaping, and autocomplete. **No tests exist for `_parse_ai_response()`** because it is `static`.
|
||||
|
||||
**Option A: Refactor `_parse_ai_response` to be testable (Recommended)**
|
||||
|
||||
Make the function non-static and add a test wrapper:
|
||||
|
||||
```c
|
||||
// In ai_client.h, add:
|
||||
gchar* ai_parse_ai_response(const gchar* response_json);
|
||||
|
||||
// In ai_client.c, change:
|
||||
// static gchar* _parse_ai_response(...) → gchar* ai_parse_ai_response(...)
|
||||
```
|
||||
|
||||
**Option B: Use a test harness with `extern` declaration**
|
||||
|
||||
Add to test file:
|
||||
```c
|
||||
extern gchar* _parse_ai_response(const gchar* response_json);
|
||||
```
|
||||
|
||||
**Recommended tests to add:**
|
||||
|
||||
| Test Name | Description |
|
||||
|-----------|-------------|
|
||||
| `test_ai_parse_response_null` | NULL input returns NULL |
|
||||
| `test_ai_parse_response_empty` | Empty string returns NULL |
|
||||
| `test_ai_parse_response_invalid_json` | Malformed JSON returns NULL |
|
||||
| `test_ai_parse_response_openai_format` | Valid OpenAI `/v1/chat/completions` response |
|
||||
| `test_ai_parse_response_openai_empty_choices` | Response with empty choices array |
|
||||
| `test_ai_parse_response_perplexity_format` | Valid Perplexity `/v1/agent` response |
|
||||
| `test_ai_parse_response_perplexity_empty_output` | Response with empty output array |
|
||||
| `test_ai_parse_response_text_fallback` | Response with only "text" field |
|
||||
| `test_ai_parse_response_unicode_content` | Content with unicode characters |
|
||||
| `test_ai_parse_response_multiline_content` | Content with newlines and tabs |
|
||||
| `test_ai_parse_response_html_content` | Content with HTML tags |
|
||||
| `test_ai_parse_response_error_format` | Response with "error" field (not "choices") |
|
||||
|
||||
#### 3.2 `tests/unittests/test_ai_client.h` — Update Required
|
||||
|
||||
Add declarations for new test functions:
|
||||
|
||||
```c
|
||||
/* JSON parsing tests */
|
||||
void test_ai_parse_response_null(void** state);
|
||||
void test_ai_parse_response_empty(void** state);
|
||||
void test_ai_parse_response_invalid_json(void** state);
|
||||
void test_ai_parse_response_openai_format(void** state);
|
||||
void test_ai_parse_response_openai_empty_choices(void** state);
|
||||
void test_ai_parse_response_perplexity_format(void** state);
|
||||
void test_ai_parse_response_perplexity_empty_output(void** state);
|
||||
void test_ai_parse_response_text_fallback(void** state);
|
||||
void test_ai_parse_response_unicode_content(void** state);
|
||||
void test_ai_parse_response_multiline_content(void** state);
|
||||
void test_ai_parse_response_html_content(void** state);
|
||||
void test_ai_parse_response_error_format(void** state);
|
||||
```
|
||||
|
||||
#### 3.3 `tests/unittests/unittests.c` — Update Required
|
||||
|
||||
Add new test entries after the existing AI client tests (around line 696):
|
||||
|
||||
```c
|
||||
// After line 696 (test_ai_json_escape_backslash_quote):
|
||||
cmocka_unit_test(test_ai_parse_response_null),
|
||||
cmocka_unit_test(test_ai_parse_response_empty),
|
||||
cmocka_unit_test(test_ai_parse_response_invalid_json),
|
||||
cmocka_unit_test(test_ai_parse_response_openai_format),
|
||||
cmocka_unit_test(test_ai_parse_response_openai_empty_choices),
|
||||
cmocka_unit_test(test_ai_parse_response_perplexity_format),
|
||||
cmocka_unit_test(test_ai_parse_response_perplexity_empty_output),
|
||||
cmocka_unit_test(test_ai_parse_response_text_fallback),
|
||||
cmocka_unit_test(test_ai_parse_response_unicode_content),
|
||||
cmocka_unit_test(test_ai_parse_response_multiline_content),
|
||||
cmocka_unit_test(test_ai_parse_response_html_content),
|
||||
cmocka_unit_test(test_ai_parse_response_error_format),
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 4. Dockerfile Changes
|
||||
|
||||
#### 4.1 `Dockerfile.ubuntu` — No Changes Needed
|
||||
|
||||
`libjson-glib-dev` is already present at line 37.
|
||||
|
||||
#### 4.2 `Dockerfile.debian` — No Changes Needed
|
||||
|
||||
`libjson-glib-dev` is already present at line 38.
|
||||
|
||||
#### 4.3 `Dockerfile.fedora` — Check Required
|
||||
|
||||
Verify `json-glib-devel` is installed. If not, add:
|
||||
|
||||
```dockerfile
|
||||
json-glib-devel \
|
||||
```
|
||||
|
||||
#### 4.4 `Dockerfile.arch` — Check Required
|
||||
|
||||
Verify `json-glib` is installed. If not, add:
|
||||
|
||||
```dockerfile
|
||||
json-glib \
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 5. Bootstrap.sh Fix
|
||||
|
||||
#### 5.1 Current State
|
||||
|
||||
[`bootstrap.sh`](bootstrap.sh):
|
||||
```sh
|
||||
#!/bin/sh
|
||||
mkdir -p m4
|
||||
autoreconf -i "$@"
|
||||
```
|
||||
|
||||
#### 5.2 Root Cause Analysis
|
||||
|
||||
The script itself is correct. The failure is due to one of:
|
||||
|
||||
1. **Missing `json-glib-1.0.pc`**: `pkg-config` cannot find json-glib in the build environment
|
||||
2. **Missing `m4/ax_pthread.m4`**: The `configure.ac` uses `ACX_PTHREAD` which requires the `ax_pthread.m4` macro
|
||||
3. **Missing `m4/` macros**: Other autoconf macros referenced in `configure.ac` are not present
|
||||
|
||||
#### 5.3 Fix Steps
|
||||
|
||||
1. **Ensure json-glib-dev is installed** in the build environment
|
||||
2. **Verify m4 macros exist:**
|
||||
```bash
|
||||
ls -la m4/
|
||||
# Should contain: ax_pthread.m4, ax_valgrind_check.m4, etc.
|
||||
```
|
||||
3. **If m4 macros are missing**, they need to be copied from the system:
|
||||
```bash
|
||||
# For Debian/Ubuntu:
|
||||
cp /usr/share/autoconf-archive/m4/ax_pthread.m4 m4/
|
||||
cp /usr/share/autoconf-archive/m4/ax_valgrind_check.m4 m4/
|
||||
```
|
||||
4. **Test bootstrap.sh manually:**
|
||||
```bash
|
||||
./bootstrap.sh
|
||||
./configure
|
||||
make
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Implementation Order
|
||||
|
||||
```mermaid
|
||||
graph TD
|
||||
A[1. Fix Makefile.am] --> B[2. Verify bootstrap.sh works]
|
||||
B --> C[3. Add test functions to test_ai_client.c]
|
||||
C --> D[4. Update test_ai_client.h]
|
||||
D --> E[5. Register tests in unittests.c]
|
||||
E --> F[6. Verify Dockerfiles have json-glib-dev]
|
||||
F --> G[7. Run make check to verify all tests pass]
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Summary of Changes
|
||||
|
||||
| File | Change Type | Lines Affected |
|
||||
|------|-------------|----------------|
|
||||
| `Makefile.am` | Modify | Line 272 (AM_CFLAGS), Line 275 (add profanity_LDADD), Line 299 (LDADD) |
|
||||
| `src/ai/ai_client.c` | None | Already complete |
|
||||
| `src/ai/ai_client.h` | None | Already complete |
|
||||
| `tests/unittests/test_ai_client.c` | Add | ~200 lines of new test functions |
|
||||
| `tests/unittests/test_ai_client.h` | Add | ~12 new declarations |
|
||||
| `tests/unittests/unittests.c` | Add | ~12 new test entries |
|
||||
| `Dockerfile.ubuntu` | None | Already has libjson-glib-dev |
|
||||
| `Dockerfile.debian` | None | Already has libjson-glib-dev |
|
||||
| `Dockerfile.fedora` | Check | Verify json-glib-devel present |
|
||||
| `Dockerfile.arch` | Check | Verify json-glib present |
|
||||
| `bootstrap.sh` | Diagnose | Fix is environmental, not script change |
|
||||
|
||||
---
|
||||
|
||||
## Verification Checklist
|
||||
|
||||
- [ ] `Makefile.am` updated with `$(json-glib_CFLAGS)` and `$(json-glib_LIBS)`
|
||||
- [ ] `bootstrap.sh` runs successfully
|
||||
- [ ] `./configure` completes without json-glib errors
|
||||
- [ ] `make` compiles without errors
|
||||
- [ ] `make check` runs all tests including new json-glib tests
|
||||
- [ ] All existing tests still pass
|
||||
- [ ] Docker builds succeed with json-glib integration
|
||||
Reference in New Issue
Block a user