Adjust a ragged row and an unreadable colour rather than throwing the file away, and notice a tool call that ran out of room

distinct_agent/harness.py

@@ -163,6 +163,8 @@ class StructuredToolHarness:
         artifacts: list[Mapping[str, Any]] = []
         model_usages: list[Mapping[str, Any]] = []
         nudged = False
+        shortened = False
+        ran_out_of_room = False
 
         # Two spare turns beyond the tool budget: one for the final answer, and
         # one for the single correction below. The loop still cannot run away,
@@ -193,6 +195,14 @@ class StructuredToolHarness:
             )
             model_usages.append(dict(inference.usage))
             action = _parse_action(inference.text, self.max_action_characters)
+            if action is None and was_truncated(inference.usage):
+                # Not prose: a call that ran past the output cap. Say so, and
+                # ask for a smaller one rather than accepting the fragment.
+                ran_out_of_room = True
+                if not shortened:
+                    shortened = True
+                    current_prompt = _too_long_prompt(current_prompt)
+                    continue
             answered_without_acting = action is None or action["type"] == "final"
             if answered_without_acting and makers and not events and not nudged:
                 # Finished on the first move, when a tool here makes files.
@@ -208,7 +218,15 @@ class StructuredToolHarness:
             if action is None:
                 progress(1.0, "Local inference complete")
                 return HarnessResult(
-                    text=inference.text,
+                    text=(
+                        inference.text
+                        + "\n\n[This run's last tool call was cut off before it "
+                        "finished, so nothing was produced by it. The request "
+                        "needs more output room than this model has, or fewer "
+                        "things asked for at once.]"
+                        if ran_out_of_room
+                        else inference.text
+                    ),
                     usage=_usage(self.name, model_usages, events),
                     tool_events=tuple(events),
                     artifacts=tuple(artifacts),
@@ -1085,6 +1103,42 @@ def _initial_tool_prompt(prompt: str, manifest: tuple[Mapping[str, Any], ...]) -
     )
 
 
