Antigravity Agent
370 lines
13 KiB
Markdown
370 lines
13 KiB
Markdown
# Code Review: `becomingone` Repository
|
|
|
|
**Reviewed by:** Claude Sonnet 4.6 (`claude-sonnet-4-6`)
|
|
**Review date:** 2026-05-25
|
|
**Rigor level:** HIGH
|
|
**Commit reviewed:** `6061f5c` (tip of main branch)
|
|
|
|
---
|
|
|
|
## Summary
|
|
|
|
This codebase implements a KAIROS-inspired temporal coherence system — a genuinely creative research project. However, a high-rigor review reveals **4 critical bugs that will cause runtime errors**, **2 security vulnerabilities**, and **a number of medium/low severity issues** that undermine the mathematical correctness the project aspires to. Fixes are provided for each.
|
|
|
|
---
|
|
|
|
## 🔴 CRITICAL — Will Cause Runtime Errors
|
|
|
|
### C1. `CoherenceCalculator.calculate()` does not exist
|
|
|
|
**Files:** `becomingone/memory/temporal.py:257`, `temporal.py:342`, `becomingone/witnessing/layer.py:244-246`
|
|
|
|
`TemporalMemory.encode()` and `TemporalMemory.retrieve()` call `self.calculator.calculate(temporal_state)`, but `CoherenceCalculator` (in `core/coherence.py`) has no `calculate` method. The only mutating method is `update(T_tau: complex)`. Calling `.calculate()` will raise `AttributeError` at runtime whenever memory encoding or retrieval is triggered.
|
|
|
|
```python
|
|
# temporal.py:257 — AttributeError at runtime
|
|
coherence = self.calculator.calculate(temporal_state) # BUG
|
|
|
|
# Fix: extract T_tau from the state's metadata, then call update()
|
|
T_tau = temporal_state.metadata.get("T_tau", complex(0, 0))
|
|
coherence = self.calculator.update(T_tau)
|
|
```
|
|
|
|
Same issue at `temporal.py:342` and `witnessing/layer.py:244-246`.
|
|
|
|
---
|
|
|
|
### C2. `TemporalState.phase_history` attribute does not exist
|
|
|
|
**File:** `becomingone/memory/temporal.py:358-359`
|
|
|
|
```python
|
|
if query_state.phase_history and signature.phase_vector: # BUG: AttributeError
|
|
query_phase = query_state.phase_history[-1]
|
|
```
|
|
|
|
`TemporalState` (defined at `core/engine.py:45-53`) has only `phase`, `coherence`, `timestamp`, and `metadata` fields — no `phase_history`. The attribute access will raise `AttributeError`. Because the condition is in an `if`, Python evaluates `query_state.phase_history` first.
|
|
|
|
**Fix:**
|
|
```python
|
|
phase_vec = temporal_state.metadata.get("raw_angles", [])
|
|
if phase_vec and signature.phase_vector:
|
|
query_phase = float(np.mean(phase_vec))
|
|
...
|
|
```
|
|
|
|
---
|
|
|
|
### C3. `TemporalMemory.encode()` silently returns `None`; callers do not check
|
|
|
|
**Files:** `becomingone/memory/temporal.py:263`, `app.py:251`
|
|
|
|
```python
|
|
# temporal.py:263
|
|
if coherence < self.attention_threshold and not force_attention:
|
|
return None # Silent None with no indication to callers
|
|
```
|
|
|
|
In `app.py:251`, the return value is captured as `sig` but never null-checked before (potential) use. Any caller treating the return as a `TemporalSignature` without checking will encounter a `NoneType` error.
|
|
|
|
**Fix:** Return a typed `Optional[TemporalSignature]` and update the docstring contract, **or** raise a meaningful exception. Add null guards in callers:
|
|
```python
|
|
sig = memory.encode(state, context={"trigger": prompt}, force_attention=True)
|
|
if sig is not None:
|
|
... # use sig
|
|
```
|
|
|
|
---
|
|
|
|
### C4. Test suite calls `async def temporalize()` without `await`
|
|
|
|
**File:** `tests/test_core.py:35, 41`
|
|
|
|
```python
|
|
def test_temporalize(self):
|
|
engine = KAIROSTemporalEngine()
|
|
result = engine.temporalize(0.1) # BUG: returns a coroutine, not a TemporalState
|
|
self.assertIsNotNone(result) # Always passes! Coroutine is not None.
|
|
```
|
|
|
|
`KAIROSTemporalEngine.temporalize` is `async def`. Calling it without `await` returns a coroutine object — the test asserts it's not None (a coroutine never is), so it passes but tests nothing. The same pattern appears at line 41.
|
|
|
|
**Fix:**
|
|
```python
|
|
import asyncio
|
|
|
|
def test_temporalize(self):
|
|
engine = KAIROSTemporalEngine()
|
|
result = asyncio.run(engine.temporalize("test phrase"))
|
|
self.assertIsInstance(result, TemporalState)
|
|
```
|
|
|
|
---
|
|
|
|
## 🟠 HIGH — Security & Concurrency
|
|
|
|
### H1. XSS Vulnerability: Raw LLM output injected via `innerHTML`
|
|
|
|
**File:** `app.py:126, 132`
|
|
|
|
```javascript
|
|
// Both lines inject unescaped LLM response text into the DOM
|
|
document.getElementById('response-minimax').innerHTML = d.emissaries.minimax;
|
|
document.getElementById('response-moonshot').innerHTML = d.emissaries.moonshot;
|
|
```
|
|
|
|
If either Minimax or Moonshot returns a response containing `<script>alert(1)</script>` or `<img src=x onerror=...>`, it will execute in the user's browser. LLM outputs are untrusted user-visible data.
|
|
|
|
**Fix:**
|
|
```javascript
|
|
// Use textContent instead of innerHTML
|
|
document.getElementById('response-minimax').textContent = d.emissaries.minimax;
|
|
document.getElementById('response-moonshot').textContent = d.emissaries.moonshot;
|
|
```
|
|
|
|
---
|
|
|
|
### H2. Race condition on Fieldprint Ledger under concurrent Flask requests
|
|
|
|
**Files:** `becomingone/memory/ledger.py:95`, `app.py:269`
|
|
|
|
The Flask app is started with `threaded=True`. The global `engine` and `memory` objects are shared across threads without any locking. More critically, `seal_signature()` in `ledger.py` performs a non-atomic read-then-write on the JSONL file:
|
|
|
|
```python
|
|
# ledger.py:73-96 — read last root, then append — NOT atomic
|
|
prev_root = get_last_merkle_root(filepath) # reads
|
|
...
|
|
with open(filepath, "a") as f: # writes
|
|
f.write(json.dumps(sealed_record) + "\n")
|
|
```
|
|
|
|
Two concurrent requests can read the same `prev_root`, compute two different next roots based on the same previous root, and corrupt the hash chain — defeating the entire cryptographic integrity guarantee.
|
|
|
|
**Fix:** Use a file-level lock (e.g., `threading.Lock` or `filelock`) around the read-compute-write operation:
|
|
```python
|
|
import threading
|
|
_ledger_lock = threading.Lock()
|
|
|
|
def seal_signature(signature_dict, filepath=LEDGER_FILE):
|
|
with _ledger_lock:
|
|
prev_root = get_last_merkle_root(filepath)
|
|
...
|
|
```
|
|
|
|
---
|
|
|
|
### H3. `asyncio.new_event_loop()` leak in Flask request handler
|
|
|
|
**File:** `app.py:229-242`
|
|
|
|
```python
|
|
loop = asyncio.new_event_loop()
|
|
asyncio.set_event_loop(loop)
|
|
emissaries_dict = loop.run_until_complete(gather_emissaries())
|
|
# ... loop.run_until_complete(process_stream())
|
|
# loop.close() is NEVER CALLED
|
|
```
|
|
|
|
Every HTTP request creates a new event loop that is never closed. This leaks OS resources (file descriptors, thread references) proportional to request count.
|
|
|
|
**Fix:** Use `asyncio.run()` (Python 3.7+) which handles loop lifecycle:
|
|
```python
|
|
emissaries_dict = asyncio.run(gather_emissaries())
|
|
asyncio.run(process_stream())
|
|
```
|
|
|
|
Or close explicitly: `loop.close()` in a `finally` block.
|
|
|
|
---
|
|
|
|
## 🟡 MEDIUM — Mathematical Correctness & Design
|
|
|
|
### M1. `datetime.utcnow()` is deprecated (raises in Python 3.14)
|
|
|
|
**Files:** `core/engine.py:48`, `core/phase.py:47`, `witnessing/layer.py:95`
|
|
|
|
```python
|
|
# Deprecated since Python 3.12, will raise DeprecationWarning
|
|
timestamp: datetime = field(default_factory=datetime.utcnow)
|
|
```
|
|
|
|
**Fix:**
|
|
```python
|
|
from datetime import datetime, timezone
|
|
timestamp: datetime = field(default_factory=lambda: datetime.now(timezone.utc))
|
|
```
|
|
|
|
---
|
|
|
|
### M2. Brownian noise added before normalization — physically incorrect
|
|
|
|
**File:** `becomingone/core/engine.py:92-99`
|
|
|
|
```python
|
|
# Stochastic noise is added to similarity
|
|
similarity += noise
|
|
|
|
# Then similarity is normalized — this CANCELS the noise's effect
|
|
magnitude = np.abs(similarity)
|
|
if magnitude > 0:
|
|
similarity = similarity / magnitude # noise term is divided out!
|
|
```
|
|
|
|
Adding noise before the normalization step means the noise shifts the unit vector's direction but not its magnitude. The stochastic energy injection intended to simulate Brownian motion is effectively lost by the subsequent L2-normalization. This is an important distinction from the cited physics model.
|
|
|
|
**Fix:** If the intent is additive stochastic resonance on the output magnitude:
|
|
```python
|
|
# Compute normalized inner product first
|
|
magnitude = np.abs(similarity)
|
|
if magnitude > 0:
|
|
similarity = similarity / magnitude
|
|
# Then add noise to the result
|
|
similarity += noise
|
|
```
|
|
|
|
---
|
|
|
|
### M3. `CollapseCondition.evaluate()` return value is inconsistent with `collapsed` property
|
|
|
|
**File:** `becomingone/core/coherence.py:344-355`
|
|
|
|
When already collapsed and coherence drops below threshold, `evaluate()` returns `(False, message)` but does **not** reset `self._collapsed`. The `.collapsed` property remains `True` while the method returned `False`. This inconsistency means:
|
|
|
|
```python
|
|
collapsed, msg = condition.evaluate(0.1) # returns (False, ...)
|
|
bool(condition.collapsed) # returns True ← contradiction
|
|
```
|
|
|
|
**Fix:** Define and document the intended semantics clearly. If collapse is permanent (irreversible), `evaluate()` should return `(True, ...)` when already collapsed regardless of current coherence. If it's transient, `self._collapsed` should be reset accordingly.
|
|
|
|
---
|
|
|
|
### M4. `asyncio` listed as a PyPI dependency
|
|
|
|
**File:** `requirements.txt:13`
|
|
|
|
```
|
|
asyncio>=3.4.3 # Invalid: asyncio is stdlib, not a PyPI package
|
|
```
|
|
|
|
`asyncio` has been part of the Python standard library since 3.4 and is not installable from PyPI. Installing this will either silently succeed (installing an obsolete shim) or fail. Remove this line.
|
|
|
|
---
|
|
|
|
### M5. `EmissaryConfig.omega` uses a truncated π literal
|
|
|
|
**File:** `becomingone/transducers/emissary.py:55`
|
|
|
|
```python
|
|
omega: float = 2.0 * 3.14159 * 10 # 3.14159 vs. math.pi = 3.141592653589793
|
|
```
|
|
|
|
All other files use `math.pi`. The truncation introduces a ~0.002% error in ω, which is small but inconsistent with the mathematical precision claimed in the documentation.
|
|
|
|
**Fix:** `omega: float = 2.0 * math.pi * 10`
|
|
|
|
---
|
|
|
|
### M6. `_create_echoes()` always scans dict insertion order, not recency
|
|
|
|
**File:** `becomingone/memory/temporal.py:657`
|
|
|
|
```python
|
|
for other_id, other_sig in list(self.signatures.items())[:50]: # First 50, not recent 50!
|
|
```
|
|
|
|
This always scans the 50 oldest entries. The `temporal_index` (a sorted list of `(created_at, id)` tuples) exists precisely for recency-ordered access but is not used here.
|
|
|
|
**Fix:**
|
|
```python
|
|
recent_ids = [sid for _, sid in self.temporal_index[-50:]]
|
|
for other_id in recent_ids:
|
|
other_sig = self.signatures.get(other_id)
|
|
if not other_sig or other_id == signature.signature_id:
|
|
continue
|
|
...
|
|
```
|
|
|
|
---
|
|
|
|
### M7. Ledger file written to CWD — fragile path behavior
|
|
|
|
**File:** `becomingone/memory/ledger.py:28`
|
|
|
|
```python
|
|
LEDGER_FILE = "fieldprint_ledger.jsonl" # Relative to whatever CWD is at runtime
|
|
```
|
|
|
|
The ledger is appended to a file in the current working directory. When run from different directories (e.g., tests vs. the Flask server), multiple disconnected ledgers will be created. The memory JSONL (`memory.jsonl`) has the same problem.
|
|
|
|
**Fix:** Use an absolute path anchored to the module or a configurable data directory:
|
|
```python
|
|
from pathlib import Path
|
|
DEFAULT_LEDGER_DIR = Path.home() / ".becomingone"
|
|
LEDGER_FILE = str(DEFAULT_LEDGER_DIR / "fieldprint_ledger.jsonl")
|
|
```
|
|
|
|
---
|
|
|
|
## 🔵 LOW — Code Quality & Maintenance
|
|
|
|
### L1. Shallow test coverage — tests pass without exercising logic
|
|
|
|
**File:** `tests/test_core.py`
|
|
|
|
The majority of tests (e.g., `test_import`, `test_instantiate`) only assert `assertIsNotNone(ClassObject)` — they verify that a class can be constructed, not that it produces correct outputs. Combined with the `async` issue in C4 above, none of the core mathematical behaviors are actually tested.
|
|
|
|
**Suggested additions:**
|
|
- Verify that coherence increases monotonically with repeated identical-phase inputs
|
|
- Test that `CollapseCondition.evaluate()` returns `(True, ...)` at and above threshold
|
|
- Test `verify_ledger()` returns `False` on a tampered ledger
|
|
- Ensure `TemporalMemory.encode()` returns `None` below threshold and a `TemporalSignature` above
|
|
|
|
---
|
|
|
|
### L2. Mixed-language comment in public API (`链条`)
|
|
|
|
**File:** `becomingone/memory/temporal.py:64, 77`
|
|
|
|
```python
|
|
parent_id: Optional[str] = None # Added: parent signature for链条
|
|
```
|
|
|
|
The Chinese character `链条` (meaning "chain") appears without a space or context. In a public API, all docstrings and inline comments should be in one consistent language.
|
|
|
|
---
|
|
|
|
### L3. `TemporalState.coherence` null-check is dead code
|
|
|
|
**File:** `becomingone/memory/temporal.py:256-259`
|
|
|
|
```python
|
|
if temporal_state.coherence is None: # Can never be True
|
|
coherence = self.calculator.calculate(temporal_state)
|
|
else:
|
|
coherence = temporal_state.coherence
|
|
```
|
|
|
|
`TemporalState.coherence` is typed `float` (non-optional, no default), so it can never be `None`. The `if` branch is dead code. Remove it and use `coherence = temporal_state.coherence` directly (and fix C1 above separately).
|
|
|
|
---
|
|
|
|
## Suggested Priority Order for Fixes
|
|
|
|
| Priority | Issue | Effort |
|
|
|---|---|---|
|
|
| 1 | C1: `calculate()` → `update()` | Low |
|
|
| 2 | C2: `phase_history` → `metadata.get(...)` | Low |
|
|
| 3 | H1: XSS `innerHTML` → `textContent` | Trivial |
|
|
| 4 | H2: Ledger race condition + threading.Lock | Medium |
|
|
| 5 | H3: Event loop leak → `asyncio.run()` | Low |
|
|
| 6 | C3: Add null guard in app.py after encode() | Low |
|
|
| 7 | C4: Fix async tests with asyncio.run() | Low |
|
|
| 8 | M1: Replace `datetime.utcnow()` | Low |
|
|
| 9 | M2: Noise before/after normalization | Medium |
|
|
| 10 | M4: Remove `asyncio` from requirements.txt | Trivial |
|
|
|
|
---
|
|
|
|
*Signed: Claude Sonnet 4.6 (`claude-sonnet-4-6`) — Anthropic AI, model knowledge cutoff August 2025*
|