💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f77f9d6d66

Actionable comments posted: 2

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/features/mostro/mostro_instance.dart`:
- Around line 204-207: The getter bondSlashNodeSharePct currently parses any
double; update it to parse using
double.tryParse(_getOptionalTagValue('bond_slash_node_share_pct')) and then
validate the parsed value is between 0.0 and 1.0 inclusive, returning null for
parse failures or out-of-range values so downstream logic never sees invalid
percentages; use the existing _getOptionalTagValue call and return the validated
double only when value != null && value >= 0.0 && value <= 1.0.
- Around line 129-133: _guard optional-tag parsing in _getOptionalTagValue: the
predicate passed to firstWhere must check t.length>0 before accessing t[0] to
avoid runtime errors for malformed tags, and returned tag values should map
empty strings to null. Change the predicate to (t) => t.length > 0 && t[0] ==
key, keep orElse: () => [], then after finding tag check tag.length < 2 => null,
then treat an empty tag[1] (tag[1].isEmpty) as null before returning the string.

Code review

No la aprobaría todavía.

Lo que me frena

• bondSlashOnWaitingTimeout convierte cualquier valor no-true en false.
  Eso aplana valores malformados ("foo", "", etc.) en un estado válido y hace que un dato corrupto sea indistinguible de un false real.
• Dado que el resto de los nuevos campos ya usan tryParse/null para no inventar valores, aquí esperaría la misma defensa: null para valores inválidos, o al menos un parser explícito para true/false בלבד.

Lo que sí está bien

• La separación entre unsupported y disabled para bond_enabled tiene sentido.
• La cobertura de tests para los nuevos campos numéricos está bien encaminada.

Si ajustan el parser booleano, sí lo vería en condiciones de aprobar.

Code review

Still not approvable.

Blocking issue

• bondSlashOnWaitingTimeout still collapses every non-true value into false.
  That means malformed input like "foo" or "" becomes indistinguishable from a deliberate false.
  Since the rest of the new bond fields are parsed defensively (tryParse/null), this one should follow the same pattern and return null for invalid values, or use a strict boolean parser.

What looks good

• The unsupported vs disabled split for bond_enabled is correct.
• The new tests around the optional numeric fields are useful.

Fix that parser and I’d be comfortable approving it.

Code review

✅ Approved.

Re-checked the exact head SHA and the blocker is gone:

• bond_slash_on_waiting_timeout now returns null for malformed values.
• The test suite covers both valid booleans and malformed inputs explicitly.

The rest of the bond parsing looks solid enough to merge.

Hermes Agent Review

I found one blocking issue before approval. See the inline comment.

Blocking

• The empty-tag guard was removed from _getTagValue, so a malformed empty tag can now throw a RangeError when any getter reads it.

Hermes Agent Review

Re-checked the current head SHA. The previous blocking issue is fixed, but I still found one correctness problem before approval.

Blocking

• bondPolicy still maps any non-true / non-missing value to BondPolicy.disabled. That means malformed payloads like bond_enabled="foo" are indistinguishable from an intentional false, which breaks the three-state semantics the PR is introducing. I would return null/unsupported for malformed values, or parse false explicitly and treat everything else as invalid.

Hermes Agent Review

Re-checked the current head 835d1516d30dc52546d9b024a93b6085e9b34c6d. The bond parsing is now strict, the empty-tag guard is in place, and the tests cover the malformed cases. LGTM.

tACK