+#: What llama-server calls a reply that ran out of room rather than finishing.
+_RAN_OUT_OF_ROOM = {"length", "limit", "max_tokens"}
+
+
+def was_truncated(usage: Mapping[str, Any]) -> bool:
+    """Whether the model stopped because it hit the output cap, not because it finished."""
+
+    return str(usage.get("stop_reason") or "").casefold() in _RAN_OUT_OF_ROOM
+
+
+def _too_long_prompt(previous: str) -> str:
+    """The last action was cut off mid-JSON. Ask for a smaller one.
+
+    A TRUNCATED TOOL CALL LOOKS EXACTLY LIKE PROSE, AND THAT COST A WORKLOAD.
+
+    Held to the action schema, the model emitted a perfectly correct call --
+    and the call was a whole recipe book in one argument, so it ran past the
+    output cap and stopped mid-string. What arrived was not valid JSON, so it
+    parsed as prose, so the run finished with a paragraph and no file, and the
+    result said "produced 0 files" as though the model had ignored the tool.
+    The two failures need opposite fixes and looked identical.
+
+    The reply itself says which happened -- ``finish_reason`` is ``length``
+    rather than ``stop`` -- so it is used, and the model is told the thing it
+    can act on: make a smaller call.
+    """
+
+    return (
+        f"{previous}\n\n"
+        "YOUR LAST ACTION WAS CUT OFF because it was too long to finish, so it\n"
+        "did nothing. Make a smaller call: put less in each argument, and if the\n"
+        "request needs a long document, produce it in several calls rather than\n"
+        "one. Reply with ONE JSON object and nothing else."
+    )
+
+
 def _nudge_prompt(previous: str, makers: tuple[str, ...]) -> str:
     """Asked for a file, wrote prose. Say so once, and ask again.
 

distinct_skills/handlers.py

@@ -330,15 +330,12 @@ def csv_table(arguments: Mapping[str, object], skill: SkillDefinition) -> dict[s
         guarded_header.append(safe)
 
     rows: list[list[str]] = []
+    adjusted: list[str] = []
     for number, line in lines[1:]:
         cells = [cell.strip() for cell in line.split(separator)]
-        if len(cells) != len(header):
-            # Padding the row would invent data and silently change what the
-            # user is told their table contains. Refusing names the line, so
-            # the caller can fix the one line rather than guess.
-            raise SkillInputError(
-                f"line {number} has {len(cells)} fields but the header has {len(header)}"
-            )
+        cells, note = fit_row(cells, len(header), number)
+        if note:
+            adjusted.append(note)
         row: list[str] = []
         for cell in cells:
             safe, changed = _guard(cell)
@@ -357,6 +354,9 @@ def csv_table(arguments: Mapping[str, object], skill: SkillDefinition) -> dict[s
         # Reported rather than hidden: a guarded cell is a cell whose text was
         # changed, and the user is entitled to know their data was edited.
         "guarded_cells": guarded,
+        # Same reasoning, same place: a row that did not fit the header was
+        # made to fit, and every one of them is named here.
+        "adjusted_rows": adjusted,
         "artifact": text_artifact(
             _filename(arguments.get("filename"), "table.csv", ".csv"), "text/csv", text
         ),
@@ -368,6 +368,44 @@ def csv_table(arguments: Mapping[str, object], skill: SkillDefinition) -> dict[s
 # --------------------------------------------------------------------------
 
 
+def fit_row(cells: list[str], width: int, number: int) -> tuple[list[str], str]:
+    """Make one row the header's width, and say what was done to it.
+
+    REFUSING THE WHOLE TABLE OVER ONE RAGGED LINE MEANT NO TABLE AT ALL.
+
+    The rule here was: a row whose field count does not match the header is an
+    error, because padding it would invent data and silently change what the
+    user is told their table contains. The instinct is right and the second
+    half is the important part -- *silently*. What it produced in practice,
+    with a model doing the typing, was a run that called the tool, got a
+    refusal about line seven, and ended with no spreadsheet: two of twenty
+    benchmark workloads, both of them asking for a table.
+
+    Nothing is invented and nothing is hidden. A short row is padded with
+    empty cells; a long one keeps every field, with the surplus joined into
+    the last column so no text is dropped. Both come back with a sentence
+    naming the line and what happened to it, which the caller reports beside
+    the table, so a person reading it can go and look at line seven exactly as
+    they could before -- with the difference that they also have their table.
+    """
+
+    if len(cells) == width:
+        return cells, ""
+    if len(cells) < width:
+        missing = width - len(cells)
+        return (
+            cells + [""] * missing,
+            f"line {number} had {len(cells)} fields where the header has {width}; "
+            f"{missing} empty cell(s) were added at the end",
+        )
+    kept = cells[: width - 1] + [" ".join(cells[width - 1 :])]
+    return (
+        kept,
+        f"line {number} had {len(cells)} fields where the header has {width}; "
+        "the surplus was joined into the last column",
+    )
+
+
 def _multiline(arguments: Mapping[str, object], name: str, *, limit: int) -> str:
     """Like ``_line`` but newlines are structure here, so they survive.
 
@@ -432,7 +470,7 @@ def _zip_bytes(parts: Mapping[str, str]) -> bytes:
 _SEPARATORS: Mapping[str, str] = {"pipe": "|", "tab": "\t", "semicolon": ";"}
 
 
-def _rows_from_text(arguments: Mapping[str, object]) -> list[list[str]]:
+def _rows_from_text(arguments: Mapping[str, object]) -> tuple[list[list[str]], list[str]]:
     """Header-then-rows lines, split on the named separator, shape-checked.
 
     The same input convention as ``csv_table`` and for the same reason: a
@@ -450,13 +488,12 @@ def _rows_from_text(arguments: Mapping[str, object]) -> list[list[str]]:
         raise SkillInputError("text needs a header line and at least one row")
     rows = [[cell.strip() for cell in line.split(separator)] for line in lines]
     width = len(rows[0])
-    for index, row in enumerate(rows[1:], start=2):
-        if len(row) != width:
-            raise SkillInputError(
-                f"line {index} has {len(row)} fields where the header has {width}. "
-                "Fix the row rather than letting a column silently shift"
-            )
-    return rows
+    adjusted: list[str] = []
+    for index in range(1, len(rows)):
+        rows[index], note = fit_row(rows[index], width, index + 1)
+        if note:
+            adjusted.append(note)
+    return rows, adjusted
 
 
 _DOCX_CONTENT_TYPES = (
@@ -607,7 +644,7 @@ def spreadsheet_xlsx(arguments: Mapping[str, object], skill: SkillDefinition) ->
 
     del skill
     _known(arguments, frozenset({"text", "separator", "filename"}))
-    rows = _rows_from_text(arguments)
+    rows, adjusted = _rows_from_text(arguments)
     body: list[str] = []
     for row_index, row in enumerate(rows, start=1):
         cells = []
@@ -631,11 +668,14 @@ def spreadsheet_xlsx(arguments: Mapping[str, object], skill: SkillDefinition) ->
         }
     )
     return {
+        # Named, never silent: a row that did not fit the header was made to
+        # fit, and the caller is told which and how.
+        "adjusted_rows": adjusted,
         "artifact": bytes_artifact(
             _filename(arguments.get("filename"), "table.xlsx", ".xlsx"),
             "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
             payload,
-        )
+        ),
     }
 
 
@@ -931,21 +971,42 @@ def theme_css(arguments: Mapping[str, object], skill: SkillDefinition) -> dict[s
     colors_text = _multiline(arguments, "colors", limit=1200)
     heading = str(arguments.get("heading_font") or "Georgia, serif").strip()[:80]
     body_font = str(arguments.get("body_font") or "Arial, sans-serif").strip()[:80]
+    # ONE BAD COLOUR LINE USED TO COST THE WHOLE THEME.
+    #
+    # "Primary: #1a1a2e" has a capital P and was refused; so was
+    # "background: rgb(20,20,30)". Either one ended the run with no stylesheet
+    # at all, which is how a benchmark workload asking for a theme file failed
+    # three calls in a row. A name that only needs normalising is normalised,
+    # a value that cannot be read is skipped and named, and the sheet is only
+    # refused when there is nothing left to put in it. Same rule as the ragged
+    # table rows: adjust, never silently.
     variables = []
+    skipped: list[str] = []
     for line in colors_text.splitlines():
         line = line.strip()
         if not line:
             continue
         key, _, value = line.partition(":")
-        key = key.strip().lower()
+        key = re.sub(r"[\s_]+", "-", key.strip().lower()).strip("-")
         value = value.strip()
+        if not re.fullmatch(r"#[0-9A-Fa-f]{3,8}", value) and re.fullmatch(
+            r"[0-9A-Fa-f]{6}|[0-9A-Fa-f]{3}", value
+        ):
+            # A hex value with the hash left off is the commonest near-miss and
+            # is unambiguous, so it is completed rather than discarded.
+            value = f"#{value}"
         if not re.fullmatch(r"[a-z][a-z0-9-]{0,30}", key):
-            raise SkillInputError(f"colour name {key!r} must be lowercase-with-hyphens")
+            skipped.append(f"{key or line!r}: not a usable colour name")
+            continue
         if not re.fullmatch(r"#[0-9A-Fa-f]{3,8}", value):
-            raise SkillInputError(f"colour {key!r} needs a #hex value, got {value!r}")
+            skipped.append(f"{key}: {value!r} is not a #hex value")
+            continue
         variables.append(f"  --{key}: {value};")
     if not variables:
-        raise SkillInputError("colors needs at least one 'name: #hex' line")
+        raise SkillInputError(
+            "colors needs at least one 'name: #hex' line"
+            + (f"; none was usable ({'; '.join(skipped[:4])})" if skipped else "")
+        )
     sheet = (
         f"/* Theme: {theme_name} — generated by distinct */\n"
         ":root {\n" + "\n".join(variables) + "\n"
@@ -956,9 +1017,12 @@ def theme_css(arguments: Mapping[str, object], skill: SkillDefinition) -> dict[s
         "h1, h2, h3, h4 { font-family: var(--font-heading); }\n"
     )
     return {
+        # Named, never silent: a colour line that could not be read was left
+        # out, and the caller is told which and why.
+        "skipped_colours": skipped,
         "artifact": text_artifact(
             _filename(arguments.get("filename"), "theme.css", ".css"), "text/css", sheet
-        )
+        ),
     }
 
 

tests/test_harness_makes_files.py

@@ -493,3 +493,72 @@ def test_a_follow_up_question_is_not_forced_to_make_a_file(tmp_path) -> None:
         progress=lambda fraction, message: None,
     )
     assert "anyOf" in schemas[0]
+
+
+# -- a call that ran out of room ---------------------------------------------
+
+
+class _Truncating:
+    """A model whose first action is cut off mid-JSON, as a real one is."""
+
+    def __init__(self, *outputs: str) -> None:
+        self._outputs = list(outputs)
+        self.prompts: list[str] = []
+
+    def run(self, model, prompt, *, cancel_event=None, progress=None, limits=None):
+        self.prompts.append(prompt)
+        text = self._outputs.pop(0) if self._outputs else '{"type":"final","answer":"done"}'
+        cut = text.startswith("{") and not text.rstrip().endswith("}")
+        from distinct_agent.runners import InferenceResult
+
+        return InferenceResult(
+            text=text,
+            usage={"runner": "fake", "stop_reason": "length" if cut else "stop"},
+        )
+
+
+def test_a_cut_off_call_is_recognised_as_one() -> None:
+    from distinct_agent.harness import was_truncated
+
+    assert was_truncated({"stop_reason": "length"})
+    assert not was_truncated({"stop_reason": "stop"})
+    assert not was_truncated({})
+
+
+def test_a_cut_off_call_is_asked_for_again_smaller(tmp_path) -> None:
+    """It looks exactly like prose, and the two need opposite fixes."""
+
+    runner = _Truncating(
+        '{"type":"tool","tool":"create-deck@1","arguments":{"slides":"a very long ',
+        json.dumps({"type": "tool", "tool": "create-deck@1", "arguments": {"slides": "A :: b"}}),
+        json.dumps({"type": "final", "answer": "deck built"}),
+    )
+    broker = _Broker((DECK,))
+    result = _run(StructuredToolHarness(), _job(), runner, broker, tmp_path)
+    assert "CUT OFF" in runner.prompts[1]
+    assert broker.calls and result.text == "deck built"
+
+
+def test_a_run_that_kept_running_out_of_room_says_so(tmp_path) -> None:
+    """"produced 0 files" hid the reason; the answer now carries it."""
+
+    fragment = '{"type":"tool","tool":"create-deck@1","arguments":{"slides":"still going '
+    result = _run(
+        StructuredToolHarness(),
+        _job(),
+        _Truncating(fragment, fragment, fragment),
+        _Broker((DECK,)),
+        tmp_path,
+    )
+    assert "cut off" in result.text
+
+
+def test_a_finished_answer_is_not_treated_as_cut_off(tmp_path) -> None:
+    result = _run(
+        StructuredToolHarness(),
+        _job(),
+        _Truncating(json.dumps({"type": "final", "answer": "all done"})),
+        _Broker((CALCULATOR,)),
+        tmp_path,
+    )
+    assert result.text == "all done"

tests/test_ragged_rows_still_make_a_table.py

@@ -0,0 +1,173 @@
+"""One bad line used to cost the whole spreadsheet, and it cost two workloads.
+
+The rule was: a row whose field count does not match the header is an error,
+because padding it would invent data and silently change what the user is told
+their table contains. The instinct is right; the load-bearing word is
+*silently*. With a model doing the typing, what it produced was a run that
+called the tool, got a refusal naming line seven, and finished with no file at
+all -- twice in twenty benchmark workloads, both of them asking for a table.
+
+So the row is made to fit and every adjustment is named in the result. Nothing
+is invented, nothing is dropped, nothing is hidden, and the person gets their
+table.
+"""
+
+from __future__ import annotations
+
+import base64
+
+import pytest
+
+from distinct_skills.handlers import csv_table, fit_row, spreadsheet_xlsx
+from distinct_skills.loader import SkillInputError
+
+HEADER = "name|qty|aisle"
+
+
+def _csv(text: str) -> tuple[str, list[str]]:
+    result = csv_table({"text": text}, None)
+    return base64.b64decode(result["artifact"]["base64"]).decode(), result["adjusted_rows"]
+
+
+# -- the adjustment itself ---------------------------------------------------
+
+
+def test_a_row_that_already_fits_is_left_exactly_as_it_is() -> None:
+    assert fit_row(["a", "b", "c"], 3, 2) == (["a", "b", "c"], "")
+
+
+def test_a_short_row_is_padded_and_the_padding_is_named() -> None:
+    cells, note = fit_row(["a", "b"], 3, 7)
+    assert cells == ["a", "b", ""]
+    assert "line 7" in note and "empty cell" in note
+
+
+def test_a_long_row_keeps_every_field_it_was_given() -> None:
+    """Dropping the surplus would lose the user's text, which is worse."""
+
+    cells, note = fit_row(["a", "b", "c", "d"], 3, 4)
+    assert cells == ["a", "b", "c d"]
+    assert "line 4" in note and "surplus" in note
+
+
+def test_nothing_is_ever_invented() -> None:
+    """Padding adds empty cells. It never guesses a value."""
+
+    cells, _note = fit_row(["a"], 4, 2)
+    assert cells == ["a", "", "", ""]
+
+
+# -- through the tools -------------------------------------------------------
+
+
+def test_a_ragged_line_no_longer_costs_the_whole_csv() -> None:
+    text, adjusted = _csv(f"{HEADER}\nflour|500 g|baking\nsalt|10 g\nsaffron|1 g|spices|extra")
+    assert "flour,500 g,baking" in text
+    assert "salt,10 g," in text
+    assert "saffron,1 g,spices extra" in text
+    assert len(adjusted) == 2
+
+
+def test_every_adjustment_names_the_line_it_happened_on() -> None:
+    """So a person can go and look at line seven exactly as they could before."""
+
+    _text, adjusted = _csv(f"{HEADER}\na|b|c\nd|e")
+    assert adjusted and adjusted[0].startswith("line 3 ")
+
+
+def test_a_table_that_needed_no_adjustment_reports_none() -> None:
+    _text, adjusted = _csv(f"{HEADER}\na|b|c\nd|e|f")
+    assert adjusted == []
+
+
+def test_the_spreadsheet_reports_its_adjustments_too() -> None:
+    result = spreadsheet_xlsx({"text": f"{HEADER}\na|b|c\nd|e"}, None)
+    assert len(result["adjusted_rows"]) == 1
+    assert result["artifact"]["size_bytes"] > 0
+
+
+# -- what is still refused ---------------------------------------------------
+
+
+def test_a_table_with_no_rows_is_still_refused() -> None:
+    """There is no row to make fit, so there is nothing to produce."""
+
+    with pytest.raises(SkillInputError):
+        csv_table({"text": HEADER}, None)
+
+
+def test_a_header_with_no_separator_is_still_refused() -> None:
+    """A one-column table is almost always a caller who used the wrong separator."""
+
+    with pytest.raises(SkillInputError):
+        csv_table({"text": "name qty aisle\na b c"}, None)
+
+
+def test_a_cell_that_would_run_as_a_formula_is_still_guarded() -> None:
+    """Making rows fit must not have loosened the thing that stops a cell running."""
+
+    text, _adjusted = _csv(f"{HEADER}\n=cmd()|1|x")
+    assert "'=cmd()" in text
+
+
+def test_the_joined_cell_is_guarded_because_joining_happens_first() -> None:
+    """Order matters: rows are made to fit, and *then* every cell is checked."""
+
+    text, adjusted = _csv(f"{HEADER}\na|b|=cmd()|tail")
+    assert adjusted
+    assert "'=cmd() tail" in text
+
+
+# -- the same rule, applied to a palette -------------------------------------
+
+
+def _theme(colors: str):
+    from distinct_skills.handlers import theme_css
+
+    result = theme_css({"name": "Night Desk", "colors": colors}, None)
+    return base64.b64decode(result["artifact"]["base64"]).decode(), result["skipped_colours"]
+
+
+def test_a_capitalised_colour_name_is_normalised_rather_than_refused() -> None:
+    """"Primary: #1a1a2e" used to end the run with no stylesheet."""
+
+    sheet, skipped = _theme("Primary: #1a1a2e")
+    assert "--primary: #1a1a2e;" in sheet
+    assert skipped == []
+
+
+def test_spaces_and_underscores_in_a_name_become_hyphens() -> None:
+    sheet, _skipped = _theme("Background Deep: #16213e\nmuted_grey: #888")
+    assert "--background-deep: #16213e;" in sheet
+    assert "--muted-grey: #888;" in sheet
+
+
+def test_a_hex_value_with_the_hash_left_off_is_completed() -> None:
+    """The commonest near-miss, and unambiguous."""
+
+    sheet, skipped = _theme("primary: 1a1a2e")
+    assert "--primary: #1a1a2e;" in sheet
+    assert skipped == []
+
+
+def test_a_colour_that_cannot_be_read_is_skipped_and_named() -> None:
+    sheet, skipped = _theme("primary: #1a1a2e\naccent: rgb(20,20,30)")
+    assert "--primary: #1a1a2e;" in sheet
+    assert "--accent" not in sheet
+    assert skipped and "accent" in skipped[0]
+
+
+def test_a_palette_with_nothing_usable_is_still_refused() -> None:
+    """There is no stylesheet to make, so saying so is the honest answer."""
+
+    from distinct_skills.handlers import theme_css
+
+    with pytest.raises(SkillInputError):
+        theme_css({"name": "x", "colors": "accent: rgb(1,2,3)"}, None)
+
+
+def test_the_refusal_says_which_lines_it_could_not_read() -> None:
+    from distinct_skills.handlers import theme_css
+
+    with pytest.raises(SkillInputError, match="rgb"):
+        theme_css({"name": "x", "colors": "accent: rgb(1,2,3)"}, None)